[PATCH 0/2] fs/qnx4: fix bh leak and extent-count OOB read in qnx4_block_map()

[PATCH 0/2] fs/qnx4: fix bh leak and extent-count OOB read in qnx4_block_map()

[Bryam Vargas]

While reviewing qnx4_block_map() I found two issues in how it handles a
freshly sb_bread()'d extent block (struct qnx4_xblk):

  1/2: the "IamXblk" signature-mismatch error path returns without
       releasing the buffer head (a leak on every malformed extent block);

  2/2: the per-block extent count xblk_num_xtnts (on-disk u8, up to 255)
       is used as the walk's loop bound but is never checked against the
       fixed QNX4_MAX_XTNTS_PER_XBLK (60) array size, so a crafted image
       can make the walk read past xblk_xtnts[60] / past the 512-byte
       extent block (CWE-125 out-of-bounds read).

Both are reachable only by mounting a crafted qnx4 image (mounting needs
CAP_SYS_ADMIN; qnx4 is not unprivileged-userns mountable), so the practical
impact is robustness/hardening: a buffer-head leak and a bounded read past
the extent block. Patch 2 is the security-relevant one.

The OOB read was confirmed with KASAN (the on-disk block is 512 bytes;
reproduced on a kmalloc(512) copy of the walk -> "slab-out-of-bounds Read
4 bytes to the right of the 512-byte region"; a live mount packs the 512B
block in a 4096B page-cache page, which hides the over-read from KASAN
there) and with an ABI-invariant (-m64/-m32) AddressSanitizer extraction.
Both are fixed by rejecting the malformed block early.

Bryam Vargas (2):
  fs/qnx4: release the buffer head on an invalid extent-block signature
  fs/qnx4: validate the extent count before walking the extent block

 fs/qnx4/inode.c | 7 +++++++
 1 file changed, 7 insertions(+)


base-commit: 43370e89f7a896a583bf33d1cd171d02630e61bf
-- 
2.43.0

[PATCH 1/2] fs/qnx4: release the buffer head on an invalid extent-block signature

[Bryam Vargas]

In qnx4_block_map(), after sb_bread() reads an extent block, the
"IamXblk" signature-mismatch path returns -EIO without releasing the
buffer head, leaking it on every malformed extent block. Release it
before returning.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 fs/qnx4/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 4deb0eeadbde..3828ba7d4492 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -109,6 +109,7 @@ unsigned long qnx4_block_map( struct inode *inode, long iblock )
 				xblk = (struct qnx4_xblk*)bh->b_data;
 				if ( memcmp( xblk->xblk_signature, "IamXblk", 7 ) ) {
 					QNX4DEBUG((KERN_ERR "qnx4: block at %ld is not a valid xtnt\n", qnx4_inode->i_xblk));
+					brelse(bh);
 					return -EIO;
 				}
 			}
-- 
2.43.0

[PATCH 2/2] fs/qnx4: validate the extent count before walking the extent block

[Bryam Vargas]

qnx4_block_map() follows the on-disk extent chain. For each extent block
(struct qnx4_xblk, one 512-byte QNX4 block) it walks xblk->xblk_xtnts[ix]
using xblk->xblk_num_xtnts as the loop bound:

	block = try_extent(&xblk->xblk_xtnts[ix], &offset);
	...
	if (++ix >= xblk->xblk_num_xtnts) { ... ix = 0; }

xblk_xtnts[] is a fixed QNX4_MAX_XTNTS_PER_XBLK (60) array, but
xblk_num_xtnts is an on-disk u8 (up to 255) that is never validated. A
crafted image with xblk_num_xtnts > 60 makes ix walk past xblk_xtnts[60],
reading past the end of the 512-byte extent block (an out-of-bounds read
of the buffer_head data).

Reject an extent count larger than the array, right after the "IamXblk"
signature check.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Everything below the --- is dropped by git am.

Class / impact: CWE-125 out-of-bounds read (root CWE-129). Reachable only
by mounting a crafted qnx4 image (CAP_SYS_ADMIN; not unprivileged-userns
mountable) and reading a file with a long extent chain. Read-only, no write
primitive; on a live mount the 512-byte block shares a 4096-byte page-cache
page so the read lands in adjacent in-page data of the same image -> a
robustness/hardening fix.

Affected: present since fs/qnx4/inode.c entered git at v2.6.12-rc2 (the
extent walk predates git). Verified at mainline v7.1-rc6 and stable
v6.12.92.

Reproducer: a qnx4 image whose file uses an indirect extent block
("IamXblk") with xblk_num_xtnts > 60 (e.g. 255); mount and read that file.

A/B verification (CONFIG_KASAN_GENERIC=y, kasan.fault=report, x86-64). The
on-disk extent block is exactly 512 bytes; reproduced on a kmalloc(512)
copy of the walk so KASAN sees the boundary:
  - Without this patch (xblk_num_xtnts = 255):
      BUG: KASAN: slab-out-of-bounds in <qnx4 xtnt walk>
      Read of size 4 ... located 4 bytes to the right of the 512-byte
        region (cache kmalloc-512)
  - Control (xblk_num_xtnts = 5): no report.
  - With this patch (xblk_num_xtnts = 255): no report, block rejected -EIO.
  Also an ABI-invariant AddressSanitizer extraction (-m64 and -m32):
  heap-buffer-overflow read without the patch, clean with it.

 fs/qnx4/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 3828ba7d4492..f244e197e2df 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -112,6 +112,12 @@ unsigned long qnx4_block_map( struct inode *inode, long iblock )
 					brelse(bh);
 					return -EIO;
 				}
+				if (xblk->xblk_num_xtnts > QNX4_MAX_XTNTS_PER_XBLK) {
+					QNX4DEBUG((KERN_ERR "qnx4: bad xtnt count %u\n",
+						   xblk->xblk_num_xtnts));
+					brelse(bh);
+					return -EIO;
+				}
 			}
 			block = try_extent(&xblk->xblk_xtnts[ix], &offset);
 			if (block) {
-- 
2.43.0