* [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
@ 2019-04-16 3:12 clinton.a.taylor
2019-04-16 4:36 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: clinton.a.taylor @ 2019-04-16 3:12 UTC (permalink / raw)
To: Intel-gfx
From: Clint Taylor <clinton.a.taylor@intel.com>
Add protections to prevent NULL de-reference for a couple variables used
in skl_check_pipe_max_pixel_clock to prevent GP exception from occurring
during some IGT tests.
References: https://bugs.freedesktop.org/show_bug.cgi?id=109084
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 4 ++++
drivers/gpu/drm/i915/intel_pm.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3bd40a4a6739..945861cef520 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11377,6 +11377,10 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
if (!ret)
ret = icl_check_nv12_planes(pipe_config);
+
+ if (WARN_ON(!intel_crtc))
+ return -EINVAL;
+
if (!ret)
ret = skl_check_pipe_max_pixel_rate(intel_crtc,
pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7357bddf9ad9..df5d01d4345b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4160,6 +4160,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8);
int bpp;
+ if (WARN_ON(!pstate))
+ return -EINVAL;
+
if (!intel_wm_plane_visible(cstate,
to_intel_plane_state(pstate)))
continue;
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
2019-04-16 3:12 [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock clinton.a.taylor
@ 2019-04-16 4:36 ` Patchwork
2019-04-16 5:44 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-04-19 21:06 ` [PATCH] " Matt Roper
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-04-16 4:36 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
URL : https://patchwork.freedesktop.org/series/59547/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5936 -> Patchwork_12811
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/59547/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_12811 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_cs_nop@fork-compute0:
- fi-icl-y: NOTRUN -> SKIP [fdo#109315] +17
- fi-blb-e6850: NOTRUN -> SKIP [fdo#109271] +18
* igt@gem_exec_basic@basic-bsd2:
- fi-icl-y: NOTRUN -> SKIP [fdo#109276] +7
* igt@gem_exec_parse@basic-rejected:
- fi-icl-y: NOTRUN -> SKIP [fdo#109289] +1
* igt@gem_exec_store@basic-bsd2:
- fi-hsw-4770: NOTRUN -> SKIP [fdo#109271] +41
* igt@kms_addfb_basic@addfb25-y-tiled-small:
- fi-byt-n2820: NOTRUN -> SKIP [fdo#109271] +51
* igt@kms_busy@basic-flip-c:
- fi-byt-n2820: NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
* igt@kms_chamelium@dp-crc-fast:
- fi-icl-y: NOTRUN -> SKIP [fdo#109284] +8
* igt@kms_chamelium@hdmi-edid-read:
- fi-hsw-peppy: NOTRUN -> SKIP [fdo#109271] +46
* igt@kms_force_connector_basic@force-load-detect:
- fi-icl-y: NOTRUN -> SKIP [fdo#109285] +3
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]
* igt@kms_psr@primary_mmap_gtt:
- fi-icl-y: NOTRUN -> SKIP [fdo#110189] +3
* igt@kms_psr@primary_page_flip:
- fi-skl-lmem: NOTRUN -> SKIP [fdo#109271] +37
* igt@prime_vgem@basic-fence-flip:
- fi-icl-y: NOTRUN -> SKIP [fdo#109294]
#### Possible fixes ####
* igt@i915_module_load@reload:
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: DMESG-FAIL [fdo#110235 ] -> PASS
* igt@i915_selftest@live_execlists:
- fi-apl-guc: INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (43 -> 42)
------------------------------
Additional (5): fi-hsw-peppy fi-hsw-4770 fi-icl-y fi-skl-lmem fi-byt-n2820
Missing (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_5936 -> Patchwork_12811
CI_DRM_5936: 0ad14bd30d830a1a355040b29bfafbe6623d84f0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4948: cf27a37b867bf31dccbe5f1b3bd84a2e606544f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12811: d3ed751e48aeb60ac003cbb6668df737ac478a47 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
d3ed751e48ae drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12811/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
2019-04-16 3:12 [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock clinton.a.taylor
2019-04-16 4:36 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-04-16 5:44 ` Patchwork
2019-04-19 21:06 ` [PATCH] " Matt Roper
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-04-16 5:44 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
URL : https://patchwork.freedesktop.org/series/59547/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5936_full -> Patchwork_12811_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12811_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12811_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12811_full:
### IGT changes ###
#### Possible regressions ####
* igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
- shard-iclb: PASS -> INCOMPLETE
Known issues
------------
Here are the changes found in Patchwork_12811_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_tiled_swapping@non-threaded:
- shard-iclb: PASS -> FAIL [fdo#108686]
* igt@i915_pm_rpm@basic-pci-d3-state:
- shard-skl: PASS -> INCOMPLETE [fdo#107807] +1
* igt@kms_atomic_transition@3x-modeset-transitions:
- shard-skl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +14
* igt@kms_busy@basic-modeset-e:
- shard-apl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-f:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
* igt@kms_color@pipe-b-gamma:
- shard-skl: PASS -> FAIL [fdo#104782]
* igt@kms_content_protection@atomic:
- shard-apl: NOTRUN -> FAIL [fdo#110321] / [fdo#110336]
* igt@kms_content_protection@legacy:
- shard-kbl: NOTRUN -> FAIL [fdo#110321] / [fdo#110336]
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: PASS -> FAIL [fdo#105363]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-mmap-wc:
- shard-skl: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-blt:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] +21
* igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
- shard-skl: NOTRUN -> SKIP [fdo#109271] +147
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
- shard-iclb: PASS -> FAIL [fdo#109247] +17
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
- shard-iclb: PASS -> FAIL [fdo#103167] +4
* igt@kms_frontbuffer_tracking@psr-2p-scndscrn-cur-indfb-draw-mmap-wc:
- shard-apl: NOTRUN -> INCOMPLETE [fdo#103927]
* igt@kms_lease@atomic_implicit_crtc:
- shard-skl: NOTRUN -> FAIL [fdo#110279]
* igt@kms_lease@cursor_implicit_plane:
- shard-apl: NOTRUN -> FAIL [fdo#110278]
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
- shard-apl: PASS -> DMESG-WARN [fdo#108566] +4
* igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
- shard-kbl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
- shard-apl: NOTRUN -> FAIL [fdo#108145] +1
- shard-skl: NOTRUN -> FAIL [fdo#108145] +3
* igt@kms_psr@cursor_render:
- shard-iclb: PASS -> FAIL [fdo#107383] / [fdo#110215] +1
* igt@kms_psr@psr2_primary_mmap_gtt:
- shard-iclb: PASS -> SKIP [fdo#109441] +2
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-kbl: PASS -> FAIL [fdo#109016]
* igt@kms_setmode@basic:
- shard-apl: NOTRUN -> FAIL [fdo#99912]
- shard-kbl: PASS -> FAIL [fdo#99912]
* igt@prime_nv_api@i915_nv_reimport_twice_check_flink_name:
- shard-apl: NOTRUN -> SKIP [fdo#109271] +49
#### Possible fixes ####
* igt@gem_ctx_isolation@vcs1-s3:
- shard-kbl: DMESG-WARN [fdo#108566] -> PASS +1
* igt@i915_pm_rpm@dpms-non-lpsp:
- shard-apl: DMESG-WARN [fdo#110376] -> PASS
* igt@i915_selftest@live_workarounds:
- shard-iclb: DMESG-FAIL [fdo#108954] -> PASS
* igt@i915_suspend@debugfs-reader:
- shard-apl: DMESG-WARN [fdo#108566] -> PASS +2
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-skl: INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: FAIL [fdo#105363] -> PASS
- shard-glk: FAIL [fdo#102887] -> PASS
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-kbl: FAIL [fdo#102887] / [fdo#105363] -> PASS
- shard-glk: FAIL [fdo#102887] / [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbc-tilingchange:
- shard-iclb: FAIL [fdo#103167] -> PASS +4
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-pwrite:
- shard-iclb: FAIL [fdo#109247] -> PASS +16
* igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
- shard-glk: SKIP [fdo#109271] -> PASS
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: FAIL [fdo#110403] -> PASS +1
* igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
- shard-glk: SKIP [fdo#109271] / [fdo#109278] -> PASS
* igt@kms_psr@psr2_sprite_blt:
- shard-iclb: SKIP [fdo#109441] -> PASS +2
* igt@kms_psr@sprite_mmap_cpu:
- shard-iclb: FAIL [fdo#107383] / [fdo#110215] -> PASS +2
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
[fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
[fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
[fdo#110278]: https://bugs.freedesktop.org/show_bug.cgi?id=110278
[fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
[fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
[fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
[fdo#110376]: https://bugs.freedesktop.org/show_bug.cgi?id=110376
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 9)
------------------------------
Missing (1): shard-hsw
Build changes
-------------
* Linux: CI_DRM_5936 -> Patchwork_12811
CI_DRM_5936: 0ad14bd30d830a1a355040b29bfafbe6623d84f0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4948: cf27a37b867bf31dccbe5f1b3bd84a2e606544f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12811: d3ed751e48aeb60ac003cbb6668df737ac478a47 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12811/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
2019-04-16 3:12 [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock clinton.a.taylor
2019-04-16 4:36 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-04-16 5:44 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-04-19 21:06 ` Matt Roper
2019-04-19 21:12 ` Chris Wilson
2 siblings, 1 reply; 6+ messages in thread
From: Matt Roper @ 2019-04-19 21:06 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: Intel-gfx
On Mon, Apr 15, 2019 at 08:12:50PM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> Add protections to prevent NULL de-reference for a couple variables used
> in skl_check_pipe_max_pixel_clock to prevent GP exception from occurring
> during some IGT tests.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=109084
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++++
> drivers/gpu/drm/i915/intel_pm.c | 3 +++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3bd40a4a6739..945861cef520 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11377,6 +11377,10 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>
> if (!ret)
> ret = icl_check_nv12_planes(pipe_config);
> +
> + if (WARN_ON(!intel_crtc))
If intel_crtc is NULL, then crtc should also be NULL as well, and we
already dereferenced that earlier:
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
So if crtc/intel_crtc were the problem, I believe this check would still
be too late to catch and prevent a crash.
> + return -EINVAL;
> +
> if (!ret)
> ret = skl_check_pipe_max_pixel_rate(intel_crtc,
> pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7357bddf9ad9..df5d01d4345b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4160,6 +4160,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8);
> int bpp;
>
> + if (WARN_ON(!pstate))
> + return -EINVAL;
The pstate here comes from drm_atomic_crtc_state_for_each_plane_state,
which does a 'for_each_if' to only execute the loop body if pstate is
non-NULL, so I don't see how this one could be possible either.
Do you see the driver tripping over either of these guards once this
patch is applied? If we've got a backtrace for a gp fault, can we
convert the RIP back into a specific line of code that triggered the
fault?
Matt
> +
> if (!intel_wm_plane_visible(cstate,
> to_intel_plane_state(pstate)))
> continue;
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
2019-04-19 21:06 ` [PATCH] " Matt Roper
@ 2019-04-19 21:12 ` Chris Wilson
2019-04-19 21:15 ` Matt Roper
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2019-04-19 21:12 UTC (permalink / raw)
To: Matt Roper, clinton.a.taylor; +Cc: Intel-gfx
Quoting Matt Roper (2019-04-19 22:06:05)
> On Mon, Apr 15, 2019 at 08:12:50PM -0700, clinton.a.taylor@intel.com wrote:
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> >
> > Add protections to prevent NULL de-reference for a couple variables used
> > in skl_check_pipe_max_pixel_clock to prevent GP exception from occurring
> > during some IGT tests.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=109084
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Martin Peres <martin.peres@linux.intel.com>
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > drivers/gpu/drm/i915/intel_pm.c | 3 +++
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3bd40a4a6739..945861cef520 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11377,6 +11377,10 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> >
> > if (!ret)
> > ret = icl_check_nv12_planes(pipe_config);
> > +
> > + if (WARN_ON(!intel_crtc))
>
> If intel_crtc is NULL, then crtc should also be NULL as well, and we
> already dereferenced that earlier:
>
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>
> So if crtc/intel_crtc were the problem, I believe this check would still
> be too late to catch and prevent a crash.
>
>
> > + return -EINVAL;
> > +
> > if (!ret)
> > ret = skl_check_pipe_max_pixel_rate(intel_crtc,
> > pipe_config);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 7357bddf9ad9..df5d01d4345b 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4160,6 +4160,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> > uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8);
> > int bpp;
> >
> > + if (WARN_ON(!pstate))
> > + return -EINVAL;
>
> The pstate here comes from drm_atomic_crtc_state_for_each_plane_state,
> which does a 'for_each_if' to only execute the loop body if pstate is
> non-NULL, so I don't see how this one could be possible either.
>
>
> Do you see the driver tripping over either of these guards once this
> patch is applied? If we've got a backtrace for a gp fault, can we
> convert the RIP back into a specific line of code that triggered the
> fault?
The bug is not for a NULL pointer dereference, but a use-after-free --
so 0x6b6b6b6b6b6b6b not 0x0.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock.
2019-04-19 21:12 ` Chris Wilson
@ 2019-04-19 21:15 ` Matt Roper
0 siblings, 0 replies; 6+ messages in thread
From: Matt Roper @ 2019-04-19 21:15 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel-gfx
On Fri, Apr 19, 2019 at 10:12:02PM +0100, Chris Wilson wrote:
> Quoting Matt Roper (2019-04-19 22:06:05)
> > On Mon, Apr 15, 2019 at 08:12:50PM -0700, clinton.a.taylor@intel.com wrote:
> > > From: Clint Taylor <clinton.a.taylor@intel.com>
> > >
> > > Add protections to prevent NULL de-reference for a couple variables used
> > > in skl_check_pipe_max_pixel_clock to prevent GP exception from occurring
> > > during some IGT tests.
> > >
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=109084
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > > drivers/gpu/drm/i915/intel_pm.c | 3 +++
> > > 2 files changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 3bd40a4a6739..945861cef520 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11377,6 +11377,10 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> > >
> > > if (!ret)
> > > ret = icl_check_nv12_planes(pipe_config);
> > > +
> > > + if (WARN_ON(!intel_crtc))
> >
> > If intel_crtc is NULL, then crtc should also be NULL as well, and we
> > already dereferenced that earlier:
> >
> > struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> >
> > So if crtc/intel_crtc were the problem, I believe this check would still
> > be too late to catch and prevent a crash.
> >
> >
> > > + return -EINVAL;
> > > +
> > > if (!ret)
> > > ret = skl_check_pipe_max_pixel_rate(intel_crtc,
> > > pipe_config);
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 7357bddf9ad9..df5d01d4345b 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4160,6 +4160,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> > > uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8);
> > > int bpp;
> > >
> > > + if (WARN_ON(!pstate))
> > > + return -EINVAL;
> >
> > The pstate here comes from drm_atomic_crtc_state_for_each_plane_state,
> > which does a 'for_each_if' to only execute the loop body if pstate is
> > non-NULL, so I don't see how this one could be possible either.
> >
> >
> > Do you see the driver tripping over either of these guards once this
> > patch is applied? If we've got a backtrace for a gp fault, can we
> > convert the RIP back into a specific line of code that triggered the
> > fault?
>
> The bug is not for a NULL pointer dereference, but a use-after-free --
> so 0x6b6b6b6b6b6b6b not 0x0.
> -Chris
Okay, that makes more sense then. But in that case I don't think the
WARN_ON tests being added in this patch will help since they're only
checking for NULL.
Matt
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-19 21:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 3:12 [PATCH] drm/i915/icl: Prevent possibe de-reference in skl_check_pipe_max_pixel_clock clinton.a.taylor
2019-04-16 4:36 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-04-16 5:44 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-04-19 21:06 ` [PATCH] " Matt Roper
2019-04-19 21:12 ` Chris Wilson
2019-04-19 21:15 ` Matt Roper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.