All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer
@ 2025-06-16 14:22 Sebastian Brzezinka
  2025-06-17 10:01 ` Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Sebastian Brzezinka @ 2025-06-16 14:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: chris.p.wilson, andi.shyti, krzysztof.niemiec, krzysztof.karas

This patch adds a defensive check in `eb_relocate_entry()` to validate
the relocation entry pointer before dereferencing it. It ensures the
pointer is non-NULL and accessible from userspace using `access_ok()`.

This prevents potential kernel crashes caused by invalid or non-canonical
pointers passed from userspace.

If the pointer is invalid, an error is logged and the
function returns -EFAULT.

The failure was observed on a Tiger Lake system while running the IGT
test `igt@gem_exec_big@single`. An appropriate patch has also been
submitted to fix the issue on the IGT side.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11713

Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index ca7e9216934a..8056dea0e656 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1427,6 +1427,12 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	struct eb_vma *target;
 	int err;
 
+	/* Sanity check for non-canonical or NULL pointer */
+	if (!reloc || !access_ok(reloc, sizeof(*reloc))) {
+		DRM_ERROR("Invalid relocation entry pointer: %p\n", reloc);
+		return -EFAULT;
+	}
+
 	/* we've already hold a reference to all valid objects */
 	target = eb_get_vma(eb, reloc->target_handle);
 	if (unlikely(!target))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer
  2025-06-16 14:22 [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer Sebastian Brzezinka
@ 2025-06-17 10:01 ` Jani Nikula
  2025-06-17 11:39 ` Tvrtko Ursulin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2025-06-17 10:01 UTC (permalink / raw)
  To: Sebastian Brzezinka, intel-gfx
  Cc: chris.p.wilson, andi.shyti, krzysztof.niemiec, krzysztof.karas

On Mon, 16 Jun 2025, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
> This patch adds a defensive check in `eb_relocate_entry()` to validate
> the relocation entry pointer before dereferencing it. It ensures the
> pointer is non-NULL and accessible from userspace using `access_ok()`.
>
> This prevents potential kernel crashes caused by invalid or non-canonical
> pointers passed from userspace.
>
> If the pointer is invalid, an error is logged and the
> function returns -EFAULT.
>
> The failure was observed on a Tiger Lake system while running the IGT
> test `igt@gem_exec_big@single`. An appropriate patch has also been
> submitted to fix the issue on the IGT side.

I don't know if the patch at hand is the right thing to do (I mean I
don't know that it *isn't* either), but some comments nonetheless.

>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11713
>

Superfluous newline. Please keep the trailer lines together.

> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index ca7e9216934a..8056dea0e656 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1427,6 +1427,12 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>  	struct eb_vma *target;
>  	int err;
>  
> +	/* Sanity check for non-canonical or NULL pointer */

Is this comment helpful to the reader?

> +	if (!reloc || !access_ok(reloc, sizeof(*reloc))) {
> +		DRM_ERROR("Invalid relocation entry pointer: %p\n", reloc);

drm_err() please.

> +		return -EFAULT;
> +	}
> +
>  	/* we've already hold a reference to all valid objects */
>  	target = eb_get_vma(eb, reloc->target_handle);
>  	if (unlikely(!target))

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer
  2025-06-16 14:22 [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer Sebastian Brzezinka
  2025-06-17 10:01 ` Jani Nikula
@ 2025-06-17 11:39 ` Tvrtko Ursulin
  2025-06-18 10:15   ` Sebastian Brzezinka
  2025-06-17 12:19 ` ✗ i915.CI.BAT: failure for " Patchwork
  2025-06-17 13:11 ` [PATCH] " kernel test robot
  3 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2025-06-17 11:39 UTC (permalink / raw)
  To: Sebastian Brzezinka, intel-gfx
  Cc: chris.p.wilson, andi.shyti, krzysztof.niemiec, krzysztof.karas


On 16/06/2025 15:22, Sebastian Brzezinka wrote:
> This patch adds a defensive check in `eb_relocate_entry()` to validate
> the relocation entry pointer before dereferencing it. It ensures the
> pointer is non-NULL and accessible from userspace using `access_ok()`.
> 
> This prevents potential kernel crashes caused by invalid or non-canonical
> pointers passed from userspace.
> 
> If the pointer is invalid, an error is logged and the
> function returns -EFAULT.
> 
> The failure was observed on a Tiger Lake system while running the IGT
> test `igt@gem_exec_big@single`. An appropriate patch has also been
> submitted to fix the issue on the IGT side.
> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11713
> 
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index ca7e9216934a..8056dea0e656 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1427,6 +1427,12 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>   	struct eb_vma *target;
>   	int err;
>   
> +	/* Sanity check for non-canonical or NULL pointer */
> +	if (!reloc || !access_ok(reloc, sizeof(*reloc))) {

It doesn't look reloc is an user pointer - otherwise there wouldn't be 
simply dereferenced just below. So something looks dodgy here, you 
probably want to dig around a bit to figure out what is really going on.

Regards,

Tvrtko

> +		DRM_ERROR("Invalid relocation entry pointer: %p\n", reloc);
> +		return -EFAULT;
> +	}
> +
>   	/* we've already hold a reference to all valid objects */
>   	target = eb_get_vma(eb, reloc->target_handle);
>   	if (unlikely(!target))


^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✗ i915.CI.BAT: failure for drm/i915: Add sanity check for relocation entry pointer in execbuffer
  2025-06-16 14:22 [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer Sebastian Brzezinka
  2025-06-17 10:01 ` Jani Nikula
  2025-06-17 11:39 ` Tvrtko Ursulin
@ 2025-06-17 12:19 ` Patchwork
  2025-06-17 13:11 ` [PATCH] " kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2025-06-17 12:19 UTC (permalink / raw)
  To: Sebastian Brzezinka; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 14669 bytes --]

== Series Details ==

Series: drm/i915: Add sanity check for relocation entry pointer in execbuffer
URL   : https://patchwork.freedesktop.org/series/150330/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_16706 -> Patchwork_150330v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_150330v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_150330v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) 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_150330v1/index.html

Participating hosts (47 -> 39)
------------------------------

  Missing    (8): bat-arlh-2 fi-ilk-650 fi-snb-2520m bat-atsm-1 bat-dg2-13 fi-blb-e6850 fi-bsw-nick fi-skl-6600u 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_busy@busy:
    - fi-elk-e7500:       [PASS][1] -> [DMESG-WARN][2] +17 other tests dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-elk-e7500/igt@gem_busy@busy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-elk-e7500/igt@gem_busy@busy.html

  * igt@gem_busy@busy@all-engines:
    - fi-hsw-4770:        [PASS][3] -> [DMESG-WARN][4] +21 other tests dmesg-warn
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-hsw-4770/igt@gem_busy@busy@all-engines.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-hsw-4770/igt@gem_busy@busy@all-engines.html

  * igt@gem_close_race@basic-process:
    - bat-kbl-2:          [PASS][5] -> [DMESG-WARN][6] +43 other tests dmesg-warn
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-kbl-2/igt@gem_close_race@basic-process.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-kbl-2/igt@gem_close_race@basic-process.html

  * igt@gem_close_race@basic-threads:
    - fi-ivb-3770:        [PASS][7] -> [DMESG-WARN][8] +1 other test dmesg-warn
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-ivb-3770/igt@gem_close_race@basic-threads.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-ivb-3770/igt@gem_close_race@basic-threads.html

  * igt@gem_exec_fence@basic-await:
    - fi-glk-j4005:       [PASS][9] -> [DMESG-FAIL][10] +1 other test dmesg-fail
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-glk-j4005/igt@gem_exec_fence@basic-await.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-glk-j4005/igt@gem_exec_fence@basic-await.html
    - bat-apl-1:          [PASS][11] -> [DMESG-FAIL][12] +1 other test dmesg-fail
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-apl-1/igt@gem_exec_fence@basic-await.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-apl-1/igt@gem_exec_fence@basic-await.html
    - fi-kbl-x1275:       [PASS][13] -> [DMESG-FAIL][14] +1 other test dmesg-fail
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-kbl-x1275/igt@gem_exec_fence@basic-await.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-kbl-x1275/igt@gem_exec_fence@basic-await.html

  * igt@gem_exec_fence@basic-await@rcs0:
    - fi-cfl-guc:         [PASS][15] -> [DMESG-WARN][16] +46 other tests dmesg-warn
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-cfl-guc/igt@gem_exec_fence@basic-await@rcs0.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-cfl-guc/igt@gem_exec_fence@basic-await@rcs0.html
    - bat-kbl-2:          [PASS][17] -> [DMESG-FAIL][18] +1 other test dmesg-fail
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-kbl-2/igt@gem_exec_fence@basic-await@rcs0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-kbl-2/igt@gem_exec_fence@basic-await@rcs0.html

  * igt@gem_exec_fence@basic-busy@bcs0:
    - fi-glk-j4005:       [PASS][19] -> [DMESG-WARN][20] +43 other tests dmesg-warn
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-glk-j4005/igt@gem_exec_fence@basic-busy@bcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-glk-j4005/igt@gem_exec_fence@basic-busy@bcs0.html

  * igt@gem_exec_fence@basic-wait:
    - fi-ivb-3770:        [PASS][21] -> [DMESG-FAIL][22] +19 other tests dmesg-fail
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-ivb-3770/igt@gem_exec_fence@basic-wait.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-ivb-3770/igt@gem_exec_fence@basic-wait.html

  * igt@gem_exec_fence@basic-wait@bcs0:
    - bat-apl-1:          [PASS][23] -> [DMESG-WARN][24] +43 other tests dmesg-warn
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-apl-1/igt@gem_exec_fence@basic-wait@bcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-apl-1/igt@gem_exec_fence@basic-wait@bcs0.html
    - fi-tgl-1115g4:      [PASS][25] -> [DMESG-WARN][26] +45 other tests dmesg-warn
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-tgl-1115g4/igt@gem_exec_fence@basic-wait@bcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-tgl-1115g4/igt@gem_exec_fence@basic-wait@bcs0.html

  * igt@gem_exec_fence@basic-wait@rcs0:
    - fi-pnv-d510:        [PASS][27] -> [DMESG-WARN][28] +15 other tests dmesg-warn
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-pnv-d510/igt@gem_exec_fence@basic-wait@rcs0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-pnv-d510/igt@gem_exec_fence@basic-wait@rcs0.html

  * igt@gem_exec_fence@basic-wait@vcs0:
    - fi-bsw-n3050:       [PASS][29] -> [DMESG-WARN][30] +43 other tests dmesg-warn
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-bsw-n3050/igt@gem_exec_fence@basic-wait@vcs0.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-bsw-n3050/igt@gem_exec_fence@basic-wait@vcs0.html

  * igt@gem_exec_fence@nb-await@bcs0:
    - fi-cfl-8700k:       [PASS][31] -> [DMESG-WARN][32] +45 other tests dmesg-warn
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-cfl-8700k/igt@gem_exec_fence@nb-await@bcs0.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-cfl-8700k/igt@gem_exec_fence@nb-await@bcs0.html

  * igt@gem_exec_parallel@engines:
    - fi-ivb-3770:        [PASS][33] -> [INCOMPLETE][34] +1 other test incomplete
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-ivb-3770/igt@gem_exec_parallel@engines.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-ivb-3770/igt@gem_exec_parallel@engines.html
    - fi-elk-e7500:       [PASS][35] -> [DMESG-FAIL][36] +17 other tests dmesg-fail
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-elk-e7500/igt@gem_exec_parallel@engines.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-elk-e7500/igt@gem_exec_parallel@engines.html

  * igt@gem_exec_parallel@engines@basic:
    - fi-kbl-7567u:       [PASS][37] -> [DMESG-WARN][38] +49 other tests dmesg-warn
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-kbl-7567u/igt@gem_exec_parallel@engines@basic.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-kbl-7567u/igt@gem_exec_parallel@engines@basic.html
    - fi-kbl-8809g:       [PASS][39] -> [DMESG-WARN][40] +41 other tests dmesg-warn
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-kbl-8809g/igt@gem_exec_parallel@engines@basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-kbl-8809g/igt@gem_exec_parallel@engines@basic.html

  * igt@gem_exec_parallel@engines@userptr:
    - fi-pnv-d510:        [PASS][41] -> [DMESG-FAIL][42] +11 other tests dmesg-fail
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-pnv-d510/igt@gem_exec_parallel@engines@userptr.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-pnv-d510/igt@gem_exec_parallel@engines@userptr.html

  * igt@gem_linear_blits@basic:
    - fi-hsw-4770:        [PASS][43] -> [DMESG-FAIL][44] +23 other tests dmesg-fail
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-hsw-4770/igt@gem_linear_blits@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-hsw-4770/igt@gem_linear_blits@basic.html

  * igt@gem_tiled_blits@basic:
    - fi-kbl-guc:         [PASS][45] -> [DMESG-WARN][46] +42 other tests dmesg-warn
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-kbl-guc/igt@gem_tiled_blits@basic.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-kbl-guc/igt@gem_tiled_blits@basic.html

  * igt@gem_wait@busy@all-engines:
    - fi-kbl-x1275:       [PASS][47] -> [DMESG-WARN][48] +39 other tests dmesg-warn
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-kbl-x1275/igt@gem_wait@busy@all-engines.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-kbl-x1275/igt@gem_wait@busy@all-engines.html
    - fi-cfl-8109u:       [PASS][49] -> [DMESG-WARN][50] +47 other tests dmesg-warn
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-cfl-8109u/igt@gem_wait@busy@all-engines.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-cfl-8109u/igt@gem_wait@busy@all-engines.html

  * igt@i915_module_load@load:
    - bat-jsl-1:          [PASS][51] -> [DMESG-WARN][52] +45 other tests dmesg-warn
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-jsl-1/igt@i915_module_load@load.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-jsl-1/igt@i915_module_load@load.html

  * igt@kms_busy@basic@flip:
    - fi-elk-e7500:       [PASS][53] -> [TIMEOUT][54] +1 other test timeout
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-elk-e7500/igt@kms_busy@basic@flip.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-elk-e7500/igt@kms_busy@basic@flip.html
    - fi-hsw-4770:        [PASS][55] -> [TIMEOUT][56] +1 other test timeout
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-hsw-4770/igt@kms_busy@basic@flip.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-hsw-4770/igt@kms_busy@basic@flip.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-bsw-n3050:       [PASS][57] -> [DMESG-FAIL][58] +1 other test dmesg-fail
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
    - fi-pnv-d510:        [PASS][59] -> [TIMEOUT][60] +2 other tests timeout
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-pnv-d510/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-pnv-d510/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fence@basic-wait:
    - bat-rpls-4:         [PASS][61] -> [DMESG-WARN][62] ([i915#13400]) +1 other test dmesg-warn
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-rpls-4/igt@gem_exec_fence@basic-wait.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-rpls-4/igt@gem_exec_fence@basic-wait.html

  * igt@i915_selftest@live:
    - bat-arlh-3:         [PASS][63] -> [DMESG-FAIL][64] ([i915#14243]) +1 other test dmesg-fail
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-arlh-3/igt@i915_selftest@live.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-arlh-3/igt@i915_selftest@live.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-cfl-8109u:       [PASS][65] -> [DMESG-WARN][66] ([i915#13735]) +32 other tests dmesg-warn
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-11:         [PASS][67] -> [DMESG-FAIL][68] ([i915#12061]) +1 other test dmesg-fail
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-dg2-11/igt@i915_selftest@live@workarounds.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-dg2-11/igt@i915_selftest@live@workarounds.html

  * igt@kms_pipe_crc_basic@read-crc:
    - fi-cfl-8109u:       [PASS][69] -> [DMESG-WARN][70] ([i915#13890]) +49 other tests dmesg-warn
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-14:         [DMESG-FAIL][71] ([i915#12061]) -> [PASS][72] +1 other test pass
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16706/bat-dg2-14/igt@i915_selftest@live@workarounds.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/bat-dg2-14/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#13400]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13400
  [i915#13735]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13735
  [i915#13890]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13890
  [i915#14243]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14243


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

  * Linux: CI_DRM_16706 -> Patchwork_150330v1

  CI-20190529: 20190529
  CI_DRM_16706: dea7240e83c9e58ec755a3d68e7db10068df6b76 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8411: d5b5d2bb4f8795a98ea58376a128b74f654b7ec1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_150330v1: dea7240e83c9e58ec755a3d68e7db10068df6b76 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150330v1/index.html

[-- Attachment #2: Type: text/html, Size: 16114 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer
  2025-06-16 14:22 [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer Sebastian Brzezinka
                   ` (2 preceding siblings ...)
  2025-06-17 12:19 ` ✗ i915.CI.BAT: failure for " Patchwork
@ 2025-06-17 13:11 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-06-17 13:11 UTC (permalink / raw)
  To: Sebastian Brzezinka, intel-gfx
  Cc: oe-kbuild-all, chris.p.wilson, andi.shyti, krzysztof.niemiec,
	krzysztof.karas

Hi Sebastian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on linus/master v6.16-rc2 next-20250617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sebastian-Brzezinka/drm-i915-Add-sanity-check-for-relocation-entry-pointer-in-execbuffer/20250616-222313
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:    https://lore.kernel.org/r/lofb2i4actwlvfk6xbtihirrc34j3pb6xecvcl433a2xbm7zy6%40akz3ko2bh2i5
patch subject: [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer
config: i386-randconfig-062-20250617 (https://download.01.org/0day-ci/archive/20250617/202506172030.rBM8TgS8-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250617/202506172030.rBM8TgS8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506172030.rBM8TgS8-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1431:24: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const [noderef] __user *ptr @@     got struct drm_i915_gem_relocation_entry const *reloc @@
   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1431:24: sparse:     expected void const [noderef] __user *ptr
   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1431:24: sparse:     got struct drm_i915_gem_relocation_entry const *reloc

vim +1431 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

  1420	
  1421	static u64
  1422	eb_relocate_entry(struct i915_execbuffer *eb,
  1423			  struct eb_vma *ev,
  1424			  const struct drm_i915_gem_relocation_entry *reloc)
  1425	{
  1426		struct drm_i915_private *i915 = eb->i915;
  1427		struct eb_vma *target;
  1428		int err;
  1429	
  1430		/* Sanity check for non-canonical or NULL pointer */
> 1431		if (!reloc || !access_ok(reloc, sizeof(*reloc))) {
  1432			DRM_ERROR("Invalid relocation entry pointer: %p\n", reloc);
  1433			return -EFAULT;
  1434		}
  1435	
  1436		/* we've already hold a reference to all valid objects */
  1437		target = eb_get_vma(eb, reloc->target_handle);
  1438		if (unlikely(!target))
  1439			return -ENOENT;
  1440	
  1441		/* Validate that the target is in a valid r/w GPU domain */
  1442		if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) {
  1443			drm_dbg(&i915->drm, "reloc with multiple write domains: "
  1444				  "target %d offset %d "
  1445				  "read %08x write %08x\n",
  1446				  reloc->target_handle,
  1447				  (int) reloc->offset,
  1448				  reloc->read_domains,
  1449				  reloc->write_domain);
  1450			return -EINVAL;
  1451		}
  1452		if (unlikely((reloc->write_domain | reloc->read_domains)
  1453			     & ~I915_GEM_GPU_DOMAINS)) {
  1454			drm_dbg(&i915->drm, "reloc with read/write non-GPU domains: "
  1455				  "target %d offset %d "
  1456				  "read %08x write %08x\n",
  1457				  reloc->target_handle,
  1458				  (int) reloc->offset,
  1459				  reloc->read_domains,
  1460				  reloc->write_domain);
  1461			return -EINVAL;
  1462		}
  1463	
  1464		if (reloc->write_domain) {
  1465			target->flags |= EXEC_OBJECT_WRITE;
  1466	
  1467			/*
  1468			 * Sandybridge PPGTT errata: We need a global gtt mapping
  1469			 * for MI and pipe_control writes because the gpu doesn't
  1470			 * properly redirect them through the ppgtt for non_secure
  1471			 * batchbuffers.
  1472			 */
  1473			if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
  1474			    GRAPHICS_VER(eb->i915) == 6 &&
  1475			    !i915_vma_is_bound(target->vma, I915_VMA_GLOBAL_BIND)) {
  1476				struct i915_vma *vma = target->vma;
  1477	
  1478				reloc_cache_unmap(&eb->reloc_cache);
  1479				mutex_lock(&vma->vm->mutex);
  1480				err = i915_vma_bind(target->vma,
  1481						    target->vma->obj->pat_index,
  1482						    PIN_GLOBAL, NULL, NULL);
  1483				mutex_unlock(&vma->vm->mutex);
  1484				reloc_cache_remap(&eb->reloc_cache, ev->vma->obj);
  1485				if (err)
  1486					return err;
  1487			}
  1488		}
  1489	
  1490		/*
  1491		 * If the relocation already has the right value in it, no
  1492		 * more work needs to be done.
  1493		 */
  1494		if (!DBG_FORCE_RELOC &&
  1495		    gen8_canonical_addr(i915_vma_offset(target->vma)) == reloc->presumed_offset)
  1496			return 0;
  1497	
  1498		/* Check that the relocation address is valid... */
  1499		if (unlikely(reloc->offset >
  1500			     ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
  1501			drm_dbg(&i915->drm, "Relocation beyond object bounds: "
  1502				  "target %d offset %d size %d.\n",
  1503				  reloc->target_handle,
  1504				  (int)reloc->offset,
  1505				  (int)ev->vma->size);
  1506			return -EINVAL;
  1507		}
  1508		if (unlikely(reloc->offset & 3)) {
  1509			drm_dbg(&i915->drm, "Relocation not 4-byte aligned: "
  1510				  "target %d offset %d.\n",
  1511				  reloc->target_handle,
  1512				  (int)reloc->offset);
  1513			return -EINVAL;
  1514		}
  1515	
  1516		/*
  1517		 * If we write into the object, we need to force the synchronisation
  1518		 * barrier, either with an asynchronous clflush or if we executed the
  1519		 * patching using the GPU (though that should be serialised by the
  1520		 * timeline). To be completely sure, and since we are required to
  1521		 * do relocations we are already stalling, disable the user's opt
  1522		 * out of our synchronisation.
  1523		 */
  1524		ev->flags &= ~EXEC_OBJECT_ASYNC;
  1525	
  1526		/* and update the user's relocation entry */
  1527		return relocate_entry(ev->vma, reloc, eb, target->vma);
  1528	}
  1529	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer
  2025-06-17 11:39 ` Tvrtko Ursulin
@ 2025-06-18 10:15   ` Sebastian Brzezinka
  0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Brzezinka @ 2025-06-18 10:15 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx
  Cc: chris.p.wilson, andi.shyti, krzysztof.niemiec, krzysztof.karas

Hi Tvrtko,

On Tue Jun 17, 2025 at 11:39 AM UTC, Tvrtko Ursulin wrote:
>
> On 16/06/2025 15:22, Sebastian Brzezinka wrote:
>> This patch adds a defensive check in `eb_relocate_entry()` to validate
>> the relocation entry pointer before dereferencing it. It ensures the
>> pointer is non-NULL and accessible from userspace using `access_ok()`.
>> 
>> This prevents potential kernel crashes caused by invalid or non-canonical
>> pointers passed from userspace.
>> 
>> If the pointer is invalid, an error is logged and the
>> function returns -EFAULT.
>> 
>> The failure was observed on a Tiger Lake system while running the IGT
>> test `igt@gem_exec_big@single`. An appropriate patch has also been
>> submitted to fix the issue on the IGT side.
>> 
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11713
>> 
>> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> index ca7e9216934a..8056dea0e656 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> @@ -1427,6 +1427,12 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>>   	struct eb_vma *target;
>>   	int err;
>>   
>> +	/* Sanity check for non-canonical or NULL pointer */
>> +	if (!reloc || !access_ok(reloc, sizeof(*reloc))) {
>
> It doesn't look reloc is an user pointer - otherwise there wouldn't be 
> simply dereferenced just below. So something looks dodgy here, you 
> probably want to dig around a bit to figure out what is really going on.

Yes, you're right, it's indeed possible to pass both kernel and userspace
pointers. I overlooked that initially. I've corrected the issue on the
IGT side, so non-canonical pointers are no longer being sent.
Additionally, I attempted to improve the handling here, though 
it still needs more work to be fully robust. Thanks for the review!

-- 
Best regards,
Sebastian


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-06-18 10:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-16 14:22 [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer Sebastian Brzezinka
2025-06-17 10:01 ` Jani Nikula
2025-06-17 11:39 ` Tvrtko Ursulin
2025-06-18 10:15   ` Sebastian Brzezinka
2025-06-17 12:19 ` ✗ i915.CI.BAT: failure for " Patchwork
2025-06-17 13:11 ` [PATCH] " kernel test robot

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.