Messages in this thread | ![/](/images/icornerl.gif) | | From | Zheng Hacker <> | Date | Fri, 21 Jul 2023 16:15:19 +0800 | Subject | Re: [PATCH] fs/jfs: Add a mutex named txEnd_lmLogClose_mutex to prevent a race condition between txEnd and lmLogClose functions |
| |
Hello Michal,
Michal Koutný <mkoutny@suse.com> 于2023年7月20日周四 17:56写道: > > Hello Zheng. > > On Mon, May 15, 2023 at 05:59:56PM +0800, Zheng Wang <zyytlz.wz@163.com> wrote: > > ================================================================== > > BUG: KASAN: slab-use-after-free in instrument_atomic_write include/linux/instrumented.h:87 [inline] > > BUG: KASAN: slab-use-after-free in clear_bit include/asm-generic/bitops/instrumented-atomic.h:41 [inline] > > BUG: KASAN: slab-use-after-free in txEnd+0x2a3/0x5a0 fs/jfs/jfs_txnmgr.c:535 > > Write of size 8 at addr ffff888021bee840 by task jfsCommit/130 > > > > CPU: 3 PID: 130 Comm: jfsCommit Not tainted 6.3.0-rc7-pasta #1 > > Is this still pertinent with the current mainline? (There were some > changes to jfs.)
Thank you very much for your reply and suggestion. I thought that this BUG still exists in the current mainline kernel version. Since I am not very familiar with this part of the code, I am not sure if the proposed fix is correct.
> > > Through analysis, it was found that a race condition occurred between two > > functions lmLogClose and txEnd, which were executed in different threads. > > The possible sequence is as follows: > > > > ------------------------------------------------------------------------- > > cpu1(free thread) | cpu2(use thread) > > ------------------------------------------------------------------------- > > lmLogClose | txEnd > > | log = JFS_SBI(tblk->sb)->log; > > sbi->log = NULL; | > > kfree(log); [1] free log | > > | clear_bit(log_FLUSH, &log->flag); [2] UAF > > That looks sane to a by-passer. > > > Fix it by add a mutex lock between lmLogClose and txEnd: > > It doesn't feel right wrt "lock data, not code" heuristics. > And when I apply that, it turns out there's already jfs_log_mutex. > I'd suggest you explain more why a new lock is needed (if that's the > preferred solutino).
You're right, I think my fix method is not a good solution. Hoping you and other developers can help fix it.
> > Thanks, > Michal
Once again, I appreciate your help and will take your feedback into consideration when working on a solution.
Best regards, Zheng
| ![\](/images/icornerr.gif) |