linux-kernel - Re: [RFC PATCH] fix use after free in xlog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Jun 2020 15:05:48 +1000
From:   Dave Chinner <david@...morbit.com>
To:     "yukuai (C)" <yukuai3@...wei.com>
Cc:     darrick.wong@...cle.com, linux-xfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, yi.zhang@...wei.com
Subject: Re: [RFC PATCH] fix use after free in xlog_wait()

On Thu, Jun 11, 2020 at 11:01:38AM +0800, yukuai (C) wrote:
> On 2020/6/11 10:28, Dave Chinner wrote
> > Actually, it's a lot simpler:
> > 
> > thread1			thread2
> > 
> > __xfs_trans_commit
> >   xfs_log_commit_cil
> >    xlog_wait
> >     schedule
> > 			xlog_cil_push_work
> > 			wake_up_all
> > 			<shutdown aborts commit>
> > 			xlog_cil_committed
> > 			kmem_free
> > 
> >     remove_wait_queue
> >      spin_lock_irqsave --> UAF
> > 
> 
> It's ture in this case, however, I got another result when I
> tried to reporduce it, which seems 'ctx' can be freed in a
> different path:

Yup, it's effectively the same thing because of the nature of the IO
failures (generated at submit time) and scheduler behaviour of
workqueues. THis means the IO completion that processes the error is
is queued to a workqueue on the same CPU. When thread 2 finishes
running (it hasn't seen an error yet) the completion work will get
get scheduled ahead of thread1 (cpu bound kernel task vs unbound
user task).  The completion work then runs the shutdown because it
saw a log IO error and because it's the commit record bio it runs
the journal checkpoint completion to abort all the items attached to
it and free the CIL context. Then thread 1 runs again.

The only difference between the two cases is which IO of the CIL
commit the request was failed on....

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com