[PATCH 0/3] affs: fix out-of-bounds bitmap access on crafted images

[PATCH 0/3] affs: fix out-of-bounds bitmap access on crafted images

[Hui Peng]

While fuzzing AFFS with KASAN I hit an out-of-bounds read roughly 8.45 MB
past sbi->s_bitmap, triggered by truncating a file on a crafted image.
Tracking it down turned up three separate problems, all dating back to
the start of git history.

The root cause is that affs_free_block() and affs_alloc_block() each
open code their own block range test, and both get it wrong in the same
two ways: neither rejects a block below s_reserved, so the subsequent
`block - s_reserved` underflows, and both use `> s_partition_size` where
the last valid block is s_partition_size - 1.

AFFS already has affs_validblock() expressing the correct range, and
affs_bread() and friends have gated on it since 2005 - so these two
functions were happily operating on blocks the rest of the filesystem
has always refused to touch. Patches 1 and 3 simply make them use it.
Patch 2 is unrelated except that I found it on the same path: the
extension block walk in affs_truncate() never checks affs_bread() for
NULL, which a crafted extension chain turns into a NULL dereference.

  1/3 is the one with the reproducer and the KASAN splat.
  2/3 is a straightforward missing NULL check.
  3/3 is the sibling of 1/3 with no reproducer - closer to hardening.

I kept them separate rather than folding 3/3 into 1/3 because the
reachability stories are quite different and I did not want the
unreproducible one to hold up the other. Equally happy to squash if you
would rather have one patch.

I also deliberately left out two extra defensive checks I had written
(a post-division `bmap >= s_bmap_count` test, and an early return when
sbi->s_bitmap is NULL); both are provably unreachable once 1/3 lands.
The reasoning is spelled out under the --- in 1/3.

Per Documentation/process/threat-model.rst these are regular bugs rather
than vulnerabilities, since mounting an image is privileged, so there is
no stable Cc and I have posted in the open.

Built with CONFIG_AFFS_FS=y, no new warnings; checkpatch clean.

Hui Peng (3):
  affs: reject out-of-range blocks in affs_free_block()
  affs: check affs_bread() return value in affs_truncate()
  affs: validate the allocation goal in affs_alloc_block()

 fs/affs/bitmap.c | 4 ++--
 fs/affs/file.c   | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/3] affs: reject out-of-range blocks in affs_free_block()

[Hui Peng]

affs_free_block() accepts any block number below or equal to
s_partition_size, then subtracts s_reserved from it:

	if (block > sbi->s_partition_size)
		goto err_range;

	blk     = block - sbi->s_reserved;
	bmap    = blk / sbi->s_bmap_bits;
	bit     = blk % sbi->s_bmap_bits;
	bm      = &sbi->s_bitmap[bmap];

Both bounds are wrong.

block is a u32 taken straight from the on-disk file header or extension
block, and nothing rejects a value below s_reserved. For block = 1 with
s_reserved = 2 the subtraction underflows to 0xffffffff, so with a 512
byte block size (s_bmap_bits = 512 * 8 - 32 = 4064) the index becomes
0xffffffff / 4064 = 1056832. sizeof(struct affs_bm_info) is 8, so
&sbi->s_bitmap[bmap] lands roughly 8.45 MB past an allocation that is
only a handful of entries long.

The upper bound is also off by one: s_partition_size is a block count,
so the last valid block is s_partition_size - 1, and a block equal to
s_partition_size is accepted today. Because s_bmap_count is
ceil((s_partition_size - s_reserved) / s_bmap_bits), that block yields
bmap == s_bmap_count exactly whenever the partition divides evenly into
bitmap blocks - a one element overrun of the same array.

Mounting a crafted AFFS image whose file header references a block below
s_reserved and truncating the file reproduces the underflow variant:

 ==================================================================
 BUG: KASAN: slab-use-after-free in affs_free_block+0x5d4/0x670
 Read of size 4 at addr ffff8881068d4c00 by task init/172

 CPU: 2 UID: 0 PID: 172 Comm: init Not tainted 7.3.0-rc3 #1
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
 Call Trace:
  <TASK>
  dump_stack_lvl+0x70/0xa0
  print_report+0x153/0x4c6
  kasan_report+0xf1/0x120
  affs_free_block+0x5d4/0x670
  affs_truncate+0x635/0x1520
  affs_setattr+0x367/0x470
  notify_change+0x941/0x1050
  do_truncate+0x1ba/0x210
  vfs_truncate+0x305/0x490
  ksys_truncate+0xd9/0x160
  __x64_sys_truncate+0x59/0x80
  do_syscall_64+0xda/0x4b0
  entry_SYSCALL_64_after_hwframe+0x77/0x7f
  </TASK>
 ==================================================================

KASAN calls it a use-after-free only because the wild address happened
to land inside an unrelated slab object that had already been freed;
the allocation and free stacks in the full report belong to a boot time
kobject_uevent_env() allocation. It is an out-of-bounds read, not a
temporal bug, and where it lands depends on the heap layout.

AFFS already has a helper that encodes the valid range, and it has done
so since the beginning of git history:

	static inline bool affs_validblock(struct super_block *sb, int block)
	{
		return(block >= AFFS_SB(sb)->s_reserved &&
		       block < AFFS_SB(sb)->s_partition_size);
	}

affs_bread(), affs_getblk(), affs_getzeroblk() and affs_getemptyblk()
all gate on it, so a block that affs_free_block() accepts today is one
that AFFS has always refused to read. Use the same helper here rather
than open coding a third variant of the test.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
The last valid block is still freeable: affs_init_bitmap() explicitly
marks every bit mapping to a block >= s_partition_size as allocated in
the final bitmap block, so s_partition_size - 1 is the highest block the
allocator can hand out and affs_validblock() accepts it.

I have left out two further checks that I had initially written, because
both are unreachable once this patch is applied and I did not want to
mix speculative hardening into a fix with a reproducer:

  - a `bmap >= sbi->s_bmap_count` test after the division. Given
    s_reserved <= block < s_partition_size we have blk <= N-1 where
    N = s_partition_size - s_reserved, and s_bmap_count = ceil(N /
    s_bmap_bits) = floor((N-1) / s_bmap_bits) + 1, so bmap is always
    <= s_bmap_count - 1.

  - an early return when sbi->s_bitmap is NULL or sbi->s_bmap_bits is 0,
    guarding the division. affs_init_bitmap() only leaves those unset on
    paths that force SB_RDONLY (including the ro->rw reconfigure path),
    and a read-only superblock cannot reach affs_truncate().

Happy to add either if you would prefer the belt and braces.

Not Cc'd to stable and posted in the open: per
Documentation/process/threat-model.rst, "bugs triggered by mounting a
corrupted or maliciously crafted file system image" are regular bugs
rather than vulnerabilities, because mounting is privileged. Say the
word if you would like it tagged for stable anyway.

Found with a QEMU/KASAN reproducer built around a crafted 4 KB AFFS
image; reproduced in six independent runs.

diff --git a/fs/affs/bitmap.c b/fs/affs/bitmap.c
--- a/fs/affs/bitmap.c
+++ b/fs/affs/bitmap.c
@@ -46,7 +46,7 @@
 
 	pr_debug("%s(%u)\n", __func__, block);
 
-	if (block > sbi->s_partition_size)
+	if (!affs_validblock(sb, block))
 		goto err_range;
 
 	blk     = block - sbi->s_reserved;
-- 
2.43.0

[PATCH 2/3] affs: check affs_bread() return value in affs_truncate()

[Hui Peng]

The extension block walk at the end of affs_truncate() does not check
the result of affs_bread():

	while (ext_key) {
		ext_bh = affs_bread(sb, ext_key);
		size = AFFS_SB(sb)->s_hashsize;
		...
			affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
		affs_free_block(sb, ext_key);
		ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);

ext_key comes from the on-disk extension chain, and affs_bread() returns
NULL for any block outside [s_reserved, s_partition_size) as well as on
a read error. AFFS_BLOCK() and AFFS_TAIL() then dereference it, so a
crafted image with an out-of-range extension pointer gives a NULL
pointer dereference while truncating.

Every other affs_bread() caller in fs/affs/amigaffs.c already checks for
NULL; this loop is the outlier. Bail out of the walk on failure.

Breaking out rather than returning keeps the affs_free_prealloc() call
at the end of the function. The remaining extension blocks are leaked in
the on-disk bitmap, which is the correct trade-off against dereferencing
NULL - the image is already corrupt at that point, and the error is
reported.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
affs_validblock() was factored out of affs_bread() by commit d5de9fd594eb
("fs/affs: add validation block function") in v4.11, but the predicate it
replaced was inline in affs_bread() since the start of git history, so
the NULL return has always been possible here.

diff --git a/fs/affs/file.c b/fs/affs/file.c
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -971,6 +971,11 @@
 
 	while (ext_key) {
 		ext_bh = affs_bread(sb, ext_key);
+		if (!ext_bh) {
+			affs_error(sb, "truncate",
+				   "Cannot read extension block %u", ext_key);
+			break;
+		}
 		size = AFFS_SB(sb)->s_hashsize;
 		if (size > blkcnt - blk)
 			size = blkcnt - blk;
-- 
2.43.0

[PATCH 3/3] affs: validate the allocation goal in affs_alloc_block()

[Hui Peng]

affs_alloc_block() applies the same too-permissive range test that
affs_free_block() did, and then performs the same arithmetic:

	if (!goal || goal > sbi->s_partition_size) {
		...
		goal = sbi->s_reserved;
	}

	blk = goal - sbi->s_reserved;
	bmap = blk / sbi->s_bmap_bits;
	bm = &sbi->s_bitmap[bmap];

	if (bm->bm_free)

A goal strictly between 0 and s_reserved passes the test, underflows the
subtraction and indexes sbi->s_bitmap far out of bounds, and a goal equal
to s_partition_size overruns it by one entry.

Unlike the free path this is not driven directly by on-disk data - goal
is derived from inode state (i_lastalloc, the last allocated block, or
0) - and I have no reproducer for it. It is the same defect in the
sibling function though, so fix it the same way, with the helper that
already defines the valid block range.

Keep the `if (goal)` guard around the warning so that a first allocation
with goal == 0, which is the normal case, stays silent.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Behaviour change worth noting: goal == 0 previously took this branch via
the `!goal` test and now takes it via affs_validblock() returning false
(0 < s_reserved for any mountable image, since the root block alone puts
s_reserved at 2). The outcome, goal = sbi->s_reserved, is identical.

No reproducer for this one - please treat it as hardening rather than a
security fix, and drop the Fixes: tag if you would rather it did not go
to stable on its own.

diff --git a/fs/affs/bitmap.c b/fs/affs/bitmap.c
--- a/fs/affs/bitmap.c
+++ b/fs/affs/bitmap.c
@@ -133,7 +133,7 @@
 		return ++AFFS_I(inode)->i_lastalloc;
 	}
 
-	if (!goal || goal > sbi->s_partition_size) {
+	if (!affs_validblock(sb, goal)) {
 		if (goal)
 			affs_warning(sb, "affs_balloc", "invalid goal %d", goal);
 		//if (!AFFS_I(inode)->i_last_block)
-- 
2.43.0