Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] mm: silent loss of memory leak detection for page->private on head pages at free time - [Medium] mm: removal of KPF_PRIVATE from /proc/kpageflags breaks userspace ABI --
commit 11221906706fcf50f2069964f35b9ce234eeabe6 Author: Zi Yan <[email protected]> mm/page-flags: check page/folio->private instead of PG_private After the changes of the prior commits, page/folio->private != NULL is now equivalent to checking PG_private. Stop checking PG_private on pages and folios and use page/folio->private instead, except swapcache and hugetlb. > diff --git a/fs/proc/page.c b/fs/proc/page.c > index 260772b20bd99..f90e1030825e9 100644 > --- a/fs/proc/page.c > +++ b/fs/proc/page.c > @@ -232,7 +232,6 @@ u64 stable_page_flags(const struct page *page) > > u |= kpf_copy_bit(k, KPF_RESERVED, PG_reserved); > u |= kpf_copy_bit(k, KPF_OWNER_2, PG_owner_2); > - u |= kpf_copy_bit(k, KPF_PRIVATE, PG_private); [Severity: Medium] Does removing the KPF_PRIVATE bit from stable_page_flags() break the userspace ABI? Any userspace tool or script relying on this bit to identify private pages will silently fail to detect them. Could this be preserved by computing it via folio_has_attached_private(folio) instead? [ ... ] > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index 7080a6a1a79e7..6d839f50bdcb7 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h [ ... ] > @@ -1169,7 +1191,7 @@ static __always_inline void > __ClearPageAnonExclusive(struct page *page) > */ > #define PAGE_FLAGS_CHECK_AT_FREE \ > (1UL << PG_lru | 1UL << PG_locked | \ > - 1UL << PG_private | 1UL << PG_private_2 | \ > + 1UL << PG_private_2 | \ [Severity: High] Does removing PG_private from PAGE_FLAGS_CHECK_AT_FREE mask memory leaks of filesystem private data for head pages? Any buggy filesystem or driver that frees a page with an attached page->private structure will silently leak it without a kernel warning. In mm/page_alloc.c, __free_pages_prepare() unconditionally executes: page->private = 0; Since PG_private was removed from PAGE_FLAGS_CHECK_AT_FREE, there is no longer a check that page->private was zero before this assignment, meaning the leaked pointer is zeroed out for the head page without any validation or warning. The equivalent explicit check for tail pages was preserved, indicating the loss of the head page check might be an unintended oversight when removing the flag. [ ... ] > diff --git a/tools/mm/page-types.c b/tools/mm/page-types.c > index 7fc5a8be5997f..47e4781c5fc38 100644 > --- a/tools/mm/page-types.c > +++ b/tools/mm/page-types.c [ ... ] > @@ -131,7 +130,6 @@ static const char * const page_flag_names[] = { > [KPF_RESERVED] = "r:reserved", > [KPF_MLOCKED] = "m:mlocked", > [KPF_OWNER_2] = "d:owner_2", > - [KPF_PRIVATE] = "P:private", > [KPF_PRIVATE_2] = "p:private_2", [Severity: Medium] Does removing the flag name from page_flag_names in page-types.c break parsing tools relying on the P:private designation? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
