Skip to contentSkip to conversation

fs/qnx4: fix bh leak and extent-count OOB read in qnx4_block_map()

PATCH New Bryam Vargas 1 participant 3 messages Jun 6

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover fs/qnx4: fix bh leak and extent-count OOB read in qnx4_block_map()

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

Signature · 1 line
2.43.0
#1 lore raw

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

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")
---
 fs/qnx4/inode.c | 1 +
 1 file changed, 1 insertion(+)
fs/qnx4/inode.c +1 -0
index 4deb0eeadbde..3828ba7d4492 100644
@@ -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;
 				}
 			}
Signature · 1 line
2.43.0
#2 lore raw

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

In reply to 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")

---
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(+)
fs/qnx4/inode.c +6 -0
index 3828ba7d4492..f244e197e2df 100644
@@ -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) {
Signature · 1 line
2.43.0