Re: [RFC PATCH 1/3] mm/huge_memory: prevent NULL pointer dereference in try_folio_split_to_order()

[David Hildenbrand (Red Hat)]

On 11/20/25 15:41, Zi Yan wrote:
> On 20 Nov 2025, at 4:25, David Hildenbrand (Red Hat) wrote:
>
> > On 11/20/25 04:59, Zi Yan wrote:
> > > folio_split_supported() used in try_folio_split_to_order() requires
> > > folio->mapping to be non NULL, but current try_folio_split_to_order() does
> > > not check it. Add the check to prevent NULL pointer dereference.
> > >
> > > There is no issue in the current code, since try_folio_split_to_order() is
> > > only used in truncate_inode_partial_folio(), where folio->mapping is not
> > > NULL.
> > >
> > > Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
> > > ---
> > >    include/linux/huge_mm.h | 7 +++++++
> > >    1 file changed, 7 insertions(+)
> > >
> > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > > index 1d439de1ca2c..0d55354e3a34 100644
> > > --- a/include/linux/huge_mm.h
> > > +++ b/include/linux/huge_mm.h
> > > @@ -407,6 +407,13 @@ static inline int split_huge_page_to_order(struct page *page, unsigned int new_o
> > >    static inline int try_folio_split_to_order(struct folio *folio,
> > >    		struct page *page, unsigned int new_order)
> > >    {
> > > +	/*
> > > +	 * Folios that just got truncated cannot get split. Signal to the
> > > +	 * caller that there was a race.
> > > +	 */
> > > +	if (!folio_test_anon(folio) && !folio->mapping)
> > > +		return -EBUSY;
> > > +
> > >    	if (!folio_split_supported(folio, new_order, SPLIT_TYPE_NON_UNIFORM, /* warns= */ false))
> > >    		return split_huge_page_to_order(&folio->page, new_order);
> > >    	return folio_split(folio, new_order, page, NULL);
> >
> > I guess we'll take the one from Wei
> >
> > https://lkml.kernel.org/r/20251119235302.24773-1-richard.weiyang@xxxxxxxxx
> >
> > right?
>
> This is different. Wei’s fix is to __folio_split(), but mine is to
> try_folio_split_to_order(). Both call folio_split_supported(), thus
> both need the folio->mapping check.

Ah, good that I double-checked :)

> That is also my question in the cover letter on whether we should
> move folio->mapping check to folio_split_supported() and return
> error code instead of bool. Otherwise, any folio_split_supported()
> caller needs to check folio->mapping.

I think the situation with truncation (-that shmem swapcache thing, 
let's ignore that for now) is that the folio cannot be split until fully 
freed. But we don't want to return -EINVAL to the caller, the assumption 
is that the folio will soon get resolved -- folio freed -- and the 
caller will be able to make progress. So it's not really expected to be 
persistent.

-EINVAL rather signals "this cannot possibly work, so fail whatever you 
are trying".

We rather want to indicate "there was some race situation, if you try 
again later it might work or might have resolved itself".

Not sure I like returning an error from folio_split_supported(), as it's 
rather a boolean check (supported vs. not supported).

Likely we could just return "false" for truncated folios in 
folio_split_supported(), but then state that that case must be handled 
upfront.

We could provide another helper to wrap the truncation check, hmmm


BTW, I wonder if the is_huge_zero_folio() check should go into 
folio_split_supported() and just return in -EINVAL. (we shouldn't really 
trigger that). Similarly we could add a hugetlb sanity check.

--
Cheers

David