Re: [PATCH 1/1] ext4: fix NULL pointer dereference in ext4_mb_load_buddy_gfp()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ext4: false NULL return premise for __filemap_get_folio() in
  mainline
--

commit 26a680b6bc63194bc1c7439d73f388c607684a53
Author: Aleksandr Panfilenko <lexpasss@inbox.lv>

ext4: fix NULL pointer dereference in ext4_mb_load_buddy_gfp()

This commit replaces IS_ERR() with IS_ERR_OR_NULL() on the fast-path
__filemap_get_folio() lookups and folio_put() guards in
ext4_mb_load_buddy_gfp().

> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index a15285224cdf6..11ca5a1481752 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -1715,7 +1715,8 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
>  
>  	/* Avoid locking the folio in the fast path ... */
>  	folio = __filemap_get_folio(inode->i_mapping, pnum, FGP_ACCESSED, 0);
> -	if (IS_ERR(folio) || !folio_test_uptodate(folio) || folio_test_locked(folio)) {
> +	if (IS_ERR_OR_NULL(folio) || !folio_test_uptodate(folio) ||
> +	    folio_test_locked(folio)) {
>  		/*
>  		 * folio_test_locked is employed to detect ongoing folio
>  		 * migrations, since concurrent migrations can lead to
> @@ -1725,7 +1726,7 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
>  		 * try to get the folio with lock in both cases to avoid
>  		 * concurrency.
>  		 */
> -		if (!IS_ERR(folio))
> +		if (!IS_ERR_OR_NULL(folio))
>  			folio_put(folio);
>  		folio = __filemap_get_folio(inode->i_mapping, pnum,
>  				FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
> @@ -1774,8 +1775,9 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
>  
>  	/* we need another folio for the buddy */
>  	folio = __filemap_get_folio(inode->i_mapping, pnum, FGP_ACCESSED, 0);
> -	if (IS_ERR(folio) || !folio_test_uptodate(folio) || folio_test_locked(folio)) {
> -		if (!IS_ERR(folio))
> +	if (IS_ERR_OR_NULL(folio) || !folio_test_uptodate(folio) ||
> +	    folio_test_locked(folio)) {
> +		if (!IS_ERR_OR_NULL(folio))
>  			folio_put(folio);
>  		folio = __filemap_get_folio(inode->i_mapping, pnum,
>  				FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);

[Severity: Medium]
Can __filemap_get_folio() return NULL in mainline, or is this a 6.1.y
backport bug?

In mainline, since 66dabbb65d67 ("mm: return an ERR_PTR from
__filemap_get_folio"), __filemap_get_folio() returns ERR_PTR(-ENOENT) on a
page-cache miss and never NULL:

mm/filemap.c:__filemap_get_folio_mpol() {
	...
	if (!folio)
		return ERR_PTR(-ENOENT);
	...
}

Commits 99b150d84e49 ("ext4: convert bd_bitmap_page to bd_bitmap_folio")
and 5eea586b47f0 ("ext4: convert bd_buddy_page to bd_buddy_folio") were
written against that contract, so IS_ERR(folio) already handles cache
misses upstream.

In 6.1.y, which lacks 66dabbb65d67 ("mm: return an ERR_PTR from
__filemap_get_folio"), __filemap_get_folio() returns NULL on all failures,
including FGP_CREAT allocation failures.  In that tree, the FGP_CREAT
calls right below in ext4_mb_load_buddy_gfp() and in
ext4_mb_get_buddy_folio_lock() still only check IS_ERR(folio), so an
allocation failure under memory pressure will pass !IS_ERR(NULL) and
dereference folio->mapping.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110193711.3106773-1-lexpasss@inbox.lv?part=1