[PATCH 0/2] i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak)

[PATCH 0/2] i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak)

[Chengyu Liu]

Two independent small fixes found by static analysis:

1/2: gen6_ppgtt_create() stores an ERR_PTR in ppgtt->base.pd when
     gen6_alloc_top_pd() fails; the teardown path
     (gen6_ppgtt_cleanup -> gen6_ppgtt_free_pd) dereferences it and
     crashes in the async release worker.  Guard both teardown sites
     with IS_ERR_OR_NULL() so an allocation failure under memory
     pressure stays a plain -ENOMEM.

2/2: i915_perf_add_config_ioctl() creates the dynamic sysfs entry
     before idr_alloc(); if idr_alloc() fails the error path frees the
     config without removing the sysfs entry, leaving attributes that
     point into freed memory (UAF read via the embedded show()
     callback).  Remove the group on that path, mirroring the remove
     ioctl's ordering.

Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---

Chengyu Liu (2):
  i915/gt: tolerate ERR_PTR pd in gen6_ppgtt teardown
  i915/perf: remove sysfs entry if idr_alloc() fails in add_config

 drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 5 ++++-
 drivers/gpu/drm/i915/i915_perf.c     | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/2] i915/gt: tolerate ERR_PTR pd in gen6_ppgtt teardown

[Chengyu Liu]

gen6_ppgtt_create() stores the result of gen6_alloc_top_pd() into
ppgtt->base.pd and, on failure, jumps to err_put with the ERR_PTR still
in the field.  i915_vm_put() runs vm->cleanup = gen6_ppgtt_cleanup(),
which calls gen6_ppgtt_free_pd(), where gen6_for_all_pdes() immediately
dereferences pd - with an ERR_PTR that is an invalid dereference and a
crash (in the async release worker).  The free_pd() right after is only
guarded against NULL, not ERR_PTR.

Bail out of gen6_ppgtt_free_pd() for an invalid pd and skip free_pd()
likewise, so an allocation failure under memory pressure stays a plain
-ENOMEM instead of an oops.

Found by static analysis.

Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 438cd4724a..72a9ad4642 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -259,6 +259,9 @@ static void gen6_ppgtt_free_pd(struct gen6_ppgtt *ppgtt)
 	struct i915_page_table *pt;
 	u32 pde;
 
+	if (IS_ERR_OR_NULL(pd))
+		return;
+
 	gen6_for_all_pdes(pt, pd, pde)
 		if (pt)
 			free_pt(&ppgtt->base.vm, pt);
@@ -271,7 +274,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 	gen6_ppgtt_free_pd(ppgtt);
 	free_scratch(vm);
 
-	if (ppgtt->base.pd)
+	if (!IS_ERR_OR_NULL(ppgtt->base.pd))
 		free_pd(&ppgtt->base.vm, ppgtt->base.pd);
 
 	mutex_destroy(&ppgtt->flush);
-- 
2.25.1

[PATCH 2/2] i915/perf: remove sysfs entry if idr_alloc() fails in add_config

[Chengyu Liu]

i915_perf_add_config_ioctl() creates the dynamic sysfs entry for the OA
config before allocating its id.  If idr_alloc() fails, the error path
drops the config reference via reg_err: without removing the sysfs
entry - unlike the normal remove path, which does sysfs_remove_group()
first.  The leftover attributes point into the freed oa_config; kernfs
only stores the attribute pointer, and show_dynamic_id() reaches the
freed object via container_of (including the show() callback itself),
so reading /sys/class/drm/cardX/metrics/<uuid>/id afterwards is a
use-after-free.

Remove the sysfs group on the idr_alloc() failure path, mirroring the
remove ioctl's ordering (under metrics_lock).

Found by static analysis.

Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 19b82427aa..40385f991a 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -4716,6 +4716,7 @@ int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
 		drm_dbg(&perf->i915->drm,
 			"Failed to create sysfs entry for OA config\n");
 		err = oa_config->id;
+		sysfs_remove_group(perf->metrics_kobj, &oa_config->sysfs_metric);
 		goto sysfs_err;
 	}
 	id = oa_config->id;
-- 
2.25.1

Re: [PATCH 1/2] i915/gt: tolerate ERR_PTR pd in gen6_ppgtt teardown

[Ville Syrjälä]

On Wed, Nov 04, 2026 at 03:33:10PM +0800, Chengyu Liu wrote:
> gen6_ppgtt_create() stores the result of gen6_alloc_top_pd() into
> ppgtt->base.pd and, on failure, jumps to err_put with the ERR_PTR still
> in the field.  i915_vm_put() runs vm->cleanup = gen6_ppgtt_cleanup(),
> which calls gen6_ppgtt_free_pd(), where gen6_for_all_pdes() immediately
> dereferences pd - with an ERR_PTR that is an invalid dereference and a
> crash (in the async release worker).  The free_pd() right after is only
> guarded against NULL, not ERR_PTR.
> 
> Bail out of gen6_ppgtt_free_pd() for an invalid pd and skip free_pd()
> likewise, so an allocation failure under memory pressure stays a plain
> -ENOMEM instead of an oops.

The correct fix would be to not assign the error pointer
into ppgtt->base.pd in the first place.

Looks like a somwhat common bad pattern in this code that
received pointers are assigned directly into their final
location, and if the pointer is an error then the pointer is
cleared. IMO one should never do that because it's very easy
to forget the clearing part. Instead one should always use 
local variable first, and only assign the final pointer after
having checked that the pointer is valid.

> 
> Found by static analysis.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
> ---
>  drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> index 438cd4724a..72a9ad4642 100644
> --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> @@ -259,6 +259,9 @@ static void gen6_ppgtt_free_pd(struct gen6_ppgtt *ppgtt)
>  	struct i915_page_table *pt;
>  	u32 pde;
>  
> +	if (IS_ERR_OR_NULL(pd))
> +		return;
> +
>  	gen6_for_all_pdes(pt, pd, pde)
>  		if (pt)
>  			free_pt(&ppgtt->base.vm, pt);
> @@ -271,7 +274,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>  	gen6_ppgtt_free_pd(ppgtt);
>  	free_scratch(vm);
>  
> -	if (ppgtt->base.pd)
> +	if (!IS_ERR_OR_NULL(ppgtt->base.pd))
>  		free_pd(&ppgtt->base.vm, ppgtt->base.pd);
>  
>  	mutex_destroy(&ppgtt->flush);
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

✗ LGCI.VerificationFailed: failure for i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak)

[Patchwork]

== Series Details ==

Series: i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak)
URL   : https://patchwork.freedesktop.org/series/177748/
State : failure

== Summary ==

Series author address 'lui2012qwq@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/192083/ (on built-in)

✓ i915.CI.BAT: success for i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak) (rev2)

[Patchwork]

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

== Series Details ==

Series: i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak) (rev2)
URL   : https://patchwork.freedesktop.org/series/177748/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_19448 -> Patchwork_177748v2
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (40 -> 39)
------------------------------

  Missing    (1): bat-dg2-13 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-rpls-4:         NOTRUN -> [SKIP][1] ([i915#4613]) +3 other tests skip
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/bat-rpls-4/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_lmem_swapping@random-engines:
    - fi-skl-6600u:       NOTRUN -> [SKIP][2] ([i915#4613]) +3 other tests skip
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/fi-skl-6600u/igt@gem_lmem_swapping@random-engines.html

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

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-skl-6600u:       NOTRUN -> [SKIP][5] ([i915#17465])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/fi-skl-6600u/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-b-edp-1:
    - fi-skl-6600u:       NOTRUN -> [SKIP][6] ([i915#17461]) +3 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-b-edp-1.html

  * igt@kms_pipe_crc_basic@read-crc:
    - fi-cfl-8109u:       [PASS][7] -> [DMESG-WARN][8] ([i915#13735] / [i915#15673]) +49 other tests dmesg-warn
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19448/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc.html

  * igt@kms_psr@psr-primary-page-flip:
    - bat-rpls-4:         NOTRUN -> [SKIP][9] ([i915#1072] / [i915#9732]) +3 other tests skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/bat-rpls-4/igt@kms_psr@psr-primary-page-flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rpls-4:         NOTRUN -> [SKIP][10] ([i915#3555])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/bat-rpls-4/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-skl-6600u:       NOTRUN -> [SKIP][11] ([i915#17463])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/fi-skl-6600u/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - bat-rpls-4:         NOTRUN -> [SKIP][12] ([i915#17448] / [i915#3708]) +2 other tests skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/bat-rpls-4/igt@prime_vgem@basic-read.html

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-skl-6600u:       [INCOMPLETE][13] -> [PASS][14] +1 other test pass
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19448/fi-skl-6600u/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/fi-skl-6600u/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  * igt@kms_pm_rpm@basic-pci-d3-state:
    - bat-rpls-4:         [INCOMPLETE][15] -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19448/bat-rpls-4/igt@kms_pm_rpm@basic-pci-d3-state.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177748v2/bat-rpls-4/igt@kms_pm_rpm@basic-pci-d3-state.html

  
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#13735]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13735
  [i915#15673]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15673
  [i915#17448]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17448
  [i915#17461]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17461
  [i915#17463]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17463
  [i915#17465]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17465
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
  [i915#4613]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4613
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732


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

  * Linux: CI_DRM_19448 -> Patchwork_177748v2

  CI-20190529: 20190529
  CI_DRM_19448: 3379b0f74f31bfc3746b692822a7700aeeebe15d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9177: 9177
  Patchwork_177748v2: 3379b0f74f31bfc3746b692822a7700aeeebe15d @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

Re: [PATCH 1/2] i915/gt: tolerate ERR_PTR pd in gen6_ppgtt teardown

[Chengyu Liu]

On Wed, Nov 04, 2026, Ville Syrjala wrote:
> The correct fix would be to not assign the error pointer into
> ppgtt->base.pd in the first place. [...] one should always use a
> local variable first, and only assign the final pointer after having
> checked that the pointer is valid.

Agreed, that's cleaner.  v2 does exactly that: gen6_alloc_top_pd()'s
result goes into a local first and ppgtt->base.pd is only assigned
after validation, so the field can never hold an error pointer.  On
the failure path it stays NULL, and gen6_ppgtt_free_pd() now bails out
on a NULL pd (the cleanup path is reached with nothing assigned).

(gen6_alloc_top_pd() itself uses the assign-then-clear pattern for
ppgtt->vma you described; this patch doesn't touch that site.)

Chengyu

On Wed, Nov 4, 2026 at 11:18 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Nov 04, 2026 at 03:33:10PM +0800, Chengyu Liu wrote:
> > gen6_ppgtt_create() stores the result of gen6_alloc_top_pd() into
> > ppgtt->base.pd and, on failure, jumps to err_put with the ERR_PTR still
> > in the field.  i915_vm_put() runs vm->cleanup = gen6_ppgtt_cleanup(),
> > which calls gen6_ppgtt_free_pd(), where gen6_for_all_pdes() immediately
> > dereferences pd - with an ERR_PTR that is an invalid dereference and a
> > crash (in the async release worker).  The free_pd() right after is only
> > guarded against NULL, not ERR_PTR.
> >
> > Bail out of gen6_ppgtt_free_pd() for an invalid pd and skip free_pd()
> > likewise, so an allocation failure under memory pressure stays a plain
> > -ENOMEM instead of an oops.
>
> The correct fix would be to not assign the error pointer
> into ppgtt->base.pd in the first place.
>
> Looks like a somwhat common bad pattern in this code that
> received pointers are assigned directly into their final
> location, and if the pointer is an error then the pointer is
> cleared. IMO one should never do that because it's very easy
> to forget the clearing part. Instead one should always use
> local variable first, and only assign the final pointer after
> having checked that the pointer is valid.
>
> >
> > Found by static analysis.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> > index 438cd4724a..72a9ad4642 100644
> > --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> > +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> > @@ -259,6 +259,9 @@ static void gen6_ppgtt_free_pd(struct gen6_ppgtt *ppgtt)
> >       struct i915_page_table *pt;
> >       u32 pde;
> >
> > +     if (IS_ERR_OR_NULL(pd))
> > +             return;
> > +
> >       gen6_for_all_pdes(pt, pd, pde)
> >               if (pt)
> >                       free_pt(&ppgtt->base.vm, pt);
> > @@ -271,7 +274,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >       gen6_ppgtt_free_pd(ppgtt);
> >       free_scratch(vm);
> >
> > -     if (ppgtt->base.pd)
> > +     if (!IS_ERR_OR_NULL(ppgtt->base.pd))
> >               free_pd(&ppgtt->base.vm, ppgtt->base.pd);
> >
> >       mutex_destroy(&ppgtt->flush);
> > --
> > 2.25.1
>
> --
> Ville Syrjälä
> Intel

✓ i915.CI.Full: success for i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak) (rev2)

[Patchwork]

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

== Series Details ==

Series: i915: two teardown fixes (gen6 ppgtt ERR_PTR, OA config sysfs leak) (rev2)
URL   : https://patchwork.freedesktop.org/series/177748/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_19448_full -> Patchwork_177748v2_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (1 -> 1)
------------------------------

  No changes in participating hosts


Changes
-------

  No changes found


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

  * Linux: CI_DRM_19448 -> Patchwork_177748v2

  CI_DRM_19448: 3379b0f74f31bfc3746b692822a7700aeeebe15d @ git://anongit.freedesktop.org/gfx-ci/linux
  Patchwork_177748v2: 3379b0f74f31bfc3746b692822a7700aeeebe15d @ 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_177748v2/index.html

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