[PATCH 0/2] qnx6: stale superblock #1 use and undefined shifts

[PATCH 0/2] qnx6: stale superblock #1 use and undefined shifts

[Matthias Goergens]

These two fixes came up while testing Hui Peng's qnx6 series.  Both bugs
date back to the initial qnx6 commit.  The patches apply on top of
Hui's v2 [1]; its 3/6 changes the context of 1/2 here.  His v3 [2]
does not apply to mainline, and I have replied to it separately.

1/2: when superblock #2 is the newer one, qnx6_fill_super() still
takes the inode and long filename trees from superblock #1, after
releasing its buffer.

2/2: qnx6_block_map() shifts a 32-bit block number by 32 or more bits
for depth-5 trees with 512 byte blocks and for depth-4 and depth-5
trees with 4K blocks, all of which the driver accepts.

Tested on mainline 40288c9206c1 plus Hui's v2 1-6, in a KASAN and
UBSAN kernel under qemu with generated images:

- An image whose newer superblock #2 points at a different inode tree
  lists that tree's file with 1/2 and the older tree's file without.

- Files in depth-5 (512 byte blocks) and depth-4 and depth-5 (4K
  blocks) trees read back with the contents they were generated with,
  with 2/2.
  Without it they read back with the wrong contents, and UBSAN reports
  shift-out-of-bounds.

- Six valid images (512 byte and 4K blocks, either superblock active,
  normal and MMI layouts), a depth-4 tree with 512 byte blocks and a
  depth-3 tree with 4K blocks read back identically with and without
  both patches.

[1] https://lore.kernel.org/all/20260921042511.1473629-1-benquike@gmail.com/
[2] https://lore.kernel.org/all/20260924073920.2782917-1-benquike@gmail.com/

Matthias Goergens (2):
  qnx6: use the active superblock after choosing superblock #2
  qnx6: avoid undefined shifts in qnx6_block_map()

 fs/qnx6/inode.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)


base-commit: 40288c9206c17eb66a603262e06a58d300d0f279
prerequisite-patch-id: 46f959e08f10dc22aa50f0dcf3b666bc797e16fb
prerequisite-patch-id: d556417ccc538855b2090601ca20abd209019457
prerequisite-patch-id: 66d4e443988c07ddde054c867da5ebdbb34c2489
prerequisite-patch-id: 873f6449aef25a36fb2a66da92cc2eb4f0a23d12
prerequisite-patch-id: 51ea17033b2899bab8950a36f880154df18a4afc
prerequisite-patch-id: 0a0f17ec064751ffb6558cedfaff3be7b2a41f79
-- 
2.55.0

[PATCH 1/2] qnx6: use the active superblock after choosing superblock #2

[Matthias Goergens]

When superblock #2 has the higher serial number, qnx6_fill_super()
makes it active in sbi->sb and sbi->sb_buf and releases bh1, but keeps
using sb1, which points into bh1's data, for the Inode and Longfile
level checks and for the root nodes passed to qnx6_private_inode().

The mount therefore reads the inode and long filename trees of the older
superblock #1, through a buffer_head it no longer holds, while the rest
of its superblock metadata comes from #2.  On an image whose two
superblocks point at different inode trees, the mount lists the files of
the older tree.  brelse() only drops the reference and the buffer stays
cached, so KASAN reports nothing and the mount does not crash.

Point sb1 at superblock #2 once it has been chosen, so that all later
uses see the active superblock.  bh2 remains held through sbi->sb_buf.

Fixes: 5d026c724220 ("fs: initial qnx6fs addition")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
 fs/qnx6/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
index 916de1e3ccc4..0dfe8a3dab83 100644
--- a/fs/qnx6/inode.c
+++ b/fs/qnx6/inode.c
@@ -407,6 +407,8 @@ static int qnx6_fill_super(struct super_block *s, struct fs_context *fc)
 		sbi->sb = (struct qnx6_super_block *)bh2->b_data;
 		brelse(bh1);
 		bh1 = NULL;
+		/* the checks and root nodes below must come from #2 too */
+		sb1 = sb2;
 		pr_info("superblock #2 active\n");
 	}
 mmi_success:

base-commit: 40288c9206c17eb66a603262e06a58d300d0f279
prerequisite-patch-id: 46f959e08f10dc22aa50f0dcf3b666bc797e16fb
prerequisite-patch-id: d556417ccc538855b2090601ca20abd209019457
prerequisite-patch-id: 66d4e443988c07ddde054c867da5ebdbb34c2489
prerequisite-patch-id: 873f6449aef25a36fb2a66da92cc2eb4f0a23d12
prerequisite-patch-id: 51ea17033b2899bab8950a36f880154df18a4afc
prerequisite-patch-id: 0a0f17ec064751ffb6558cedfaff3be7b2a41f79
-- 
2.55.0

[PATCH 2/2] qnx6: avoid undefined shifts in qnx6_block_map()

[Matthias Goergens]

qnx6_block_map() takes each level's index from the 32-bit block number
by shifting it right by ptrbits * depth.  QNX6_PTR_MAX_LEVELS allows
five levels, and ptrbits is 7 for 512 byte blocks and 10 for 4K
blocks, so a depth-5 tree with 512 byte blocks shifts by 35 and a
depth-4 tree with 4K blocks by 40.  Such trees are deeper than any
32-bit block number needs, but the driver accepts them, and a shift by
32 or more is undefined.  UBSAN reports shift-out-of-bounds, and on
x86, which masks the shift count, files in such trees read back with
the wrong contents; at depth 5 with 512 byte blocks UBSAN also reports
index 16 out of range for di_block_ptr[].

The index bits at those offsets are zero for any 32-bit block number,
so use zero for them explicitly and walk the remaining levels as
before.  Casting the block number to u64 would not be enough: with 64K
blocks, which sb_set_blocksize() accepts on 64K-page kernels, ptrbits
is 14 and the shift reaches 70.

Fixes: 5d026c724220 ("fs: initial qnx6fs addition")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
 fs/qnx6/inode.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
index 0dfe8a3dab83..fd61cf27b81d 100644
--- a/fs/qnx6/inode.c
+++ b/fs/qnx6/inode.c
@@ -124,8 +124,13 @@ static unsigned qnx6_block_map(struct inode *inode, unsigned no)
 	int depth = ei->di_filelevels;
 	int i;
 
+	/*
+	 * For valid levels bitdelta can reach the width of no (e.g. 35 for
+	 * 512 byte blocks at depth 5).  Index bits beyond no are zero, and
+	 * shifting by that much would be undefined.
+	 */
 	bitdelta = ptrbits * depth;
-	levelptr = no >> bitdelta;
+	levelptr = bitdelta < BITS_PER_TYPE(no) ? no >> bitdelta : 0;
 
 	if (levelptr > QNX6_NO_DIRECT_POINTERS - 1) {
 		pr_err("Requested file block number (%u) too big.", no);
@@ -141,7 +146,8 @@ static unsigned qnx6_block_map(struct inode *inode, unsigned no)
 			return 0;
 		}
 		bitdelta -= ptrbits;
-		levelptr = (no >> bitdelta) & mask;
+		levelptr = bitdelta < BITS_PER_TYPE(no) ?
+			   (no >> bitdelta) & mask : 0;
 		ptr = ((__fs32 *)bh->b_data)[levelptr];
 
 		if (!qnx6_check_blockptr(ptr)) {
-- 
2.55.0