Received-SPF: pass (google.com: domain of linux-kernel+bounces-72412-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <tencent_7D2D4F8D8DCEA5FA8D85922ADDDB5DEE9907@qq.com>
From: Edward Adam Davis <eadavis@qq.com>
To: willy@infradead.org
Cc: eadavis@qq.com,
	jfs-discussion@lists.sourceforge.net,
	linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	shaggy@kernel.org,
	syzbot+c244f4a09ca85dd2ebc1@syzkaller.appspotmail.com,
	syzkaller-bugs@googlegroups.com
Subject: Re: [PATCH] jfs: fix uaf in jfs_syncpt
Date: Tue, 20 Feb 2024 14:17:01 +0800
In-Reply-To: <ZdQlbc0Hb0UZy6od@casper.infradead.org>
References: <ZdQlbc0Hb0UZy6od@casper.infradead.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

On Tue, 20 Feb 2024 04:07:09 +0000, Matthew Wilcox wrote:
> > During the execution of the jfs lazy commit, the jfs file system was unmounted,
> > causing the sbi and jfs log objects to be released, triggering this issue.
> > The solution is to add mutex to synchronize jfs lazy commit and jfs unmount
> > operations.
> 
> Why is that the solution?  LAZY_LOCK with IN_LAZYCOMMIT is supposed to
LAZY_LOCK not cover jfs umount.
> cover this.  Please be more verbose in your commit messages.  Describe
> what is going wrong and why; that will allow people to understand why
> this is the correct solution to the problem.
[Syz reported]
BUG: KASAN: slab-use-after-free in __mutex_waiter_is_first kernel/locking/mutex.c:197 [inline]
BUG: KASAN: slab-use-after-free in __mutex_lock_common kernel/locking/mutex.c:686 [inline]
BUG: KASAN: slab-use-after-free in __mutex_lock+0x8f4/0x9d0 kernel/locking/mutex.c:752
Read of size 8 at addr ffff8880272d2908 by task jfsCommit/131

CPU: 3 PID: 131 Comm: jfsCommit Not tainted 6.8.0-rc4-syzkaller-00388-gced590523156 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0xc4/0x620 mm/kasan/report.c:488
 kasan_report+0xda/0x110 mm/kasan/report.c:601
 __mutex_waiter_is_first kernel/locking/mutex.c:197 [inline]
 __mutex_lock_common kernel/locking/mutex.c:686 [inline]
 __mutex_lock+0x8f4/0x9d0 kernel/locking/mutex.c:752
 jfs_syncpt+0x2a/0xa0 fs/jfs/jfs_logmgr.c:1039
 txEnd+0x30d/0x5a0 fs/jfs/jfs_txnmgr.c:549
 txLazyCommit fs/jfs/jfs_txnmgr.c:2684 [inline]
 jfs_lazycommit+0x77d/0xb20 fs/jfs/jfs_txnmgr.c:2733
 kthread+0x2c6/0x3b0 kernel/kthread.c:388
 ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242


Freed by task 5177:
 kasan_save_stack+0x33/0x60 mm/kasan/common.c:47
 kasan_save_track+0x14/0x30 mm/kasan/common.c:68
 kasan_save_free_info+0x3f/0x60 mm/kasan/generic.c:640
 poison_slab_object mm/kasan/common.c:241 [inline]
 __kasan_slab_free+0x121/0x1c0 mm/kasan/common.c:257
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2121 [inline]
 slab_free mm/slub.c:4299 [inline]
 kfree+0x124/0x370 mm/slub.c:4409
 lmLogClose+0x585/0x710 fs/jfs/jfs_logmgr.c:1461
 jfs_umount+0x2f0/0x440 fs/jfs/jfs_umount.c:114
 jfs_put_super+0x88/0x1d0 fs/jfs/super.c:194

[Analyze]
This issue occurs due to task 131 executing jfs lazy commit and task 5177 executing
jfs put super (which will release objects such as sbi and jfs log).

The solution is to use mutex to sort the two tasks and determine whether the log
and sbi objects are valid before using them. 
This way, regardless of who executes the two tasks first, the latter can determine
whether the log and sbi objects are valid or invalid, thus avoiding the current problem.