Re: [PATCH v2 4/4] mm: page_table_check: Ensure user pages are not slab pages
From: David Hildenbrand
Date: Tue May 16 2023 - 08:55:23 EST
On 16.05.23 13:51, Ruihan Li wrote:
On Mon, May 15, 2023 at 12:28:54PM -0400, Pasha Tatashin wrote:
On Mon, May 15, 2023 at 9:10 AM Ruihan Li <lrh2000@xxxxxxxxxx> wrote:
The current uses of PageAnon in page table check functions can lead to
type confusion bugs between struct page and slab [1], if slab pages are
accidentally mapped into the user space. This is because slab reuses the
bits in struct page to store its internal states, which renders PageAnon
ineffective on slab pages.
Since slab pages are not expected to be mapped into the user space, this
patch adds BUG_ON(PageSlab(page)) checks to make sure that slab pages
are not inadvertently mapped. Otherwise, there must be some bugs in the
kernel.
Reported-by: syzbot+fcf1a817ceb50935ce99@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://lore.kernel.org/lkml/000000000000258e5e05fae79fc1@xxxxxxxxxx/ [1]
Fixes: df4e817b7108 ("mm: page table check")
Cc: <stable@xxxxxxxxxxxxxxx> # 5.17
Signed-off-by: Ruihan Li <lrh2000@xxxxxxxxxx>
Acked-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
I would also update order in mm/memory.c
static int validate_page_before_insert(struct page *page)
{
if (PageAnon(page) || PageSlab(page) || page_has_type(page))
It is not strictly a bug there, as it works by accident, but
PageSlab() should go before PageAnon(), because without checking if
this is PageSlab() we should not be testing for PageAnon().
Right. Perhaps it would be better to send another patch for this
separately.
Probably not really worth it IMHO. With PageSlab() we might have
PageAnon() false-positives. Either will take the same path here ...
On a related note, stable_page_flags() checks PageKsm()/PageAnon()
without caring about PageSlab().
At least it's just a debugging interface and will indicate KPF_SLAB in
any case as well ...
--
Thanks,
David / dhildenb