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

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

On Wed, Jul 16, 2025 at 04:58:04PM +0200, Antonio Quartulli wrote:
> I just found this issue in the last linux-next Coverity report and it
> caught my attention.
> I am not familiar with this code, therefore I am sending this patch
> as RFC because I am not 100% sure whether this is a false positive or
> not.
> However, it seems potentially legit to me:
>
> In __folio_split(), when looping over folios we dereference
> `mapping` before ensuring it is non-NULL.
>
> Following code in the loop body performs such check, thus
> suggesting that `mapping` may be NULL and accessing it
> without any check may be dangerous.
>
> Add NULL check before passing it to shmem_mapping().
>
> Cc: Zi Yan <ziy@nvidia.com>
> Fixes: 00527733d0dc ("mm/huge_memory: add two new (not yet used) functions for folio_split()")
> Addresses-Coverity-ID: 1647614 ("FORWARD_NULL")
> Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>

Thanks for the patch :) but sorry it is in practice not a thing that can
happen. See below for analysis.

> ---
>  mm/huge_memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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).

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