[PATCH 6.1.y 0/2] Fix NULL folio dereferences from IS_ERR() checks in 6.1.191

[PATCH 6.1.y 0/2] Fix NULL folio dereferences from IS_ERR() checks in 6.1.191

[Elliot Berman]

We hit a kernel oops in 6.1.191 in ext4_mb_regular_allocator(), When
ext4's s_buddy_cache pages are evicted, the next block allocation
dereferences a NULL folio.

$ echo 3 > /proc/sys/vm/drop_caches
$ dd if=/dev/zero of=/mnt/b0 bs=1M count=4 conv=fsync
BUG: kernel NULL pointer dereference, address: 0000000000000000

PGD 5f19067 P4D 0
Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 908 Comm: dd Not tainted 6.1.191 #1-NixOS
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
RIP: 0010:ext4_mb_load_buddy_gfp (fs/ext4/mballoc.c:1536 (discriminator 1)) ext4
Call Trace:
 <TASK>
ext4_mb_regular_allocator (fs/ext4/mballoc.c:1641 fs/ext4/mballoc.c:2798) ext4
ext4_mb_new_blocks (fs/ext4/mballoc.c:5793) ext4
 ? ext4_find_extent (fs/ext4/extents.c:925) ext4
ext4_ext_map_blocks (fs/ext4/extents.c:4369) ext4
   ...

The cause is a mismatch in these backports first released in 6.1.191:
    d34e4fd4e73f ("ext4: convert bd_bitmap_page to bd_bitmap_folio")
    862483175d3d ("ext4: convert bd_buddy_page to bd_buddy_folio")

The ext4 mballoc folio conversions were written for v6.4 and later, where
__filemap_get_folio() returns ERR_PTR() on failure. 6.1 does not have 
commit 66dabbb65d67 ("mm: return an ERR_PTR from __filemap_get_folio") and
returns NULL instead. The backports should check for NULL and not IS_ERR.

These came in as dependencies of a3a7d360252c ("ext4: fix e4b bitmap
inconsistency reports"), also new in 6.1.191. We could revert all 3
patches and translate Yongjian's patch from `folio_test_locked(folio)`
-> `PageLocked(page)` instead. I suspect backporting 66dabbb65d67 ("mm:
return an ERR_PTR from __filemap_get_folio") is too heavy for stable,
since it requires updating all callers.

Since the same mistake could exist elsewhere, I ran a Coccinelle script
over the tree to flag similar missed backports. Besides ext4, I found one
more case in ntfs3:
    95b917a14b15 ("ntfs: Do not overwrite uptodate pages")

That one needs a folio allocation failure while reading a compressed
file, so it required crafting a fault injection in __filemap_add_folio to
reproduce. I've fixed this one as well.

Reported-by: Tom Bereknyei <tomberek@gmail.com>
Signed-off-by: Elliot Berman <elliotjb@elliotjb.com>
---
Elliot Berman (2):
      ext4: fix NULL folio dereference in mballoc
      bs/ntfs3: fix NULL folio dereference in ntfs_lock_new_page()

 fs/ext4/mballoc.c  | 28 ++++++++++++++--------------
 fs/ntfs3/frecord.c |  4 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)
---
base-commit: 08889840163ebcdb7e24bf7ec311bc2e6fc3aaea
change-id: 20261101-ext4-ntfs3-null-folio-23da60d4940e

Best regards,
--  
Elliot Berman <elliotjb@elliotjb.com>

[PATCH 6.1.y 1/2] ext4: fix NULL folio dereference in mballoc

[Elliot Berman]

In 6.1, __filemap_get_folio() returns NULL on failure. The backported
folio conversion of mballoc keeps the original IS_ERR() checks, so they
are always false here. The fast-path lookup in ext4_mb_load_buddy_gfp()
has no FGP_CREAT and returns NULL whenever the bitmap or buddy page of
an initialised group has been evicted from the page cache, e.g. by
drop_caches or memory pressure. The next block allocation from that
group then oopses under ext4_mb_regular_allocator().

$ echo 3 > /proc/sys/vm/drop_caches, in 0.51 seconds)
$ dd if=/dev/zero of=/mnt/b0 bs=1M count=4 conv=fsync
BUG: kernel NULL pointer dereference, address: 0000000000000000
PGD 5f19067 P4D 0
Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 908 Comm: dd Not tainted 6.1.191 #1-NixOS
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
RIP: 0010:ext4_mb_load_buddy_gfp (fs/ext4/mballoc.c:1536 (discriminator 1)) ext4
Call Trace:
 <TASK>
ext4_mb_regular_allocator (fs/ext4/mballoc.c:1641 fs/ext4/mballoc.c:2798) ext4
ext4_mb_new_blocks (fs/ext4/mballoc.c:5793) ext4
 ? ext4_find_extent (fs/ext4/extents.c:925) ext4
ext4_ext_map_blocks (fs/ext4/extents.c:4369) ext4
   ...

Check for NULL instead and return -ENOMEM, as failures did before the
conversion.

Fixes: d34e4fd4e73f ("ext4: convert bd_bitmap_page to bd_bitmap_folio")
Fixes: 862483175d3d ("ext4: convert bd_buddy_page to bd_buddy_folio")
Reported-by: Tom Bereknyei <tomberek@gmail.com>
Signed-off-by: Elliot Berman <elliotjb@elliotjb.com>
---
 fs/ext4/mballoc.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index e77d56c533c0..d0b276ce9401 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1375,8 +1375,8 @@ static int ext4_mb_get_buddy_page_lock(struct super_block *sb,
 	poff = block % blocks_per_page;
 	folio = __filemap_get_folio(inode->i_mapping, pnum,
 			FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
-	if (IS_ERR(folio))
-		return PTR_ERR(folio);
+	if (!folio)
+		return -ENOMEM;
 	BUG_ON(folio->mapping != inode->i_mapping);
 	e4b->bd_bitmap_folio = folio;
 	e4b->bd_bitmap = folio_address(folio) + (poff * sb->s_blocksize);
@@ -1389,8 +1389,8 @@ static int ext4_mb_get_buddy_page_lock(struct super_block *sb,
 	/* blocks_per_page == 1, hence we need another page for the buddy */
 	folio = __filemap_get_folio(inode->i_mapping, block + 1,
 			FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
-	if (IS_ERR(folio))
-		return PTR_ERR(folio);
+	if (!folio)
+		return -ENOMEM;
 	BUG_ON(folio->mapping != inode->i_mapping);
 	e4b->bd_buddy_folio = folio;
 	return 0;
@@ -1533,7 +1533,7 @@ 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 (!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
@@ -1543,11 +1543,11 @@ 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 (folio)
 			folio_put(folio);
 		folio = __filemap_get_folio(inode->i_mapping, pnum,
 				FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
-		if (!IS_ERR(folio)) {
+		if (folio) {
 			if (WARN_RATELIMIT(folio->mapping != inode->i_mapping,
 	"ext4: bitmap's mapping != inode->i_mapping\n")) {
 				/* should never happen */
@@ -1567,8 +1567,8 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
 			folio_unlock(folio);
 		}
 	}
-	if (IS_ERR(folio)) {
-		ret = PTR_ERR(folio);
+	if (!folio) {
+		ret = -ENOMEM;
 		goto err;
 	}
 	if (!folio_test_uptodate(folio)) {
@@ -1585,12 +1585,12 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
 	poff = block % blocks_per_page;
 
 	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 (!folio || !folio_test_uptodate(folio) || folio_test_locked(folio)) {
+		if (folio)
 			folio_put(folio);
 		folio = __filemap_get_folio(inode->i_mapping, pnum,
 				FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
-		if (!IS_ERR(folio)) {
+		if (folio) {
 			if (WARN_RATELIMIT(folio->mapping != inode->i_mapping,
 	"ext4: buddy bitmap's mapping != inode->i_mapping\n")) {
 				/* should never happen */
@@ -1609,8 +1609,8 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
 			folio_unlock(folio);
 		}
 	}
-	if (IS_ERR(folio)) {
-		ret = PTR_ERR(folio);
+	if (!folio) {
+		ret = -ENOMEM;
 		goto err;
 	}
 	if (!folio_test_uptodate(folio)) {

-- 
2.54.0

[PATCH 6.1.y 2/2] bs/ntfs3: fix NULL folio dereference in ntfs_lock_new_page()

[Elliot Berman]

In 6.1, __filemap_get_folio() returns NULL on failure.  The backport of
commit 95b917a14b15 ("ntfs: Do not overwrite uptodate pages") keeps the
original IS_ERR() check, which is always false here, so a failed folio
allocation while reading a compressed file dereferences NULL in
folio_test_uptodate(). Check for NULL and return ERR_PTR(-ENOMEM).

Fixes: 95b917a14b15 ("ntfs: Do not overwrite uptodate pages")
Signed-off-by: Elliot Berman <elliotjb@elliotjb.com>
---
 fs/ntfs3/frecord.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index a44b7338621e..fa443144f6d1 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -2172,8 +2172,8 @@ static struct page *ntfs_lock_new_page(struct address_space *mapping,
 			FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
 	struct page *page;
 
-	if (IS_ERR(folio))
-		return ERR_CAST(folio);
+	if (!folio)
+		return ERR_PTR(-ENOMEM);
 
 	if (!folio_test_uptodate(folio))
 		return folio_file_page(folio, index);

-- 
2.54.0

Re: [PATCH 6.1.y 2/2] bs/ntfs3: fix NULL folio dereference in ntfs_lock_new_page()

[sashiko-bot]

> In 6.1, __filemap_get_folio() returns NULL on failure.  The backport of
> commit 95b917a14b15 ("ntfs: Do not overwrite uptodate pages") keeps the
> original IS_ERR() check, which is always false here, so a failed folio
> allocation while reading a compressed file dereferences NULL in
> folio_test_uptodate(). Check for NULL and return ERR_PTR(-ENOMEM).
> 
> Fixes: 95b917a14b15 ("ntfs: Do not overwrite uptodate pages")
> Signed-off-by: Elliot Berman <elliotjb@elliotjb.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-ext4-ntfs3-null-folio-v1-0-4fd91ee23a4a@elliotjb.com?part=2

Re: [PATCH 6.1.y 1/2] ext4: fix NULL folio dereference in mballoc

[sashiko-bot]

> In 6.1, __filemap_get_folio() returns NULL on failure. The backported
> folio conversion of mballoc keeps the original IS_ERR() checks, so they
> are always false here. The fast-path lookup in ext4_mb_load_buddy_gfp()
> has no FGP_CREAT and returns NULL whenever the bitmap or buddy page of
> an initialised group has been evicted from the page cache, e.g. by
> drop_caches or memory pressure. The next block allocation from that
> group then oopses under ext4_mb_regular_allocator().
> 
> $ echo 3 > /proc/sys/vm/drop_caches, in 0.51 seconds)
> $ dd if=/dev/zero of=/mnt/b0 bs=1M count=4 conv=fsync
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> PGD 5f19067 P4D 0
> Oops: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 0 PID: 908 Comm: dd Not tainted 6.1.191 #1-NixOS
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
> [ ... ]
> 
> Fixes: d34e4fd4e73f ("ext4: convert bd_bitmap_page to bd_bitmap_folio")
> Fixes: 862483175d3d ("ext4: convert bd_buddy_page to bd_buddy_folio")
> Reported-by: Tom Bereknyei <tomberek@gmail.com>
> Signed-off-by: Elliot Berman <elliotjb@elliotjb.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-ext4-ntfs3-null-folio-v1-0-4fd91ee23a4a@elliotjb.com?part=1

Re: [PATCH 6.1.y 0/2] Fix NULL folio dereferences from IS_ERR() checks in 6.1.191

[Naman Jain]

> Elliot Berman <elliotjb@elliotjb.com>
>
> We hit a kernel oops in 6.1.191 in ext4_mb_regular_allocator(), When
> ext4's s_buddy_cache pages are evicted, the next block allocation
> dereferences a NULL folio.
> The cause is a mismatch in these backports first released in 6.1.191:
>     d34e4fd4e73f ("ext4: convert bd_bitmap_page to bd_bitmap_folio")
>     862483175d3d ("ext4: convert bd_buddy_page to bd_buddy_folio")
>
> The ext4 mballoc folio conversions were written for v6.4 and later,
> where __filemap_get_folio() returns ERR_PTR() on failure. 6.1 does
> not have commit 66dabbb65d67 ("mm: return an ERR_PTR from
> __filemap_get_folio") and returns NULL instead. The backports should
> check for NULL and not IS_ERR.
> Since the same mistake could exist elsewhere, I ran a Coccinelle script
> over the tree to flag similar missed backports. Besides ext4, I found
> one more case in ntfs3:
>     95b917a14b15 ("ntfs: Do not overwrite uptodate pages")
>
> That one needs a folio allocation failure while reading a compressed
> file, so it required crafting a fault injection in
> __filemap_add_folio to reproduce. I've fixed this one as well.

I actually sent a similar patch a little later to fix the same issue :)
We can now ignore that. For ntfs3, there is already a patch fixing the
same issue:

https://lore.kernel.org/all/20261029031815.2783998-1-cmllamas@google.com/

For the series:
Reviewed-by: Naman Jain <namjain@linux.microsoft.com>

Regards,
Naman Jain