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: stable@vger.kernel.org Signed-off-by: Yuejie Shi --- 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