[PATCH] adfs: bound the F+ directory entry name length

adfs_fplus_getnext() copies a directory entry name into
struct object_info::name, which is a fixed char[ADFS_MAX_NAME_LEN]
(260 bytes) living on adfs_fplus_iterate()'s stack:

	obj->name_len = le32_to_cpu(bde.bigdirobnamelen);

	offset = adfs_fplus_offset(h, le32_to_cpu(h->bigdirentries));
	offset += le32_to_cpu(bde.bigdirobnameptr);

	ret = adfs_dir_copyfrom(obj->name, dir, offset, obj->name_len);

bigdirobnamelen is a raw on-disk __le32 and is not checked against
anything -- not against the 260-byte destination, not against
ADFS_FPLUS_NAME_LEN (255), not even against bigdirnamesize.
adfs_fplus_validate_header() checks bigdirnamelen, bigdirnamesize and
bigdirentries, but nothing validates the per-entry name length.

adfs_dir_copyfrom() does not save us either.  It bounds only the source,
and even that bound is incomplete: the test

	if (index + (remain < len) >= dir->nr_buffers)
		return -EINVAL;

covers at most one extra buffer, while the tail

	memcpy(dst, dir->bhs[index]->b_data + offset, len);

is not capped at the remaining size of that buffer.  So a large
bigdirobnamelen is an out-of-bounds read of the directory buffer_heads
*and* a linear out-of-bounds write of the same, attacker-chosen, 32-bit
length past object_info::name on the kernel stack.

adfs_object_fixup() then makes it slightly worse: it loops over the same
name_len, and with the "ftsuffix" mount option appends four more bytes at
obj->name[obj->name_len++], which overflows even for name_len == 260.

Reject a name length that cannot fit, and while here make
adfs_dir_copyfrom() check the whole requested range against the
directory's buffers and copy through a loop, so no single memcpy can run
off the end of a buffer_head.

Mounting a crafted image requires CAP_SYS_ADMIN in the initial user
namespace -- ADFS is FS_REQUIRES_DEV and not FS_USERNS_MOUNT -- so this
is not reachable by a plain unprivileged local user.  It matters for the
usual untrusted-media paths: automounted removable media, disk images
handed to a privileged mounting agent (container/VM image tooling), and
forensic or CI systems that mount images to inspect them.  After the
mount, the trigger is a single getdents64() on the root directory, i.e.
"ls /mnt".

  losetup /dev/loop0 adfs-w4.img          # F+ image, bigdirobnamelen=300
  mount -t adfs -o ro /dev/loop0 /mnt
  ls -la /mnt

  BUG: KASAN: stack-out-of-bounds in adfs_dir_copyfrom+0xcc/0x150
  Write of size 300 at addr ffff80008ac6799c by task ls/132
   __asan_memcpy+0x54/0xa0
   adfs_dir_copyfrom+0xcc/0x150
   adfs_fplus_getnext+0x200/0x240
   adfs_fplus_iterate+0x144/0x1b8
   adfs_iterate+0x12c/0x2c0
   iterate_dir+0x12c/0x400
   __arm64_sys_getdents64+0xf0/0x230

followed by cascading stack-out-of-bounds reports in adfs_object_fixup()
and filldir64() walking the smashed frame.  300 is only the smallest
value that demonstrates it; the field is 32 bits wide.

Valid F+ directories are unaffected: RISC OS caps an F+ object name at
ADFS_FPLUS_NAME_LEN (255) bytes, which still leaves room for the four
byte ",xyz" filetype suffix inside ADFS_MAX_NAME_LEN, and the reworked
adfs_dir_copyfrom() copies exactly the same bytes as before for any
request that was in bounds.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: [email protected]
Signed-off-by: Yuejie Shi <[email protected]>
---
 fs/adfs/dir.c       | 15 +++++++++------
 fs/adfs/dir_fplus.c |  2 ++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/adfs/dir.c b/fs/adfs/dir.c
index 11afa9e157aa..44953247acad 100644
--- a/fs/adfs/dir.c
+++ b/fs/adfs/dir.c
@@ -19,15 +19,20 @@ int adfs_dir_copyfrom(void *dst, struct adfs_dir *dir,
 		      size_t len)
 {
 	struct super_block *sb = dir->sb;
+	size_t size = (size_t)dir->nr_buffers << sb->s_blocksize_bits;
 	unsigned int index, remain;
 
+	if (offset >= size || len > size - offset)
+		return -EINVAL;
+
 	index = offset >> sb->s_blocksize_bits;
 	offset &= sb->s_blocksize - 1;
-	remain = sb->s_blocksize - offset;
-	if (index + (remain < len) >= dir->nr_buffers)
-		return -EINVAL;
 
-	if (remain < len) {
+	while (len) {
+		remain = sb->s_blocksize - offset;
+		if (remain > len)
+			remain = len;
+
 		memcpy(dst, dir->bhs[index]->b_data + offset, remain);
 		dst += remain;
 		len -= remain;
@@ -35,8 +40,6 @@ int adfs_dir_copyfrom(void *dst, struct adfs_dir *dir,
 		offset = 0;
 	}
 
-	memcpy(dst, dir->bhs[index]->b_data + offset, len);
-
 	return 0;
 }
 
diff --git a/fs/adfs/dir_fplus.c b/fs/adfs/dir_fplus.c
index 4a15924014da..517ffcc91429 100644
--- a/fs/adfs/dir_fplus.c
+++ b/fs/adfs/dir_fplus.c
@@ -192,6 +192,8 @@ adfs_fplus_getnext(struct adfs_dir *dir, struct object
 	obj->indaddr  = le32_to_cpu(bde.bigdirindaddr);
 	obj->attr     = le32_to_cpu(bde.bigdirattr);
 	obj->name_len = le32_to_cpu(bde.bigdirobnamelen);
+	if (obj->name_len > ADFS_FPLUS_NAME_LEN)
+		return -EIO;
 
 	offset = adfs_fplus_offset(h, le32_to_cpu(h->bigdirentries));
 	offset += le32_to_cpu(bde.bigdirobnameptr);
--
2.51.0