[Intel-gfx] [PATCH] drm/i915/display: Fix NULL-crtc deref in calc_min_cdclk()

[Chris Wilson]

Quoting Ville Syrjälä (2020-02-03 13:29:07)
> On Sun, Feb 02, 2020 at 01:43:38PM +0000, Chris Wilson wrote:
> > [   23.419442] BUG: KASAN: null-ptr-deref in intel_plane_calc_min_cdclk+0x82/0x440 [i915]
> > [   23.419527] Read of size 4 at addr 00000000000000f8 by task insmod/735
> > [   23.419578]
> > [   23.419644] CPU: 2 PID: 735 Comm: insmod Not tainted 5.5.0+ #114
> > [   23.419716] Hardware name: ��������������������������������� ���������������������������������/���������������������������������, BIOS RYBDWi35.86A.0246.2
> > [   23.419793] Call Trace:
> > [   23.419864]  dump_stack+0xef/0x16e
> > [   23.419927]  __kasan_report.cold+0x60/0x90
> > [   23.420157]  ? intel_plane_calc_min_cdclk+0x82/0x440 [i915]
> > [   23.420397]  intel_plane_calc_min_cdclk+0x82/0x440 [i915]
> > [   23.420630]  intel_atomic_check+0x455f/0x65a0 [i915]
> > [   23.420708]  ? mark_held_locks+0x90/0x90
> > [   23.420929]  ? intel_crtc_duplicate_state+0x2e/0x1b0 [i915]
> > [   23.421172]  ? intel_plane_duplicate_state+0x2d/0xc0 [i915]
> > [   23.421239]  ? __drm_dbg+0xa4/0x120
> > [   23.421303]  ? __kasan_kmalloc.constprop.0+0xc2/0xd0
> > [   23.421355]  ? __kmalloc_track_caller+0x23a/0x320
> > [   23.421602]  ? intel_calc_active_pipes+0x1c0/0x1c0 [i915]
> > [   23.421852]  sanitize_watermarks+0x220/0x510 [i915]
> > [   23.422092]  ? intel_atomic_check+0x65a0/0x65a0 [i915]
> > [   23.422164]  ? drm_modeset_unlock_all+0x88/0x130
> > [   23.422402]  intel_modeset_init+0x1b76/0x3c90 [i915]
> > [   23.422647]  ? intel_finish_reset+0x2d0/0x2d0 [i915]
> > [   23.422851]  ? intel_irq_install+0x12c/0x210 [i915]
> > [   23.423076]  i915_driver_probe+0x13e7/0x2930 [i915]
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > index 91ab6e2ab1fd..83505bd8ad80 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -165,14 +165,15 @@ int intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
> >               intel_atomic_get_new_plane_state(state, plane);
> >       struct intel_crtc *crtc = to_intel_crtc(plane_state->hw.crtc);
> >       const struct intel_cdclk_state *cdclk_state;
> > -     struct intel_crtc_state *new_crtc_state =
> > -             intel_atomic_get_new_crtc_state(state, crtc);
> > -     const struct intel_crtc_state *old_crtc_state =
> > -             intel_atomic_get_old_crtc_state(state, crtc);
> 
> Doh. Not NULL safe clearly.
> 
> > +     const struct intel_crtc_state *old_crtc_state;
> > +     struct intel_crtc_state *new_crtc_state;
> >  
> > -     if (!plane_state->uapi.visible || !plane->min_cdclk)
> > +     if (!crtc || !plane_state->uapi.visible || !plane->min_cdclk)
> 
> Should be no need for the !crtc check. We can't have visible==true
> without a crtc.

Ta, I thought that might be the case.
-Chris