[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ea4fe05-c6e0-472a-a01b-ab6a0c4c9f76@oracle.com>
Date: Fri, 23 Aug 2024 14:10:49 -0500
From: Dave Kleikamp <dave.kleikamp@...cle.com>
To: Edward Adam Davis <eadavis@...com>,
syzbot+3c010e21296f33a5dc16@...kaller.appspotmail.com
Cc: jfs-discussion@...ts.sourceforge.net, syzkaller-bugs@...glegroups.com,
linux-kernel@...r.kernel.org
Subject: Re: [Jfs-discussion] [PATCH] jfs: Fix uaf in dbFreeBits
On 8/16/24 10:55PM, Edward Adam Davis via Jfs-discussion wrote:
> [syzbot reported]
> ==================================================================
> BUG: KASAN: slab-use-after-free in __mutex_lock_common kernel/locking/mutex.c:587 [inline]
> BUG: KASAN: slab-use-after-free in __mutex_lock+0xfe/0xd70 kernel/locking/mutex.c:752
> Read of size 8 at addr ffff8880229254b0 by task syz-executor357/5216
>
> CPU: 0 UID: 0 PID: 5216 Comm: syz-executor357 Not tainted 6.11.0-rc3-syzkaller-00156-gd7a5aa4b3c00 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/27/2024
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:93 [inline]
> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
> print_address_description mm/kasan/report.c:377 [inline]
> print_report+0x169/0x550 mm/kasan/report.c:488
> kasan_report+0x143/0x180 mm/kasan/report.c:601
> __mutex_lock_common kernel/locking/mutex.c:587 [inline]
> __mutex_lock+0xfe/0xd70 kernel/locking/mutex.c:752
> dbFreeBits+0x7ea/0xd90 fs/jfs/jfs_dmap.c:2390
> dbFreeDmap fs/jfs/jfs_dmap.c:2089 [inline]
> dbFree+0x35b/0x680 fs/jfs/jfs_dmap.c:409
> dbDiscardAG+0x8a9/0xa20 fs/jfs/jfs_dmap.c:1650
> jfs_ioc_trim+0x433/0x670 fs/jfs/jfs_discard.c:100
> jfs_ioctl+0x2d0/0x3e0 fs/jfs/ioctl.c:131
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:907 [inline]
> __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:893
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>
> Freed by task 5218:
> kasan_save_stack mm/kasan/common.c:47 [inline]
> kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
> kasan_save_free_info+0x40/0x50 mm/kasan/generic.c:579
> poison_slab_object+0xe0/0x150 mm/kasan/common.c:240
> __kasan_slab_free+0x37/0x60 mm/kasan/common.c:256
> kasan_slab_free include/linux/kasan.h:184 [inline]
> slab_free_hook mm/slub.c:2252 [inline]
> slab_free mm/slub.c:4473 [inline]
> kfree+0x149/0x360 mm/slub.c:4594
> dbUnmount+0x11d/0x190 fs/jfs/jfs_dmap.c:278
> jfs_mount_rw+0x4ac/0x6a0 fs/jfs/jfs_mount.c:247
> jfs_remount+0x3d1/0x6b0 fs/jfs/super.c:454
> reconfigure_super+0x445/0x880 fs/super.c:1083
> vfs_cmd_reconfigure fs/fsopen.c:263 [inline]
> vfs_fsconfig_locked fs/fsopen.c:292 [inline]
> __do_sys_fsconfig fs/fsopen.c:473 [inline]
> __se_sys_fsconfig+0xb6e/0xf80 fs/fsopen.c:345
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> [Analysis]
> There are two paths (dbUnmount and dbDiscardAG) that generate race
> condition when accessing bmap, which leads to the occurrence of uaf.
>
> Use the lock s_umount to synchronize them, in order to avoid uaf caused
> by race condition.
I'm afraid this is insufficient. dbUnmount() will actually free
JFS_SBI(ipbmap->i_sb)->bmap and set the pointer to NULL.
I think we need to serialize the entire function jfs_ioc_trim(), even
before initializing bmp. I don't know what other codepaths might run
into the same issue though.
Shaggy
>
> Reported-and-tested-by: syzbot+3c010e21296f33a5dc16@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3c010e21296f33a5dc16
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> ---
> fs/jfs/jfs_dmap.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index cb3cda1390ad..a409ae18454a 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -1645,7 +1645,9 @@ s64 dbDiscardAG(struct inode *ip, int agno, s64 minlen)
> * call jfs_issue_discard() itself */
> if (!(JFS_SBI(sb)->flag & JFS_DISCARD))
> jfs_issue_discard(ip, tt->blkno, tt->nblocks);
> + down_read(&sb->s_umount);
> dbFree(ip, tt->blkno, tt->nblocks);
> + up_read(&sb->s_umount);
> trimmed += tt->nblocks;
> }
> kfree(totrim);
Powered by blists - more mailing lists