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

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

[Chris Wilson]

[   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@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@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);
+	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)
 		return 0;
 
+	old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc);
+	new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+
 	new_crtc_state->min_cdclk[plane->id] =
 		plane->min_cdclk(new_crtc_state, plane_state);
 
-- 
2.25.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Fix NULL-crtc deref in calc_min_cdclk()

[Patchwork]

== Series Details ==

Series: drm/i915/display: Fix NULL-crtc deref in calc_min_cdclk()
URL   : https://patchwork.freedesktop.org/series/72875/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7856 -> Patchwork_16380
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_16380 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16380, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16380:

### IGT changes ###

#### Warnings ####

  * igt@kms_psr@primary_mmap_gtt:
    - fi-skl-6770hq:      [SKIP][1] ([fdo#109271]) -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7856/fi-skl-6770hq/igt@kms_psr@primary_mmap_gtt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/fi-skl-6770hq/igt@kms_psr@primary_mmap_gtt.html

  
Known issues
------------

  Here are the changes found in Patchwork_16380 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [PASS][3] -> [FAIL][4] ([i915#217])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7856/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-icl-y:           [DMESG-FAIL][5] ([fdo#108569]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7856/fi-icl-y/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/fi-icl-y/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [DMESG-FAIL][7] ([i915#1052]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7856/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  * igt@i915_selftest@live_gtt:
    - fi-hsw-4770:        [TIMEOUT][9] ([fdo#112271]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7856/fi-hsw-4770/igt@i915_selftest@live_gtt.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/fi-hsw-4770/igt@i915_selftest@live_gtt.html

  
#### Warnings ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [DMESG-FAIL][11] ([i915#553] / [i915#725]) -> [DMESG-FAIL][12] ([i915#725])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7856/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/fi-hsw-4770/igt@i915_selftest@live_blt.html

  
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#1052]: https://gitlab.freedesktop.org/drm/intel/issues/1052
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725


Participating hosts (43 -> 40)
------------------------------

  Additional (6): fi-bdw-5557u fi-byt-j1900 fi-bwr-2160 fi-bsw-kefka fi-skl-lmem fi-bsw-nick 
  Missing    (9): fi-ilk-m540 fi-bsw-n3050 fi-hsw-4200u fi-bsw-cyan fi-ivb-3770 fi-bdw-samus fi-byt-clapper fi-skl-6700k2 fi-snb-2600 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7856 -> Patchwork_16380

  CI-20190529: 20190529
  CI_DRM_7856: a113999b001035a5b6474407b228363c163574a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5411: 86c6ab8a0b6696bdb2153febd350af7fa02fbb00 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16380: a25a96831d8e5877cd75598340a0164ad4df2592 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a25a96831d8e drm/i915/display: Fix NULL-crtc deref in calc_min_cdclk()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16380/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@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.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		return 0;
>  
> +	old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc);
> +	new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> +
>  	new_crtc_state->min_cdclk[plane->id] =
>  		plane->min_cdclk(new_crtc_state, plane_state);
>  
> -- 
> 2.25.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx