From: Dave Chinner <david@fromorbit.com>
To: Yu Kuai <yukuai3@huawei.com>
Cc: darrick.wong@oracle.com, linux-xfs@vger.kernel.org,
linux-kernel@vger.kernel.org, yi.zhang@huawei.com
Subject: Re: [RFC PATCH] fix use after free in xlog_wait()
Date: Thu, 11 Jun 2020 12:28:48 +1000 [thread overview]
Message-ID: <20200611022848.GQ2040@dread.disaster.area> (raw)
In-Reply-To: <20200611013952.2589997-1-yukuai3@huawei.com>
On Thu, Jun 11, 2020 at 09:39:52AM +0800, Yu Kuai wrote:
> I recently got UAF by running generic/019 in qemu:
>
> ==================================================================
> BUG: KASAN: use-after-free in __lock_acquire+0x4508/0x68c0
> Read of size 8 at addr ffff88811327f080 by task fio/11147
....
> remove_wait_queue+0x1d/0x180
> xfs_log_commit_cil+0x1d9e/0x2a50
> __xfs_trans_commit+0x292/0xec0
Ok, so this is waking up from a the CIL context overrunning the hard
size limit....
> Freed by task 6826:
> save_stack+0x1b/0x40
> __kasan_slab_free+0x12c/0x170
> kfree+0xd6/0x300
> kvfree+0x42/0x50
> xlog_cil_committed+0xa9c/0xf30
> xlog_cil_push_work+0xa8c/0x1250
> process_one_work+0xa3e/0x17a0
> worker_thread+0x8e2/0x1050
> kthread+0x355/0x470
> ret_from_fork+0x22/0x30
Hmmmm. The CIL push work freed the context which means somethign
went wrong somewhere - we must be in CIL commit error path here...
/me checks generic/019
Oh, it's a repeated shutdown test. Right, so we're getting a
shutdown in the middle of a CIL push when the CIL is hard throttling
callers and the CIL context gets freed before the throttled tasks
can be woken.
Gotcha. Yup, that's a real issue, thanks for reporting it!
> I think the reason is that when 'ctx' is freed in xlog_cil_committed(),
> a previous call to xlog_wait(&ctx->xc_ctx->push_wait, ...) hasn't finished
> yet. Thus when remove_wait_queue() is called, UAF will be triggered
> since 'ctx' was freed:
>
> thread1 thread2 thread3
>
> __xfs_trans_commit
> xfs_log_commit_cil
> xlog_wait
> schedule
> xlog_cil_push_work
> wake_up_all
> xlog_cil_committed
> kmem_free
> remove_wait_queue
> spin_lock_irqsave --> UAF
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
> Instead, make sure waitqueue_active(&ctx->push_wait) return false before
> freeing 'ctx'.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> fs/xfs/xfs_log_cil.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index b43f0e8f43f2..59b21485b0fc 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -607,7 +607,7 @@ xlog_cil_committed(
>
> if (!list_empty(&ctx->busy_extents))
> xlog_discard_busy_extents(mp, ctx);
> - else
> + else if (!waitqueue_active(&ctx->push_wait))
> kmem_free(ctx);
That will just leak the memory instead, which is no better.
Let me go write a patch to fix this.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2020-06-11 2:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-11 1:39 [RFC PATCH] fix use after free in xlog_wait() Yu Kuai
2020-06-11 2:28 ` Dave Chinner [this message]
2020-06-11 2:45 ` [PATCH] xfs: fix use-after-free on CIL context on shutdown Dave Chinner
2020-06-11 15:11 ` Brian Foster
2020-06-16 1:16 ` yukuai (C)
2020-06-16 2:38 ` Dave Chinner
2020-06-19 13:46 ` Christoph Hellwig
2020-06-11 3:01 ` [RFC PATCH] fix use after free in xlog_wait() yukuai (C)
2020-06-11 5:05 ` Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200611022848.GQ2040@dread.disaster.area \
--to=david@fromorbit.com \
--cc=darrick.wong@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).