Re: [RFC] mm/huge_memory: prevent potential NULL pointer dereference

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Wed, Jul 16, 2025 at 09:05:14PM +0200, Antonio Quartulli wrote:
> Hi Lorenzo,
>
> On 16/07/2025 17:07, Lorenzo Stoakes wrote:
> > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > > index 389620c65a5f..d649026db95a 100644
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -3802,7 +3802,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> > >
> > >   			/* Some pages can be beyond EOF: drop them from cache */
> > >   			if (new_folio->index >= end) {
> >
> > It's kind of not _hugely_ clear but, if anon - which implies no mapping - we set
> > end == -1.
> >
> > At which point this condition cannot evaluate true (index is at page granularity
> > so even MAX_UINT64 would be page shifted and still not equal -1).
>
> I may be missing something, but why can't "index >= -1" be true?

These are unsigned long's, -1 in two's complement this means -1 translates
to the maximum possible unsigned long. Index cannot == the maximum number
as this is a _page_ index so a folio would need to reference a page larger
than could be represented in a 64-bit system for that to be so.

>
> In any case, with Zi's patch the NULL check comes before the rest so the
> problem does not exist anymore there.
>
> Regards,
>
> >
> > Under all other circumstances, mapping will be non-NULL.
> >
> > > -				if (shmem_mapping(mapping))
> > > +				if (mapping && shmem_mapping(mapping))
> > >   					nr_shmem_dropped += folio_nr_pages(new_folio);
> > >   				else if (folio_test_clear_dirty(new_folio))
> > >   					folio_account_cleaned(
> > > --
> > > 2.49.1
> > >
> > >
> >
> > Cheers, Lorenzo
>
> --
> Antonio Quartulli
>
> CEO and Co-Founder
> Mandelbit Srl
> https://www.mandelbit.com
>
>