Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Silent memory leak of folio private data due to missing check for 
leaked `folio->private` data when a head page is freed.
--

commit ebe4dd474a496c46897e3f790e03a43cc0508eaf
Author: Zi Yan <[email protected]>

mm/page-flags: remove PG_private

folio->private != NULL indicates a folio carries private data, replacing
PG_private. All PG_private users are converted. Remove PG_private and
reserve the space as PG_folio for future use. Unused PG_private functions
are removed too.

> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 08988877331ba..10f4ad075ea48 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h

[ ... ]

> @@ -105,7 +101,7 @@ enum pageflags {
>       PG_owner_2,             /* Owner use. If pagecache, fs may use */
>       PG_arch_1,
>       PG_reserved,
> -     PG_private,             /* If pagecache, has fs-private data */
> +     PG_folio,               /* Do not use: reserved for folio 
> identification */

[Severity: High]
With the removal of PG_private, does the free path for head pages fail to
validate whether private data is leaked?

Previously, the memory management subsystem checked for PG_private during page
freeing (via PAGE_FLAGS_CHECK_AT_FREE) to catch filesystem leaks, and issued a
bad_page() warning if it was still set. 

While tail pages are explicitly checked for leaked private data in the free
path:

        if (tail_page->private) {
                bad_page(tail_page, "nonzero private");
                ...

The head page's private field appears to be unconditionally zeroed without
validation in __free_pages_prepare() in mm/page_alloc.c:

        page_cpupid_reset_last(page);
        page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
        page->private = 0;
        reset_page_owner(page, order);

Can this silently mask bugs where filesystems leak private data (like an
un-detached buffer_head)? Should we add a check for page->private != NULL
for head pages before unconditionally zeroing it?

>       PG_private_2,           /* If pagecache, has fs aux data */
>       PG_reclaim,             /* To be reclaimed asap */
>       PG_swapbacked,          /* Page is backed by RAM/swap */

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=16

Reply via email to