[PATCH] ocfs2: fix out-of-bounds write in ocfs2_remove_refcount_extent

[Ian Bridges <icb@fastmail.org>]

[BUG]
Unlinking a refcounted file whose refcount tree has leaf blocks
triggers a fortify panic due to an out-of-bounds write.

[CAUSE]
When the last leaf block is removed from a refcount tree,
ocfs2_remove_refcount_extent() converts the root back to leaf mode
with a bulk memset on &rb->rf_records. rf_records sits in an anonymous
union with rf_list. rf_list.l_tree_depth aliases rf_records.rl_count,
and is 0 for a single-level tree. With rl_count equal to 0, the memset
writes past the 16-byte declared size of rf_records, which the fortify
checker catches.

[FIX]
Replace the bulk memset on &rb->rf_records with a correctly-bounded
memset on rl_recs[] alone, after setting rl_count to the correct value.

Fixes: 2f26f58df041 ("ocfs2: annotate flexible array members with __counted_by_le()")
Reported-by: syzbot+3ef989aae096b30f1663@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3ef989aae096b30f1663
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
This patch contains a proposed fix for a crash reported by syzbot
in __ocfs2_decrease_refcount().

The file names and offsets in this description are from commit
7cb1c5b32a2bfde961fff8d5204526b609bcb30a from this repo:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git

I also have a small test harness that reproduces the original panic,
which I can make available as well.

The Bug

The ocfs2_refcount_list structure (fs/ocfs2/ocfs2_fs.h:935) contains
two fields relevant to this bug: rl_count, the maximum number of
refcount record slots, and rl_used, the number of occupied slots. The
flexible array member rl_recs is annotated __counted_by_le(rl_count)
(fs/ocfs2/ocfs2_fs.h:942), which instructs the compiler to use
rl_count as the runtime bound for __builtin_dynamic_object_size
queries on rl_recs when CONFIG_FORTIFY_SOURCE is enabled.

ocfs2_remove_refcount_extent() (fs/ocfs2/refcounttree.c:2066) is
called when the last refcount record is removed from a leaf block. If
that leaf was also the last one, the root is converted back from tree
mode to leaf mode. The restore path (fs/ocfs2/refcounttree.c:2131–2137)
zeros the header fields and resets the on-disk record area with a memset
on &rb->rf_records (fs/ocfs2/refcounttree.c:2134).

This is an out-of-bounds write of the rf_records struct member.
rf_records and rf_list share the same memory in an anonymous union; the
first field of rf_list, l_tree_depth, occupies the same bytes as
rf_records.rl_count. A refcount tree is always single-level, so
l_tree_depth is 0, which means rl_count reads as 0 at the memset call
site. The declared size of rf_records is therefore 16 bytes (the fixed
header with zero record slots), but the memset writes the full remainder
of the block (past the end of the struct member).

Here is a step-by-step breakdown of how the crash is triggered by
unlinking a refcounted file:

0. Inode eviction calls through to ocfs2_commit_truncate()
(fs/ocfs2/alloc.c:7241), which calls ocfs2_remove_btree_range()
(fs/ocfs2/alloc.c:5714) for each extent range. Because the extents
are refcounted, ocfs2_decrease_refcount() (fs/ocfs2/alloc.c:5800)
is called to decrement the reference count for each cluster.
1. For each cluster whose reference count reaches zero,
ocfs2_decrease_refcount_rec() (fs/ocfs2/refcounttree.c:2158)
removes the corresponding record from the leaf block.
2. When the last record is removed from a leaf block, rl_used reaches
0 and ocfs2_remove_refcount_extent() (fs/ocfs2/refcounttree.c:2203)
is called to detach it from the tree root's extent list.
3. If that was the last leaf, the root has no remaining extents and
the restore-to-leaf-mode path at fs/ocfs2/refcounttree.c:2125 is
entered.
4. The memset at fs/ocfs2/refcounttree.c:2134 writes past the
16-byte declared size of rf_records. The fortify checker reads
rl_count, which is 0 because it aliases l_tree_depth in the union,
and fires.

The Proposed Fix

Replace the bulk memset on &rb->rf_records with explicit initialisation
of each header field of ocfs2_refcount_list, followed by a
correctly-bounded memset on rl_recs[] alone. rl_count is set before the
memset so that the fortify checker can read it at runtime to verify the
write against the declared array size.

 fs/ocfs2/refcounttree.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 8eee5be4d1ed..7323bde70caa 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -2131,10 +2131,15 @@ static int ocfs2_remove_refcount_extent(handle_t *handle,
 		rb->rf_flags = 0;
 		rb->rf_parent = 0;
 		rb->rf_cpos = 0;
-		memset(&rb->rf_records, 0, sb->s_blocksize -
-		       offsetof(struct ocfs2_refcount_block, rf_records));
+		rb->rf_records.rl_used = 0;
+		rb->rf_records.rl_reserved2 = 0;
+		rb->rf_records.rl_reserved1 = 0;
+		/* rl_count determines the memset size and fortify object size. */
 		rb->rf_records.rl_count =
 				cpu_to_le16(ocfs2_refcount_recs_per_rb(sb));
+		memset(rb->rf_records.rl_recs, 0,
+		       le16_to_cpu(rb->rf_records.rl_count) *
+		       sizeof(*rb->rf_records.rl_recs));
 	}
 
 	ocfs2_journal_dirty(handle, ref_root_bh);
-- 
2.47.3