[PATCH v3] Memory leak error in qxl unbind

[PATCH v3] Memory leak error in qxl unbind

[Óscar Megía López]

I discovered OOM after run script below around two hours in my computer:

while :; do
    echo [pci qxl id] > /sys/bus/pci/drivers/qxl/unbind
    echo [pci qxl id] > /sys/bus/pci/drivers/qxl/bind
done

If you run 10000 times above script kmemleak does not report any
memory leak, but if you run above script about two hours several
OOM ocurs and at the end kernel panic.

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

Here's the chain:

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.

You must apply the patch from the link "[PATCH v3] drm/qxl: fix
use-after-free in qxl_irq_handler on PCI" before testing.
If you don't apply this patch, you will get a UAF error when running
the script above.

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_take, check if
pt->pages.node is NULL in ttm_pool_type_fini and ttm_pool_type_init
return value in ttm_pool_mgr_init.

Solved new issues:
- [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.

Solved pre-existing issues:
- [High] `ttm_pool_type_init()` ignores the return value of `list_lru_init()`, leading to a NULL pointer dereference if allocation fails.
- [High] `ttm_pool_shrink()` assumes `shrinker_list` is never empty, causing memory corruption and crashes during module unload if triggered.

Assisted-by: OpenCode:1.17.18-Big Pickle/DeepSeek V4 Flash
Link: https://lore.kernel.org/virtualization/20260727110212.64913-1-megia.oscar@gmail.com/T/#u
Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
---
 drivers/gpu/drm/drm_gem.c        |  1 +
 drivers/gpu/drm/ttm/ttm_device.c |  5 ++-
 drivers/gpu/drm/ttm/ttm_pool.c   | 71 +++++++++++++++++++++++++-------
 3 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index e3ed684ddcf2..b3f608619cfa 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -143,6 +143,7 @@ static void
 drm_gem_init_release(struct drm_device *dev, void *ptr)
 {
 	drm_vma_offset_manager_destroy(dev->vma_offset_manager);
+	idr_destroy(&dev->object_name_idr);
 }
 
 /**
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index d3bfb9a696a7..c880a0430363 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -96,7 +96,10 @@ 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 |
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 278bbe7a11ad..9e82454f818e 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,
@@ -438,6 +444,9 @@ static unsigned int ttm_pool_shrink(int nid, unsigned long num_to_free)
 	struct ttm_pool_type *pt;
 	unsigned int num_pages;
 
+	if (shrinker_list.prev == NULL && shrinker_list.next == NULL)
+		return 0;
+
 	down_read(&pool_shrink_rwsem);
 	spin_lock(&shrinker_lock);
 	pt = list_first_entry(&shrinker_list, typeof(*pt), shrinker_list);
@@ -1198,6 +1207,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);
 
@@ -1376,6 +1396,22 @@ 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(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < NR_PAGE_ORDERS; ++i) {
+		ttm_pool_type_fini(&global_write_combined[i]);
+		list_lru_destroy(&global_write_combined[i].pages);
+		ttm_pool_type_fini(&global_uncached[i]);
+		list_lru_destroy(&global_uncached[i].pages);
+		ttm_pool_type_fini(&global_dma32_write_combined[i]);
+		list_lru_destroy(&global_dma32_write_combined[i].pages);
+		ttm_pool_type_fini(&global_dma32_uncached[i]);
+		list_lru_destroy(&global_dma32_uncached[i].pages);
+	}
+}
+
 /**
  * ttm_pool_mgr_init - Initialize globals
  *
@@ -1386,6 +1422,7 @@ 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;
 
 	int nid;
 	for_each_node(nid) {
@@ -1423,8 +1460,10 @@ int ttm_pool_mgr_init(unsigned long num_pages)
 #endif
 
 	mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
-	if (!mm_shrinker)
-		return -ENOMEM;
+	if (!mm_shrinker) {
+		ret = -ENOMEM;
+		goto err_shrinker;
+	}
 
 	mm_shrinker->count_objects = ttm_pool_shrinker_count;
 	mm_shrinker->scan_objects = ttm_pool_shrinker_scan;
@@ -1434,6 +1473,10 @@ int ttm_pool_mgr_init(unsigned long num_pages)
 	shrinker_register(mm_shrinker);
 
 	return 0;
+
+err_shrinker:
+	ttm_pool_type_fini_and_list_lru_destroy();
+	return ret;
 }
 
 /**
@@ -1443,16 +1486,12 @@ 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);
-	WARN_ON(!list_empty(&shrinker_list));
+    if (backup_fault_inject.dname) {
+        dput(backup_fault_inject.dname);
+        backup_fault_inject.dname = NULL;
+    }
+
+    shrinker_free(mm_shrinker);
+    ttm_pool_type_fini_and_list_lru_destroy();
+    WARN_ON(!list_empty(&shrinker_list));
 }
-- 
2.55.0

Re: [PATCH v3] Memory leak error in qxl unbind

[Markus Elfring]

…
> Fix: Check ret from ttm_pool_mgr_init; if non-zero, goto out cleans up
>refcount + debugfs.
…
> Fix: err_shrinker: label that finalizes + destroys all 64 pool types
> before returning.
> 
> You must apply the patch from the link "[PATCH v3] drm/qxl: fix
> use-after-free in qxl_irq_handler on PCI" before testing.
> If you don't apply this patch, you will get a UAF error when running
> the script above.
> 
> 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_take, check if
> pt->pages.node is NULL in ttm_pool_type_fini and ttm_pool_type_init
> return value in ttm_pool_mgr_init.
> 
> Solved new issues:
> - [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.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

See also:
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n145
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2-rc5#n34


> Solved pre-existing issues:
> - [High] `ttm_pool_type_init()` ignores the return value of `list_lru_init()`, leading to a NULL pointer dereference if allocation fails.
> - [High] `ttm_pool_shrink()` assumes `shrinker_list` is never empty, causing memory corruption and crashes during module unload if triggered.

Would another change description variant become better?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n81


…
> Signed-off-by: Óscar Megía López <megia.oscar@gmail.com>
> ---
>  drivers/gpu/drm/drm_gem.c        |  1 +
…

Some contributors would appreciate patch version descriptions.
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n310

Regards,
Markus

Re: [PATCH v3] Memory leak error in qxl unbind

[kernel test robot]

Hi Óscar,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v7.2-rc5 next-20260731]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/scar-Meg-a-L-pez/Memory-leak-error-in-qxl-unbind/20260803-140059
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20260727114155.69204-1-megia.oscar%40gmail.com
patch subject: [PATCH v3] Memory leak error in qxl unbind
config: parisc-defconfig (https://download.01.org/0day-ci/archive/20260803/202608031736.7YYw01c3-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260803/202608031736.7YYw01c3-lkp@intel.com/reproduce)

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

All errors (new ones prefixed by >>):

   drivers/gpu/drm/ttm/ttm_pool.c: In function 'ttm_pool_mgr_fini':
>> drivers/gpu/drm/ttm/ttm_pool.c:1510:9: error: 'backup_fault_inject' undeclared (first use in this function)
    1510 |     if (backup_fault_inject.dname) {
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/ttm/ttm_pool.c:1510:9: note: each undeclared identifier is reported only once for each function it appears in


vim +/backup_fault_inject +1510 drivers/gpu/drm/ttm/ttm_pool.c

  1502	
  1503	/**
  1504	 * ttm_pool_mgr_fini - Finalize globals
  1505	 *
  1506	 * Cleanup the global pools and unregister the MM shrinker.
  1507	 */
  1508	void ttm_pool_mgr_fini(void)
  1509	{
> 1510	    if (backup_fault_inject.dname) {

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

Re: [PATCH v3] Memory leak error in qxl unbind

[kernel test robot]

Hi Óscar,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v7.2-rc5 next-20260731]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/scar-Meg-a-L-pez/Memory-leak-error-in-qxl-unbind/20260803-140059
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20260727114155.69204-1-megia.oscar%40gmail.com
patch subject: [PATCH v3] Memory leak error in qxl unbind
config: loongarch-defconfig (https://download.01.org/0day-ci/archive/20260803/202608031807.sRyvtAVm-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project bacfe2950f8218268fcc0a8765644ea0c15f0360)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260803/202608031807.sRyvtAVm-lkp@intel.com/reproduce)

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

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/ttm/ttm_pool.c:1510:9: error: use of undeclared identifier 'backup_fault_inject'
    1510 |     if (backup_fault_inject.dname) {
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/ttm/ttm_pool.c:1511:14: error: use of undeclared identifier 'backup_fault_inject'
    1511 |         dput(backup_fault_inject.dname);
         |              ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/ttm/ttm_pool.c:1512:9: error: use of undeclared identifier 'backup_fault_inject'
    1512 |         backup_fault_inject.dname = NULL;
         |         ^~~~~~~~~~~~~~~~~~~
   3 errors generated.


vim +/backup_fault_inject +1510 drivers/gpu/drm/ttm/ttm_pool.c

  1502	
  1503	/**
  1504	 * ttm_pool_mgr_fini - Finalize globals
  1505	 *
  1506	 * Cleanup the global pools and unregister the MM shrinker.
  1507	 */
  1508	void ttm_pool_mgr_fini(void)
  1509	{
> 1510	    if (backup_fault_inject.dname) {

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