On 6/28/26 17:40, Óscar Megía López wrote:
> 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 v2] drm/qxl: fix
> use-after-free to qxl_irq_handler in PCI mode" before testing.
> If you don't apply this patch, you will get a UAF error when running
> the script above.
>
> Assisted-by: OpenCode:1.17.8-Big Pickle
> Link:
> https://lore.kernel.org/virtualization/[email protected]/T/#u
> Signed-off-by: Óscar Megía López <[email protected]>
> ---
> drivers/gpu/drm/ttm/ttm_device.c | 5 ++++-
> drivers/gpu/drm/ttm/ttm_pool.c | 37 ++++++++++++++++++++++++++------
> include/drm/ttm/ttm_pool.h | 2 ++
> 3 files changed, 37 insertions(+), 7 deletions(-)
>
> 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;
> +
Please put that into a separate patch.
> 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..e0c82804a07d 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -1198,6 +1198,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);
>
> @@ -1386,6 +1397,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 +1435,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 +1448,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;
Instead of the error handling move shrinker_alloc() before initializing the
pool types.
> }
>
> /**
> @@ -1442,17 +1460,24 @@ int ttm_pool_mgr_init(unsigned long num_pages)
> * Cleanup the global pools and unregister the MM shrinker.
> */
> void ttm_pool_mgr_fini(void)
> +{
> + shrinker_free(mm_shrinker);
> + ttm_pool_type_fini_and_list_lru_destroy();
> + WARN_ON(!list_empty(&shrinker_list));
> +}
> +
> +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);
> }
> -
> - shrinker_free(mm_shrinker);
> - WARN_ON(!list_empty(&shrinker_list));
> }
> diff --git a/include/drm/ttm/ttm_pool.h b/include/drm/ttm/ttm_pool.h
> index 26ee592e1994..bda8e816a206 100644
> --- a/include/drm/ttm/ttm_pool.h
> +++ b/include/drm/ttm/ttm_pool.h
> @@ -97,4 +97,6 @@ int ttm_pool_restore_and_alloc(struct ttm_pool *pool,
> struct ttm_tt *tt,
> int ttm_pool_mgr_init(unsigned long num_pages);
> void ttm_pool_mgr_fini(void);
>
> +void ttm_pool_type_fini_and_list_lru_destroy(void);
That function doesn't need to be exported and so doesn't belong into the header.
Regards,
Christian.
> +
> #endif
> --
> 2.54.0
>
