[lvc-project] [PATCH] ocfs2: validate la_size before ocfs2_clear_local_alloc()

[Dmitry Morgun]

la_size, like the dirty flag, is read from disk. If dirty != 0,
ocfs2_begin_local_alloc_recovery() is always called, which
immediately invokes ocfs2_clear_local_alloc(). At this point,
ocfs2_clear_local_alloc() uses la_size as the loop bound without
validating it first. If la_size is corrupted, the loop writes past
the end of la_bitmap and may eventually start writing into memory
that has already been freed.

BUG: KASAN: use-after-free in ocfs2_clear_local_alloc fs/ocfs2/localalloc.c:919 [inline]
BUG: KASAN: use-after-free in ocfs2_begin_local_alloc_recovery+0xb07/0xc00 fs/ocfs2/localalloc.c:515
CPU: 1 PID: 2386 Comm: syz.2.190 Not tainted 6.1.174-syzkaller-00520-g10c505401422 #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
Call Trace:
 <TASK>
 ocfs2_clear_local_alloc fs/ocfs2/localalloc.c:919 [inline]
 ocfs2_begin_local_alloc_recovery+0xb07/0xc00 fs/ocfs2/localalloc.c:515
 ocfs2_check_volume fs/ocfs2/super.c:2448 [inline]
 ocfs2_mount_volume fs/ocfs2/super.c:1819 [inline]
 ocfs2_fill_super+0x2033/0x3dc0 fs/ocfs2/super.c:1082
 mount_bdev+0x356/0x410 fs/super.c:1443
 legacy_get_tree+0x108/0x220 fs/fs_context.c:632
 vfs_get_tree+0x8e/0x300 fs/super.c:1573
 do_new_mount fs/namespace.c:3078 [inline]
 path_mount+0x6af/0x1f70 fs/namespace.c:3408
 do_mount fs/namespace.c:3421 [inline]
 __do_sys_mount fs/namespace.c:3629 [inline]
 __se_sys_mount fs/namespace.c:3606 [inline]
 __x64_sys_mount+0x283/0x300 fs/namespace.c:3606
 do_syscall_x64 arch/x86/entry/common.c:46 [inline]
 do_syscall_64+0x35/0x80 fs/namespace.c:76
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8

The validation of la_size currently exists only in
ocfs2_load_local_alloc(), but that function is called after
ocfs2_clear_local_alloc(). As a result, memory corruption occurs
before the invalid value is detected and -EINVAL is returned.

Adding the same validation to ocfs2_begin_local_alloc_recovery()
before calling ocfs2_clear_local_alloc() prevents the out-of-bounds
write by failing the recovery early with -EINVAL.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: ccd979bdbce9 ("OCFS2: The Second Oracle Cluster Filesystem")
Signed-off-by: Dmitry Morgun <d.morgun at ispras.ru>
---
A similar issue also exists in ocfs2_complete_local_alloc_recovery().
The i_total and la_bm_off fields are also read from disk and used as
loop bounds and offsets without prior validation. With a corrupted
filesystem image, they could lead to similar out-of-bounds accesses
during the completion of local alloc recovery. Therefore, a more
complete solution would be to introduce a shared validation helper
for all relevant on-disk local alloc fields (la_size, i_total,
la_bm_off, and others) and invoke it before starting the recovery
process.

This patch fixes only the reported reproducer, triggered 
by an invalid la_size. Other fields like i_total and la_bm_off are
still unvalidated, so a similarly corrupted image could trigger 
an analogous bug elsewhere. Maintainers' input would be welcome 
on whether a shared validation helper is preferred over targeted 
per-field fixes.

 fs/ocfs2/localalloc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index c4426d12a..ce03903ed 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -481,6 +481,7 @@ int ocfs2_begin_local_alloc_recovery(struct ocfs2_super *osb,
 	struct buffer_head *alloc_bh = NULL;
 	struct inode *inode = NULL;
 	struct ocfs2_dinode *alloc;
+	struct ocfs2_local_alloc *la;
 
 	trace_ocfs2_begin_local_alloc_recovery(slot_num);
 
@@ -512,6 +513,16 @@ int ocfs2_begin_local_alloc_recovery(struct ocfs2_super *osb,
 	memcpy((*alloc_copy), alloc_bh->b_data, alloc_bh->b_size);
 
 	alloc = (struct ocfs2_dinode *) alloc_bh->b_data;
+	la = OCFS2_LOCAL_ALLOC(alloc);
+
+	if ((la->la_size == 0) ||
+		(le16_to_cpu(la->la_size) > ocfs2_local_alloc_size(inode->i_sb))) {
+		mlog(ML_ERROR, "Local alloc size is invalid (la_size = %u)\n",
+			le16_to_cpu(la->la_size));
+		status = -EINVAL;
+		goto bail;
+	}
+
 	ocfs2_clear_local_alloc(alloc);
 
 	ocfs2_compute_meta_ecc(osb->sb, alloc_bh->b_data, &alloc->i_check);
-- 
2.34.1