* [PATCH v5 0/6] Series from memory leak on qxl unbind
@ 2026-08-11 19:42 Óscar Megía López
2026-08-11 19:42 ` [PATCH v5 1/6] Memory leak error in " Óscar Megía López
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/6] Memory leak error in qxl unbind
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
@ 2026-08-11 19:42 ` Óscar Megía López
2026-08-11 19:54 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 2/6] list_lru_init() does not check return value Óscar Megía López
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 2/6] list_lru_init() does not check return value
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
2026-08-11 19:42 ` [PATCH v5 1/6] Memory leak error in " Óscar Megía López
@ 2026-08-11 19:42 ` Óscar Megía López
2026-08-11 19:55 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru Óscar Megía López
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
2026-08-11 19:42 ` [PATCH v5 1/6] Memory leak error in " Óscar Megía López
2026-08-11 19:42 ` [PATCH v5 2/6] list_lru_init() does not check return value Óscar Megía López
@ 2026-08-11 19:42 ` Óscar Megía López
2026-08-11 19:54 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 4/6] ttm_pool_type_init() does not check return value Óscar Megía López
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 4/6] ttm_pool_type_init() does not check return value
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
` (2 preceding siblings ...)
2026-08-11 19:42 ` [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru Óscar Megía López
@ 2026-08-11 19:42 ` Óscar Megía López
2026-08-11 19:52 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru Óscar Megía López
2026-08-11 19:42 ` [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value Óscar Megía López
5 siblings, 1 reply; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
` (3 preceding siblings ...)
2026-08-11 19:42 ` [PATCH v5 4/6] ttm_pool_type_init() does not check return value Óscar Megía López
@ 2026-08-11 19:42 ` Óscar Megía López
2026-08-11 19:54 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value Óscar Megía López
5 siblings, 1 reply; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
` (4 preceding siblings ...)
2026-08-11 19:42 ` [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru Óscar Megía López
@ 2026-08-11 19:42 ` Óscar Megía López
2026-08-11 19:50 ` sashiko-bot
5 siblings, 1 reply; 13+ messages in thread
From: Óscar Megía López @ 2026-08-11 19:42 UTC (permalink / raw)
To: Christian Koenig, Huang Rui
Cc: Óscar Megía López, Matthew Auld, Matthew Brost,
dri-devel, linux-kernel, linux-kernel-mentees, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value
2026-08-11 19:42 ` [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value Óscar Megía López
@ 2026-08-11 19:50 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-11 19:50 UTC (permalink / raw)
To: Óscar Megía López; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 4/6] ttm_pool_type_init() does not check return value
2026-08-11 19:42 ` [PATCH v5 4/6] ttm_pool_type_init() does not check return value Óscar Megía López
@ 2026-08-11 19:52 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-11 19:52 UTC (permalink / raw)
To: Óscar Megía López; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru
2026-08-11 19:42 ` [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru Óscar Megía López
@ 2026-08-11 19:54 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-11 19:54 UTC (permalink / raw)
To: Óscar Megía López; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru
2026-08-11 19:42 ` [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru Óscar Megía López
@ 2026-08-11 19:54 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-11 19:54 UTC (permalink / raw)
To: Óscar Megía López; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/6] Memory leak error in qxl unbind
2026-08-11 19:42 ` [PATCH v5 1/6] Memory leak error in " Óscar Megía López
@ 2026-08-11 19:54 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-11 19:54 UTC (permalink / raw)
To: Óscar Megía López; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/6] list_lru_init() does not check return value
2026-08-11 19:42 ` [PATCH v5 2/6] list_lru_init() does not check return value Óscar Megía López
@ 2026-08-11 19:55 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-11 19:55 UTC (permalink / raw)
To: Óscar Megía López; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-08-11 19:55 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 19:42 [PATCH v5 0/6] Series from memory leak on qxl unbind Óscar Megía López
2026-08-11 19:42 ` [PATCH v5 1/6] Memory leak error in " Óscar Megía López
2026-08-11 19:54 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 2/6] list_lru_init() does not check return value Óscar Megía López
2026-08-11 19:55 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 3/6] ttm_pool_fini() does not destroy list lru Óscar Megía López
2026-08-11 19:54 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 4/6] ttm_pool_type_init() does not check return value Óscar Megía López
2026-08-11 19:52 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 5/6] ttm_pool_mgr_fini() does not destroy the list_lru Óscar Megía López
2026-08-11 19:54 ` sashiko-bot
2026-08-11 19:42 ` [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value Óscar Megía López
2026-08-11 19:50 ` sashiko-bot
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.