On 4/10/24 2:05AM, Edward Adam Davis via Jfs-discussion wrote:
[syzbot reported]
general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 0 PID: 5061 Comm: syz-executor404 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
RIP: 0010:dtInsertEntry+0xd0c/0x1780 fs/jfs/jfs_dtree.c:3713
...
[Analyze]
When the pointer h has the same value as p, after writing name in UniStrncpy_to_le(),
p->header.flag will be cleared.
This will cause the previously true judgment "p->header.flag & BT-LEAF" to change
to no after writing the name operation, this leads to entering an incorrect branch
and accessing the uninitialized object ih when judging this condition for the
second time.
[Fix]
When allocating slots from the freelist, we start from the second one to preserve
the header of p from being incorrectly modified.
The freelist is simply corrupted. It should never be set to 0. We cannot
assume that slot[1] is on the free list. Probably the best we can do is
to add more sanity checking to the freelist value, and/or any slot's
next & prev value. That could potentially involve a lot more checks.
I've been accepting patches here and there for specific syzbot failures,
but any comprehensive sanity checking of every data structure would be a
huge effort.
What makes a fix a little more difficult is that dtInsertEntry returns
void and it would be difficult to gracefully recover. One could change
it to return an error and have all the callers check that. But I'm
afraid, just using a valid slot number would only lead to further data
corruption.
Thanks,
Shaggy
Reported-by: syzbot+bba84aef3a26fb93deb9@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
---
fs/jfs/jfs_dtree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 031d8f570f58..deb2a5cc78d8 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -3618,7 +3618,8 @@ static void dtInsertEntry(dtpage_t * p, int index, struct component_name * key,
kname = key->name;
/* allocate a free slot */
- hsi = fsi = p->header.freelist;
+ hsi = fsi = p->header.freelist = p->header.freelist == 0 ?
+ 1 : p->header.freelist;
h = &p->slot[fsi];
p->header.freelist = h->next;
--p->header.freecnt;
