[PATCH v5 0/6] Series from memory leak on qxl unbind

[PATCH v5 0/6] Series from memory leak on qxl unbind

[Óscar Megía López]

This is a series of bugs that leak memory on qxl unbind.

For test you must apply first ("[PATCH v3] drm/qxl: fix use-after-free
in qxl_irq_handler on PCI") because if not, you will get
"---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---".

Link: https://lore.kernel.org/lkml/
20260727110212.64913-1-megia.oscar@gmail.com/

Óscar Megía López (6):
  Memory leak error in qxl unbind
  list_lru_init() does not check return value
  ttm_pool_fini() does not destroy list lru
  ttm_pool_type_init() does not check return value
  ttm_pool_mgr_fini() does not destroy the list_lru
  ttm_pool_mgr_init() does not check return value

 drivers/gpu/drm/ttm/ttm_device.c |   4 +-
 drivers/gpu/drm/ttm/ttm_pool.c   | 194 +++++++++++++++++++++++++------
 include/drm/ttm/ttm_pool.h       |   2 +-
 3 files changed, 161 insertions(+), 39 deletions(-)

-- 
2.55.0

[PATCH v5 1/6] Memory leak error in qxl unbind

[Óscar Megía López]

I discovered an OOM after run the script below
(I updated it and added a sleep to allow enough time for the cache to
recover):

while [ 1 -eq 1 ]; do\
    i=$((i+1)); echo 0000:00:01.0 > /sys/bus/pci/drivers/qxl/unbind;\
    if (($i%1000==0)); then\
        echo i=$i; free;\
        grep nr_free_pages /proc/vmstat;\
        grep -E "PageTables|VmallocUsed|Slab|Reclaimable" /proc/meminfo;\
        sync; echo 3 > /proc/sys/vm/drop_caches;\
        echo 1 > /proc/sys/vm/compact_memory;\
        sleep 10s;\
        free;\
        grep nr_free_pages /proc/vmstat;\
        grep -E "PageTables|VmallocUsed|Slab|Reclaimable" /proc/meminfo;\
        uptime;\
    fi;\
    echo 0000:00:01.0 > /sys/bus/pci/drivers/qxl/bind;\
done

The OOM isn't just a simple leak; it's a refcount corruption which renders
the list_lru fix dead code after the first mid-init failure.

Fixed check if shrinker_list is empty holding shrinker_lock.
Fixed check return value from ttm_pool_type_init and run
ttm_pool_type_fini and list_lru_destroy for every pt initialized.
Fixed change return value from ttm_pool_init to int.

This patch depends on patch ("[PATCH v3] drm/qxl: fix use-after-free in
qxl_irq_handler on PCI"), link [1] below.

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Assisted-by: claude.ai:Sonnet 5
Link: https://lore.kernel.org/lkml/
20260727110212.64913-1-megia.oscar@gmail.com/ [1]
Link: https://lore.kernel.org/dri-devel/
20260731053047.24503-1-megia.oscar@gmail.com/ [2]
Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
Changes in v2:
 - Bug 1: ttm_global_init ignores ttm_pool_mgr_init() return.
   If shrinker_alloc() fails under memory pressure, ttm_pool_mgr_init
   returns -ENOMEM with pool types already initialized (64 list_lru_init
   calls done). ttm_global_init ignored this and returned 0, leaving orphaned
   pool types with a NULL mm_shrinker.

   Fix: Check ret from ttm_pool_mgr_init; if non-zero, goto out cleans up
   refcount + debugfs.

 - Bug 2: ttm_pool_mgr_init leaks pool types on shrinker_alloc failure
   If shrinker_alloc fails after all 64 pool types were list_lru_init'd,
   the function returned -ENOMEM without undoing them. With Bug 1 now
   triggering proper error handling, this undo is necessary.

   Fix: err_shrinker: label that finalizes + destroys all 64 pool types
   before returning.

Changes in v3:
 - Fix: "Unchecked list_lru_init() return value in ttm_pool_type_init()
   causes a deterministic NULL pointer dereference in the newly added
   error path."
   Now check list_lru_init return value in ttm_pool_type_init() and
   returns error if any.

 - Solved pre-existing issues reported by kernel test robot:
   - [High] `ttm_pool_type_init()` ignores the return value of
     `list_lru_init()`, leading to a NULL pointer dereference
     if allocation fails.

     Fix: get return value from list_lru_init and return error if any.

   - [High] `ttm_pool_shrink()` assumes `shrinker_list` is never empty,
     causing memory corruption and crashes during module unload
     if triggered.

     Fix: Check if shrinker_list is empty and return 0 if it is empty.

Changes in v4:
 - removed check return value in ttm_pool_mgr_init, now in new patch
 ("[PATCH] ttm: Add error handling for ttm_pool_mgr_init()")
 link [2] above.
 - Fixed check empty shrinker_list.
 - Check return value from ttm_pool_type_init.
 - Move up shrinker_alloc.
 - Deleted dput(backup_fault_inject.dname);
 - Fixed issue [High] The patch introduces a use-after-free race condition
   between `ttm_pool_type_fini()` and the active memory shrinker
   `ttm_pool_shrink()` by calling `list_lru_destroy()` prematurely as
   reported by kernel test robot.

   Fix: separate ttm_pool_type_fini and list_lru_destroy. Then, add
   ttm_pool_synchronize_shrinkers between them.
---
 drivers/gpu/drm/ttm/ttm_pool.c | 62 ++++++++++++++++++++++++----------
 include/drm/ttm/ttm_pool.h     |  2 +-
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 278bbe7a11ad..88c0d33eed1a 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -437,13 +437,21 @@ static unsigned int ttm_pool_shrink(int nid, unsigned long num_to_free)
 	LIST_HEAD(dispose);
 	struct ttm_pool_type *pt;
 	unsigned int num_pages;
+	int empty = 0;
 
 	down_read(&pool_shrink_rwsem);
 	spin_lock(&shrinker_lock);
-	pt = list_first_entry(&shrinker_list, typeof(*pt), shrinker_list);
-	list_move_tail(&pt->shrinker_list, &shrinker_list);
+	if ((shrinker_list.prev == &shrinker_list) && (shrinker_list.next == &shrinker_list)) {
+		empty = 1;
+	} else {
+		pt = list_first_entry(&shrinker_list, typeof(*pt), shrinker_list);
+		list_move_tail(&pt->shrinker_list, &shrinker_list);
+	}
 	spin_unlock(&shrinker_lock);
 
+	if (empty)
+		return 0;
+
 	num_pages = list_lru_walk_node(&pt->pages, nid, pool_move_to_dispose_list, &dispose, &num_to_free);
 	num_pages *= 1 << pt->order;
 
@@ -1122,6 +1130,18 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
 	return shrunken ? shrunken : ret;
 }
 
+/**
+ * ttm_pool_synchronize_shrinkers - Wait for all running shrinkers to complete.
+ *
+ * This is useful to guarantee that all shrinker invocations have seen an
+ * update, before freeing memory, similar to rcu.
+ */
+static void ttm_pool_synchronize_shrinkers(void)
+{
+	down_write(&pool_shrink_rwsem);
+	up_write(&pool_shrink_rwsem);
+}
+
 /**
  * ttm_pool_init - Initialize a pool
  *
@@ -1132,10 +1152,13 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
  *
  * Initialize the pool and its pool types.
  */
-void ttm_pool_init(struct ttm_pool *pool, struct device *dev,
+int ttm_pool_init(struct ttm_pool *pool, struct device *dev,
 		   int nid, unsigned int alloc_flags)
 {
-	unsigned int i, j;
+	unsigned int i, j, k;
+	int ret;
+	struct ttm_pool_type *initialized[TTM_NUM_CACHING_TYPES * NR_PAGE_ORDERS];
+	unsigned int n_initialized = 0;
 
 	WARN_ON(!dev && ttm_pool_uses_dma_alloc(pool));
 
@@ -1152,23 +1175,28 @@ void ttm_pool_init(struct ttm_pool *pool, struct device *dev,
 			if (pt != &pool->caching[i].orders[j])
 				continue;
 
-			ttm_pool_type_init(pt, pool, i, j);
+			ret = ttm_pool_type_init(pt, pool, i, j);
+			if (ret)
+				goto error;
+
+			initialized[n_initialized++] = pt;
 		}
 	}
-}
-EXPORT_SYMBOL(ttm_pool_init);
 
-/**
- * ttm_pool_synchronize_shrinkers - Wait for all running shrinkers to complete.
- *
- * This is useful to guarantee that all shrinker invocations have seen an
- * update, before freeing memory, similar to rcu.
- */
-static void ttm_pool_synchronize_shrinkers(void)
-{
-	down_write(&pool_shrink_rwsem);
-	up_write(&pool_shrink_rwsem);
+	return 0;
+
+error:
+	for (k = 0; k < n_initialized; ++k)
+		ttm_pool_type_fini(initialized[k]);
+
+	ttm_pool_synchronize_shrinkers();
+
+	for (k = 0; k < n_initialized; ++k)
+		list_lru_destroy(&initialized[k]->pages);
+
+	return ret;
 }
+EXPORT_SYMBOL(ttm_pool_init);
 
 /**
  * ttm_pool_fini - Cleanup a pool
diff --git a/include/drm/ttm/ttm_pool.h b/include/drm/ttm/ttm_pool.h
index 26ee592e1994..66248323c2c1 100644
--- a/include/drm/ttm/ttm_pool.h
+++ b/include/drm/ttm/ttm_pool.h
@@ -81,7 +81,7 @@ int ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
 		   struct ttm_operation_ctx *ctx);
 void ttm_pool_free(struct ttm_pool *pool, struct ttm_tt *tt);
 
-void ttm_pool_init(struct ttm_pool *pool, struct device *dev,
+int ttm_pool_init(struct ttm_pool *pool, struct device *dev,
 		   int nid, unsigned int alloc_flags);
 void ttm_pool_fini(struct ttm_pool *pool);
 
-- 
2.55.0

Re: [PATCH v5 1/6] Memory leak error in qxl unbind

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Missing `up_read(&pool_shrink_rwsem)` on the early return path in `ttm_pool_shrink()` causes a lock leak and subsequent deadlocks.
- [High] Callers of `ttm_pool_init()` are not updated to check its new `int` return value, leading to NULL pointer dereferences on initialization failure.
- [Low] Assigning the `void` return value of `ttm_pool_type_init()` to an `int` variable causes a compilation failure.
- [Low] Reinventing `list_empty()` with direct manual pointer comparisons.

Pre-existing issues:
- [High] Missing `list_lru_destroy()` in normal pool teardown paths (`ttm_pool_fini()` and `ttm_pool_mgr_fini()`) causes persistent memory leaks.
--

--- Patch [1]: [PATCH v5 1/6] Memory leak error in qxl unbind ---
commit 575a0762d1411d04c459167fe4dee31c7eee4aff
Author: Óscar Megía López <megia.oscar@gmail.com>

Memory leak error in qxl unbind

This commit attempts to fix a refcount corruption and memory leak in qxl
unbind by changing ttm_pool_init to return an error, checking for an empty
shrinker_list, and cleaning up list_lru structures on init failure.

Link: https://lore.kernel.org/lkml/20260727110212.64913-1-megia.oscar@gmail.com/
Link: https://lore.kernel.org/dri-devel/20260731053047.24503-1-megia.oscar@gmail.com/

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 1bf37023fed66..73284497cc785 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -457,13 +457,21 @@ static unsigned int ttm_pool_shrink(int nid, unsigned long num_to_free)
>  	LIST_HEAD(dispose);
>  	struct ttm_pool_type *pt;
>  	unsigned int num_pages;
> +	int empty = 0;
>  
>  	down_read(&pool_shrink_rwsem);
>  	spin_lock(&shrinker_lock);
> -	pt = list_first_entry(&shrinker_list, typeof(*pt), shrinker_list);
> -	list_move_tail(&pt->shrinker_list, &shrinker_list);
> +	if ((shrinker_list.prev == &shrinker_list) && (shrinker_list.next == &shrinker_list)) {

[Severity: Low]
Could we use list_empty(&shrinker_list) here instead of manually checking
the prev and next pointers?

> +		empty = 1;
> +	} else {
> +		pt = list_first_entry(&shrinker_list, typeof(*pt), shrinker_list);
> +		list_move_tail(&pt->shrinker_list, &shrinker_list);
> +	}
>  	spin_unlock(&shrinker_lock);
>  
> +	if (empty)
> +		return 0;

[Severity: High]
Does this early return leak the pool_shrink_rwsem read lock?

The down_read(&pool_shrink_rwsem) is acquired at the beginning of the
function, but if the shrinker_list is empty, this path returns early
without calling up_read().

This can cause any subsequent attempt to acquire a write lock (such as in
ttm_pool_synchronize_shrinkers during module unload) to block indefinitely
and deadlock the system.

[ ... ]

> @@ -1231,10 +1251,13 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
>   *
>   * Initialize the pool and its pool types.
>   */
> -void ttm_pool_init(struct ttm_pool *pool, struct device *dev,
> +int ttm_pool_init(struct ttm_pool *pool, struct device *dev,

[Severity: High]
Are the callers of ttm_pool_init() updated to check this new int return
value?

For example, ttm_device_init() calls ttm_pool_init() but ignores the return
value. If ttm_pool_init() fails under memory pressure and cleans up the
lists, proceeding with driver initialization can cause a NULL pointer
dereference when subsequent allocations attempt to use the destroyed LRU
lists.

>  		   int nid, unsigned int alloc_flags)
>  {
> -	unsigned int i, j;
> +	unsigned int i, j, k;
> +	int ret;
> +	struct ttm_pool_type *initialized[TTM_NUM_CACHING_TYPES * NR_PAGE_ORDERS];
> +	unsigned int n_initialized = 0;

[ ... ]

> @@ -1251,23 +1274,28 @@ void ttm_pool_init(struct ttm_pool *pool, struct device *dev,
>  			if (pt != &pool->caching[i].orders[j])
>  				continue;
>  
> -			ttm_pool_type_init(pt, pool, i, j);
> +			ret = ttm_pool_type_init(pt, pool, i, j);

[Severity: Low]
Does this assignment cause a build failure?

It appears ttm_pool_type_init() is still defined as returning void, so
assigning its result to the int ret variable will break the build.

> +			if (ret)
> +				goto error;
> +
> +			initialized[n_initialized++] = pt;
>  		}
>  	}
>  
> +	return 0;
> +
> +error:
> +	for (k = 0; k < n_initialized; ++k)
> +		ttm_pool_type_fini(initialized[k]);

[Severity: High]
This is a pre-existing issue, but does ttm_pool_type_fini() fail to destroy
the list_lru structures?

While ttm_pool_type_fini() drains the LRU via list_lru_walk(), it doesn't
call list_lru_destroy(&pt->pages). This causes a persistent memory leak of
the node arrays on every device unbind or module unload where the normal
pool teardown paths (like ttm_pool_fini and ttm_pool_mgr_fini) are used.

> +
> +	ttm_pool_synchronize_shrinkers();
> +
> +	for (k = 0; k < n_initialized; ++k)
> +		list_lru_destroy(&initialized[k]->pages);
> +
> +	return ret;
>  }
> +EXPORT_SYMBOL(ttm_pool_init);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811194224.121597-1-megia.oscar@gmail.com?part=1

[PATCH v5 2/6] list_lru_init() does not check return value

[Óscar Megía López]

Bug: list_lru_init() does not check return value in
ttm_pool_type_init().

Fix: Check the return value from list_lru_init() and propagate the error.

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Assisted-by: claude.ai:Sonnet 5
Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_pool.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 88c0d33eed1a..8637f7942347 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -354,17 +354,23 @@ static struct page *ttm_pool_type_take(struct ttm_pool_type *pt, int nid)
 }
 
 /* Initialize and add a pool type to the global shrinker list */
-static void ttm_pool_type_init(struct ttm_pool_type *pt, struct ttm_pool *pool,
+static int ttm_pool_type_init(struct ttm_pool_type *pt, struct ttm_pool *pool,
 			       enum ttm_caching caching, unsigned int order)
 {
+	int ret = 0;
+
 	pt->pool = pool;
 	pt->caching = caching;
 	pt->order = order;
-	list_lru_init(&pt->pages);
+	ret = list_lru_init(&pt->pages);
+	if (ret)
+		return ret;
 
 	spin_lock(&shrinker_lock);
 	list_add_tail(&pt->shrinker_list, &shrinker_list);
 	spin_unlock(&shrinker_lock);
+
+	return 0;
 }
 
 static enum lru_status pool_move_to_dispose_list(struct list_head *item,
-- 
2.55.0

Re: [PATCH v5 2/6] list_lru_init() does not check return value

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The return value of `ttm_pool_type_init()` is ignored in `ttm_pool_mgr_init()`, leading to kernel panics if `list_lru_init()` fails.
- [High] External callers of `ttm_pool_init()` and `ttm_pool_mgr_init()` ignore their error return values, leading to silent failures and crashes.

Pre-existing issues:
- [High] Missing `list_lru_destroy()` calls in normal teardown paths leak memory on every pool destruction and module unload.
--

--- Patch [2]: [PATCH v5 2/6] list_lru_init() does not check return value ---
commit 05689bd64e30d8c766538356d67e8a63f77ecc7c
Author: Óscar Megía López <megia.oscar@gmail.com>

list_lru_init() does not check return value

This patch adds an error check for list_lru_init() in ttm_pool_type_init(),
propagating the error code back to the caller if the initialization fails.

Cc: <stable@vger.kernel.org> # 7.1.0

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 73284497cc785..e116b83db1356 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -374,17 +374,23 @@ static struct page *ttm_pool_type_take(struct ttm_pool_type *pt, int nid)
>  }
>  
>  /* Initialize and add a pool type to the global shrinker list */
> -static void ttm_pool_type_init(struct ttm_pool_type *pt, struct ttm_pool *pool,
> +static int ttm_pool_type_init(struct ttm_pool_type *pt, struct ttm_pool *pool,
>  			       enum ttm_caching caching, unsigned int order)

[Severity: High]
Since ttm_pool_type_init() now returns an error code, do the callers need
to be updated to check it?

Looking at ttm_pool_mgr_init(), it calls ttm_pool_type_init() in a loop
without checking the return value. If list_lru_init() fails, it skips adding
the node to shrinker_list.

During module unload, ttm_pool_mgr_fini() will call ttm_pool_type_fini(),
which executes list_del() on the uninitialized items. Will this result in a
NULL pointer dereference?

Also, ttm_pool_type_take() calls list_lru_walk_node() unconditionally on
pt->pages. Can this crash if it was never initialized?

[Severity: High]
Similarly, do external callers of ttm_pool_init() and ttm_pool_mgr_init()
need to check for errors now that list_lru_init() failures are propagated?

Callers like ttm_device_init() and amdgpu_ttm_pools_init() appear to ignore
the return values. If initialization fails, the device driver might later
attempt an allocation or unbind the device, accessing uninitialized list_lru
structures.

>  {
> +	int ret = 0;
> +
>  	pt->pool = pool;
>  	pt->caching = caching;
>  	pt->order = order;
> -	list_lru_init(&pt->pages);
> +	ret = list_lru_init(&pt->pages);

[Severity: High]
This is a pre-existing issue, but does normal teardown leak the list_lru
allocations?

list_lru_init() dynamically allocates node arrays. While ttm_pool_type_init()
sets it up, ttm_pool_type_fini() does not appear to call list_lru_destroy().

Neither ttm_pool_fini() nor ttm_pool_mgr_fini() call list_lru_destroy() on
normal device unbinds or module unloads. If a device is repeatedly bound and
unbound via sysfs, will this systematically leak memory and eventually exhaust
system memory?

> +	if (ret)
> +		return ret;
>  
>  	spin_lock(&shrinker_lock);
>  	list_add_tail(&pt->shrinker_list, &shrinker_list);
>  	spin_unlock(&shrinker_lock);
> +
> +	return 0;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811194224.121597-1-megia.oscar@gmail.com?part=2

[PATCH v5 3/6] ttm_pool_fini() does not destroy list lru

[Óscar Megía López]

Bug: ttm_pool_fini() does not destroy list lru with list_lru_destroy().

Fix: Add list_lru_destroy() after ttm_pool_type_fini().

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Assisted-by: claude.ai:Sonnet 5
Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_pool.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 8637f7942347..87c843f52736 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -1232,6 +1232,17 @@ void ttm_pool_fini(struct ttm_pool *pool)
 	 * that no shrinker is concurrently freeing pages from the pool.
 	 */
 	ttm_pool_synchronize_shrinkers();
+
+	for (i = 0; i < TTM_NUM_CACHING_TYPES; ++i) {
+		for (j = 0; j < NR_PAGE_ORDERS; ++j) {
+			struct ttm_pool_type *pt;
+
+			pt = ttm_pool_select_type(pool, i, j);
+			if (pt != &pool->caching[i].orders[j])
+				continue;
+			list_lru_destroy(&pt->pages);
+		}
+	}
 }
 EXPORT_SYMBOL(ttm_pool_fini);
 
-- 
2.55.0

Re: [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] ttm_pool_mgr_init() ignores the return value of ttm_pool_type_init(), which can lead to a NULL pointer dereference.
- [High] ttm_pool_shrink() leaks the pool_shrink_rwsem read lock when the shrinker list is empty.
- [High] ttm_global_init() ignores the return value of ttm_pool_mgr_init().
- [High] ttm_device_init() ignores the return value of ttm_pool_init(), causing a potential NULL pointer dereference.
- [Medium] Missing list_lru_destroy() in ttm_pool_mgr_fini() causes a memory leak for global pools on module unload.
- [Medium] Memory leak on the shrinker_alloc() error path in ttm_pool_mgr_init().
--

--- Patch [3]: [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru ---
commit bab162cb05ce53ff29cbacb61b7c2543d1b98ecc
Author: Óscar Megía López <megia.oscar@gmail.com>
ttm_pool_fini() does not destroy list lru

This commit adds a missing call to list_lru_destroy() in ttm_pool_fini()
after calling ttm_pool_type_fini(). It ensures the list lru structures
are properly destroyed when finalizing a ttm pool.

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index e116b83db1356..b95c6548adf7f 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -1331,6 +1331,17 @@ void ttm_pool_fini(struct ttm_pool *pool)
>  	 * that no shrinker is concurrently freeing pages from the pool.
>  	 */
>  	ttm_pool_synchronize_shrinkers();
> +
> +	for (i = 0; i < TTM_NUM_CACHING_TYPES; ++i) {
> +		for (j = 0; j < NR_PAGE_ORDERS; ++j) {
> +			struct ttm_pool_type *pt;
> +
> +			pt = ttm_pool_select_type(pool, i, j);
> +			if (pt != &pool->caching[i].orders[j])
> +				continue;
> +			list_lru_destroy(&pt->pages);
> +		}
> +	}
>  }
>  EXPORT_SYMBOL(ttm_pool_fini);

[Severity: Medium]
This is a pre-existing issue, but does ttm_pool_mgr_fini() leak memory
on module unload?

When the module is unloaded, it calls ttm_pool_type_fini() to free the pages,
but it appears to miss calling list_lru_destroy() for the global pools:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_mgr_fini() {
	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
		ttm_pool_type_fini(&global_write_combined[i]);
		...
	}
}

Will this permanently leak the memory allocated for the list_lru structures
during initialization?

[Severity: High]
This isn't a bug introduced by this patch, but does ttm_pool_mgr_init()
ignore the return value of ttm_pool_type_init()?

If list_lru_init() fails due to memory exhaustion, the pool's list_lru
remains uninitialized. The loop continues without aborting:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_mgr_init() {
	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
		ttm_pool_type_init(&global_write_combined[i], NULL,
				   ttm_write_combined, i);
		...
	}
}

Later, when pages are freed, list_lru_add() might blindly dereference
a NULL lru->node array:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_type_give() {
	rcu_read_lock();
	list_lru_add(&pt->pages, &p->lru, nid, NULL);
	rcu_read_unlock();
}

Could this cause a kernel panic?

[Severity: High]
This is a pre-existing issue, but does ttm_pool_shrink() leak the
pool_shrink_rwsem read lock when the shrinker list is empty?

The read lock is acquired via down_read(&pool_shrink_rwsem). If the
shrinker_list is empty, it sets a flag and returns early without releasing
the lock:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_shrink() {
	down_read(&pool_shrink_rwsem);
	spin_lock(&shrinker_lock);
	if ((shrinker_list.prev == &shrinker_list) && ...) {
		empty = 1;
	} else {
		...
	}
	spin_unlock(&shrinker_lock);

	if (empty)
		return 0;
}

Could this deadlock the system when subsequent attempts try to acquire
a write lock in ttm_pool_synchronize_shrinkers()?

[Severity: High]
This isn't a bug introduced by this patch, but does ttm_global_init() mask
initialization failures?

The return value of ttm_pool_mgr_init() is discarded:

drivers/gpu/drm/ttm/ttm_device.c:ttm_global_init() {
	ttm_pool_mgr_init(num_pages);
	ttm_tt_mgr_init(num_pages, num_dma32);
}

If ttm_pool_mgr_init() fails, mm_shrinker might be left as NULL, which
could break the memory reclaim subsystem for TTM pools. Should this error
be propagated?

[Severity: High]
This is a pre-existing issue, but does ttm_device_init() ignore the return
value of ttm_pool_init()?

If ttm_pool_init() fails, it cleans up its partial state and returns an error
code, leaving bdev->pool uninitialized:

drivers/gpu/drm/ttm/ttm_device.c:ttm_device_init() {
	if (dev)
		nid = dev_to_node(dev);
	else
		nid = NUMA_NO_NODE;

	ttm_pool_init(&bdev->pool, dev, nid, alloc_flags);
}

During cleanup, ttm_device_fini() invokes ttm_pool_fini(), which calls
ttm_pool_type_fini(). This executes list_del(&pt->shrinker_list), which
might crash with a NULL pointer dereference because the list head was never
initialized. Can this lead to a crash on teardown?

[Severity: Medium]
This isn't a bug introduced by this patch, but is there a memory leak on
the shrinker_alloc() error path in ttm_pool_mgr_init()?

If shrinker_alloc() fails, the function returns directly:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_mgr_init() {
	mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
	if (!mm_shrinker)
		return -ENOMEM;
}

Since the global pool types were already initialized dynamically, will
returning directly here permanently leak the list_lru memory allocated
for those pools?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811194224.121597-1-megia.oscar@gmail.com?part=3

[PATCH v5 4/6] ttm_pool_type_init() does not check return value

[Óscar Megía López]

Bug: ttm_pool_mgr_init() does not check ttm_pool_type_init() return
value and does not free pool if returns error.

Fix: Move up shrinker_alloc(), check ttm_pool_type_init() return and free
pool types and shrinker if non-zero and return error.

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Assisted-by: claude.ai:Sonnet 5
Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_pool.c | 100 ++++++++++++++++++++++++++++++---
 1 file changed, 92 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 87c843f52736..74d8770f41d8 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -1421,6 +1421,54 @@ static inline u64 ttm_get_node_memory_size(int nid)
 	return managed_pages * PAGE_SIZE;
 }
 
+static void ttm_pool_type_fini_and_list_lru_destroy(unsigned int nr)
+{
+	unsigned int i;
+
+	if (nr == 0)
+		return;
+
+	for (i = 0; i < nr; ++i) {
+		ttm_pool_type_fini(&global_write_combined[i]);
+		ttm_pool_type_fini(&global_uncached[i]);
+		ttm_pool_type_fini(&global_dma32_write_combined[i]);
+		ttm_pool_type_fini(&global_dma32_uncached[i]);
+	}
+
+	/* We removed the pool types from the LRU, but we need to also make sure
+	 * that no shrinker is concurrently freeing pages from the pool.
+	 */
+	ttm_pool_synchronize_shrinkers();
+
+	for (i = 0; i < nr; ++i) {
+		list_lru_destroy(&global_write_combined[i].pages);
+		list_lru_destroy(&global_uncached[i].pages);
+		list_lru_destroy(&global_dma32_write_combined[i].pages);
+		list_lru_destroy(&global_dma32_uncached[i].pages);
+	}
+
+}
+
+static void ttm_pool_type_fini_and_list_lru_destroy_partial(
+				struct ttm_pool_type *types[], unsigned int n)
+{
+	unsigned int k;
+
+	if (n == 0)
+		return;
+
+	for (k = 0; k < n; ++k)
+		ttm_pool_type_fini(types[k]);
+
+	/* We removed the pool types from the LRU, but we need to also make sure
+	 * that no shrinker is concurrently freeing pages from the pool.
+	 */
+	ttm_pool_synchronize_shrinkers();
+
+	for (k = 0; k < n; ++k)
+		list_lru_destroy(&types[k]->pages);
+}
+
 /**
  * ttm_pool_mgr_init - Initialize globals
  *
@@ -1431,6 +1479,8 @@ static inline u64 ttm_get_node_memory_size(int nid)
 int ttm_pool_mgr_init(unsigned long num_pages)
 {
 	unsigned int i;
+	int ret = 0;
+	struct ttm_pool_type *types_free[3];
 
 	int nid;
 	for_each_node(nid) {
@@ -1445,15 +1495,53 @@ int ttm_pool_mgr_init(unsigned long num_pages)
 	spin_lock_init(&shrinker_lock);
 	INIT_LIST_HEAD(&shrinker_list);
 
+	mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
+	if (!mm_shrinker)
+		return -ENOMEM;
+
 	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
-		ttm_pool_type_init(&global_write_combined[i], NULL,
+		ret = ttm_pool_type_init(&global_write_combined[i], NULL,
 				   ttm_write_combined, i);
-		ttm_pool_type_init(&global_uncached[i], NULL, ttm_uncached, i);
+		if (ret) {
+			ttm_pool_type_fini_and_list_lru_destroy(i);
+			shrinker_free(mm_shrinker);
+			return ret;
+		}
+
+		ret = ttm_pool_type_init(&global_uncached[i], NULL, ttm_uncached, i);
+		if (ret) {
+			types_free[0] = &global_write_combined[i];
+			ttm_pool_type_fini_and_list_lru_destroy_partial(types_free, 1);
 
-		ttm_pool_type_init(&global_dma32_write_combined[i], NULL,
+			ttm_pool_type_fini_and_list_lru_destroy(i);
+			shrinker_free(mm_shrinker);
+			return ret;
+		}
+
+		ret = ttm_pool_type_init(&global_dma32_write_combined[i], NULL,
 				   ttm_write_combined, i);
-		ttm_pool_type_init(&global_dma32_uncached[i], NULL,
+		if (ret) {
+			types_free[0] = &global_write_combined[i];
+			types_free[1] = &global_uncached[i];
+			ttm_pool_type_fini_and_list_lru_destroy_partial(types_free, 2);
+
+			ttm_pool_type_fini_and_list_lru_destroy(i);
+			shrinker_free(mm_shrinker);
+			return ret;
+		}
+
+		ret = ttm_pool_type_init(&global_dma32_uncached[i], NULL,
 				   ttm_uncached, i);
+		if (ret) {
+			types_free[0] = &global_write_combined[i];
+			types_free[1] = &global_uncached[i];
+			types_free[2] = &global_dma32_write_combined[i];
+			ttm_pool_type_fini_and_list_lru_destroy_partial(types_free, 3);
+
+			ttm_pool_type_fini_and_list_lru_destroy(i);
+			shrinker_free(mm_shrinker);
+			return ret;
+		}
 	}
 
 #ifdef CONFIG_DEBUG_FS
@@ -1467,10 +1555,6 @@ int ttm_pool_mgr_init(unsigned long num_pages)
 #endif
 #endif
 
-	mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
-	if (!mm_shrinker)
-		return -ENOMEM;
-
 	mm_shrinker->count_objects = ttm_pool_shrinker_count;
 	mm_shrinker->scan_objects = ttm_pool_shrinker_scan;
 	mm_shrinker->batch = TTM_SHRINKER_BATCH;
-- 
2.55.0

Re: [PATCH v5 4/6] ttm_pool_type_init() does not check return value

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing return value check of ttm_pool_mgr_init() combined with destructive error cleanup leads to double free and list poisoning panic.
--

--- Patch [4]: [PATCH v5 4/6] ttm_pool_type_init() does not check return value ---
commit 141193c84a0c35c0b77d8194cafc18a0ddfa41ee
Author: Óscar Megía López <megia.oscar@gmail.com>

ttm_pool_type_init() does not check return value

This commit modifies ttm_pool_mgr_init() to check the return value of
ttm_pool_type_init(). If initialization fails, it cleans up the partially
allocated structures, frees the shrinker, and returns the error code.

Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index b95c6548adf7f..8d9891ea8c6aa 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c

[ ... ]

> @@ -1537,6 +1585,8 @@ static inline u64 ttm_get_node_memory_size(int nid)
>  int ttm_pool_mgr_init(unsigned long num_pages)
>  {
>  	unsigned int i;
> +	int ret = 0;
> +	struct ttm_pool_type *types_free[3];
>  
>  	int nid;
>  	for_each_node(nid) {

[ ... ]

> +	mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
> +	if (!mm_shrinker)
> +		return -ENOMEM;
> +
>  	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
> -		ttm_pool_type_init(&global_write_combined[i], NULL,
> +		ret = ttm_pool_type_init(&global_write_combined[i], NULL,
>  				   ttm_write_combined, i);
> -		ttm_pool_type_init(&global_uncached[i], NULL, ttm_uncached, i);
> +		if (ret) {
> +			ttm_pool_type_fini_and_list_lru_destroy(i);
> +			shrinker_free(mm_shrinker);
> +			return ret;

[Severity: High]
If ttm_pool_mgr_init() fails and returns an error here, can this lead to
a double-free or panic during teardown?

Looking at the caller ttm_global_init() in drivers/gpu/drm/ttm/ttm_device.c,
the return value is ignored:

ttm_global_init(void)
{
        ...
        ttm_pool_mgr_init(num_pages);
        ttm_tt_mgr_init(num_pages, num_dma32);
        ...
}

Because the caller ignores the error, the system will continue running with
partially destroyed global state. When the module is eventually unloaded,
ttm_pool_mgr_fini() will run unconditionally:

ttm_pool_mgr_fini(void)
{
        ...
        for (i = 0; i < NR_PAGE_ORDERS; ++i) {
                ttm_pool_type_fini(&global_write_combined[i]);
                ...
        }
        ...
        shrinker_free(mm_shrinker);
}

Since mm_shrinker isn't set to NULL after the error path calls
shrinker_free(), won't ttm_pool_mgr_fini() trigger a double-free on the
shrinker?

Also, won't ttm_pool_mgr_fini() attempt to access list entries that have
already been poisoned or deleted by
ttm_pool_type_fini_and_list_lru_destroy() in the error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811194224.121597-1-megia.oscar@gmail.com?part=4

[PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru

[Óscar Megía López]

Bug: ttm_pool_mgr_fini() does not destroy the list_lru.

Fix: Add list_lru_destroy() after ttm_pool_type_fini().

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Assisted-by: claude.ai:Sonnet 5
Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_pool.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 74d8770f41d8..876b6d3a632d 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -1572,16 +1572,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
  */
 void ttm_pool_mgr_fini(void)
 {
-	unsigned int i;
-
-	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
-		ttm_pool_type_fini(&global_write_combined[i]);
-		ttm_pool_type_fini(&global_uncached[i]);
-
-		ttm_pool_type_fini(&global_dma32_write_combined[i]);
-		ttm_pool_type_fini(&global_dma32_uncached[i]);
-	}
-
 	shrinker_free(mm_shrinker);
+	ttm_pool_type_fini_and_list_lru_destroy(NR_PAGE_ORDERS);
 	WARN_ON(!list_empty(&shrinker_list));
 }
-- 
2.55.0

Re: [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Destroying the global list_lru structures before removing the TTM debugfs files introduces a NULL pointer dereference vulnerability.

Pre-existing issues:
- [High] ttm_pool_shrink() leaks a read lock on pool_shrink_rwsem when shrinker_list is empty.
- [High] The `ttm_pool_shrinker_scan()` function can enter an infinite busy-loop if `allocated_pages` is positive but all `list_lru`s are empty.
--

--- Patch [5]: [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru ---
commit 39533ac28eb17cc8fb3c32efa95aac77421f3675
Author: Óscar Megía López <megia.oscar@gmail.com>

ttm_pool_mgr_fini() does not destroy the list_lru

This commit adds list_lru_destroy() after ttm_pool_type_fini() to ensure
the list_lru structures are properly destroyed during pool manager teardown,
resolving a resource leak.

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 8d9891ea8c6aa..0d063df864f37 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -1678,16 +1678,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
>   */
>  void ttm_pool_mgr_fini(void)
>  {
[ ... ]
>  	shrinker_free(mm_shrinker);
> +	ttm_pool_type_fini_and_list_lru_destroy(NR_PAGE_ORDERS);

[Severity: High]
Does this introduce a race condition with debugfs?

Looking at the teardown sequence in ttm_global_release(), ttm_pool_mgr_fini()
is called before debugfs_remove(ttm_debugfs_root). This leaves a window
where the page_pool debugfs file remains accessible after the underlying
list_lru structures are completely destroyed.

If a user reads the page_pool debugfs file during this window,
ttm_pool_debugfs_globals_show() iterates over the global pool arrays and
calls list_lru_count(). Since list_lru_destroy() frees the internal nodes
array and sets lru->node to NULL, won't this result in a NULL pointer
dereference inside list_lru_count_node()?

This is a pre-existing issue, but I noticed a lock leak in ttm_pool_shrink()
that could be triggered during this same teardown window. When shrinker_list
is empty, ttm_pool_shrink() has an early return path:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_shrink() {
        down_read(&pool_shrink_rwsem);
        spin_lock(&shrinker_lock);
        if ((shrinker_list.prev == &shrinker_list) && ...) {
                empty = 1;
        ...
        if (empty)
                return 0;
        ...
}

Since down_read(&pool_shrink_rwsem) is not released on this early return
path, won't this leak the read lock and cause a deadlock during subsequent
operations?

This is also a pre-existing issue, but the shrinker scan loop appears
vulnerable to an infinite busy-loop.

In ttm_pool_shrinker_scan(), there is a loop checking allocated_pages:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_shrinker_scan() {
        do
                num_freed += ttm_pool_shrink(sc->nid, sc->nr_to_scan);
        while (num_freed < sc->nr_to_scan &&
               atomic_long_read(&allocated_pages[sc->nid]));
        ...
}

Meanwhile, ttm_pool_type_take() isolates pages from the LRU before
updating the counter:

drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_type_take() {
        ret = list_lru_walk_node(&pt->pages, ...);
        if (ret == 1 && p) {
                atomic_long_sub(1 << pt->order, &allocated_pages[nid]);
        ...
}

During the window where a page is removed from the LRU but allocated_pages
has not yet been decremented, the LRU could be empty while allocated_pages
is still positive. If ttm_pool_shrinker_scan() runs concurrently and sees
ttm_pool_shrink() return 0, could it spin indefinitely without yielding the
CPU, especially since there is no cond_resched() inside the loop?

>  	WARN_ON(!list_empty(&shrinker_list));
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811194224.121597-1-megia.oscar@gmail.com?part=5

[PATCH v5 6/6] ttm_pool_mgr_init() does not check return value

[Óscar Megía López]

Bug: ttm_pool_mgr_init() does not check return value in
ttm_global_init().

Fix: Check the return value from ttm_pool_mgr_init() and propagate
the error.

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Assisted-by: claude.ai:Sonnet 5
Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index d3bfb9a696a7..896b766712d0 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -96,7 +96,9 @@ static int ttm_global_init(void)
 		>> PAGE_SHIFT;
 	num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT));
 
-	ttm_pool_mgr_init(num_pages);
+	ret = ttm_pool_mgr_init(num_pages);
+	if (ret)
+		goto out;
 	ttm_tt_mgr_init(num_pages, num_dma32);
 
 	glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32 |
-- 
2.55.0

Re: [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing cleanup of `ttm_pool_mgr_init()` allocations when `alloc_page()` fails in `ttm_global_init()`.
--

--- Patch [6]: [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value ---
commit 18452a40ead8211d272e9f32ad64ceec8ef1cf22
Author: Óscar Megía López <megia.oscar@gmail.com>

ttm_pool_mgr_init() does not check return value

This commit fixes a missing return value check for ttm_pool_mgr_init() in
ttm_global_init(). It propagates the error by jumping to the out label if
initialization fails.

Cc: <stable@vger.kernel.org> # 7.1.0
Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")

> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index d3bfb9a696a74..896b766712d04 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -96,7 +96,9 @@ static int ttm_global_init(void)
>  		>> PAGE_SHIFT;
>  	num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT));
>  
> -	ttm_pool_mgr_init(num_pages);
> +	ret = ttm_pool_mgr_init(num_pages);
> +	if (ret)
> +		goto out;

[Severity: High]
This is a pre-existing issue, but does the error handling at the out label
properly clean up after a successful ttm_pool_mgr_init() call?

If the dummy_read_page allocation fails later in this function, we
jump to the out label returning -ENOMEM:

        glob->dummy_read_page = alloc_page(__GFP_ZERO);
        if (unlikely(glob->dummy_read_page == NULL)) {
                ret = -ENOMEM;
                goto out;
        }

However, the cleanup at the out label doesn't appear to call
ttm_pool_mgr_fini() or unregister the shrinker:

out:
        if (ret && ttm_debugfs_root)
                debugfs_remove(ttm_debugfs_root);
        if (ret)
                --ttm_glob_use_count;
        mutex_unlock(&ttm_global_mutex);
        return ret;

Since ttm_pool_mgr_init() registers a shrinker (mm_shrinker) and
initializes global spinlocks and lists:

        spin_lock_init(&shrinker_lock);
        INIT_LIST_HEAD(&shrinker_list);
        ...
        mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");

Could this leave a registered shrinker behind on failure?

If ttm_global_init() is called again on a subsequent driver load, it would
unconditionally re-initialize shrinker_lock and shrinker_list. If the
originally leaked shrinker is running concurrently during memory reclaim,
could this cause lock or list corruption?

>  	ttm_tt_mgr_init(num_pages, num_dma32);
>  
>  	glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32 |

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811194224.121597-1-megia.oscar@gmail.com?part=6