Re: [PATCH 6.18.y] Revert "mm/page_owner: fix memory leak in page_owner_stack_fops->release()"

[Mauricio Faria de Oliveira <mfo@igalia.com>]

On 2026-11-09 11:15, Hamid Nassiby wrote:
> This reverts commit 05a8edf607be826738dfc620a17d9d98e8976762, which is
> commit a76a5ae2c6c6 upstream.

Thanks for handling this and submitting a fix.

Reviewed-by: Mauricio Faria de Oliveira <mfo@igalia.com>
Tested-by: Mauricio Faria de Oliveira <mfo@igalia.com>

cheers,

>
> The upstream change is only correct on top of commit 156c0c5d1463
> ("mm/page_owner: introduce struct stack_print_ctx"), which first appeared
> in v6.19. That commit makes page_owner_stack_open() allocate a struct
> stack_print_ctx through seq_open_private() and moves the stack_list
> cursor into it, so ->release() must free it with seq_release_private().
> 6.18 does not have that commit: page_owner_stack_open() passes a size of 0
> to seq_open_private(), so there is nothing to leak, and
> stack_start()/stack_next() store the current struct stack node directly
> in m->private.
> 
> The upstream commit was backported because it is tagged for stable and
> carries a Fixes tag for 765973a09803 ("mm,page_owner: display all stacks
> and their count"), but the leak it fixes only exists since 156c0c5d1463.
> 
> seq_release_private() kfree()s m->private. Closing show_stacks before
> EOF therefore frees a struct stack node that is still linked in
> stack_list, and the next reader of show_stacks walks into freed memory.
> On v6.18.60 (x86_64 QEMU, KASAN, page_owner=on), a single 4 KiB read()
> and close() followed by a full read gives:
> 
>   BUG: KASAN: slab-use-after-free in stack_print+0x165/0x170
>   Read of size 8 at addr ff110000062adfc0 by task init/1
>   ...
>   Allocated by task 1:
>    __kasan_kmalloc+0x7f/0x90
>    __set_page_owner+0x4a5/0x720
>   ...
>   Freed by task 1:
>    kfree+0xf8/0x460
>    seq_release_private+0x58/0x110
>    full_proxy_release+0xa1/0x130
>    __fput+0x35c/0xac0
>    __x64_sys_close+0x79/0xd0
>   ...
>   The buggy address belongs to the object at ff110000062adfc0
>    which belongs to the cache kmalloc-16 of size 16
> 
> Without KASAN, the same bug panics arm64 hardware running a
> 6.12.108-based kernel ("head -12 show_stacks; cat show_stacks").
> 
> With this revert applied, the QEMU reproducer completes 20
> partial-read/full-read cycles without a KASAN report. mm/page_owner.c is
> unchanged between v6.18.60 and v6.18.62.
> 
> Backporting 156c0c5d1463 instead is not proposed: it prepares for the
> new show_handles and show_stacks_handles debugfs files rather than
> fixing anything, so reverting the backport is the minimal change for
> stable.
> 
> Link: https://lore.kernel.org/stable/20260106170556.173097317@linuxfoundation.org/
> Signed-off-by: Hamid Nassiby <hamid.nassiby@nokia.com>
> ---
> 
> Notes:
>     The analysis, the QEMU/KASAN reproducer and this changelog were
>     prepared with AI assistance. The arm64 crash was hit on our own
>     hardware; I reviewed the analysis and confirmed the fix there, with
>     repeated partial and full reads of show_stacks on the reverted kernel.
>     
>     The same backport is in 6.12.y as 7fbea59f737e; a separate
>     [PATCH 6.12.y] revert is sent alongside this one.
> 
>  mm/page_owner.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index bc2676414..589ec37c9 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -936,7 +936,7 @@ static const struct file_operations page_owner_stack_operations = {
>  	.open		= page_owner_stack_open,
>  	.read		= seq_read,
>  	.llseek		= seq_lseek,
> -	.release	= seq_release_private,
> +	.release	= seq_release,
>  };
>  
>  static int page_owner_threshold_get(void *data, u64 *val)
> 
> base-commit: c3f63d70f236bb7685e6d11d1d9b5b351552e600

-- 
Mauricio