The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] xfs: fix use-after-free of buf_log_item in xlog_cil_build_lv_chain
@ 2026-06-04  9:42 Gou Hao
  2026-06-10 12:28 ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Gou Hao @ 2026-06-04  9:42 UTC (permalink / raw)
  To: cem, djwong, dchinner
  Cc: linux-xfs, linux-kernel, niecheng1, zhanjun, gouhaojake, gouhao

xfs_buf_item_done() frees the buf_item via xfs_buf_item_relse() but
does not remove the item from the CIL log_items list (li_cil). When the
item is freed through an error/shutdown/abort path before the CIL push
worker processes it, the freed memory remains linked in ctx->log_items.

The CIL push worker in xlog_cil_build_lv_chain() then dereferences
the freed object via item->li_lv, triggering a KASAN slab-use-after-free.
For details, see Link[1].

Add down_read() on xc_ctx_lock before list_del_init() in
xfs_buf_item_done() to safely remove the item from the CIL list.  This
uses the same lock that protects CIL list operations: insertions are
done under xc_ctx_lock read-side (xlog_cil_insert_items) and removals
under write-side (xlog_cil_build_lv_chain).  The read lock is safe here
because xfs_buf_item_done() is always called in process context (workqueue
or direct I/O wait) and cannot deadlock with the CIL push worker which
holds the write lock during xlog_cil_build_lv_chain - the worker does not
trigger metadata buffer I/O that would call xfs_buf_item_done().

Reported-by: syzbot+598a791b31c498b63c6b@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a069a95.050a0220.2921a.0006.GAE@google.com/T/ [1]
Fixes: 816c330b605c ("xfs: factor out stale buffer item completion")
Cc: stable@vger.kernel.org
Suggested-by: Zhan Jun <zhanjun@uniontech.com>
Signed-off-by: Gou Hao <gouhao@uniontech.com>
---
 fs/xfs/xfs_buf_item.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 8487635579e5..75529dfd1170 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -1067,6 +1067,11 @@ void
 xfs_buf_item_done(
 	struct xfs_buf		*bp)
 {
+	if (bp->b_log_item->bli_item.li_log->l_cilp) {
+		down_read(&bp->b_log_item->bli_item.li_log->l_cilp->xc_ctx_lock);
+		list_del_init(&bp->b_log_item->bli_item.li_cil);
+		up_read(&bp->b_log_item->bli_item.li_log->l_cilp->xc_ctx_lock);
+	}
 	/*
 	 * If we are forcibly shutting down, this may well be off the AIL
 	 * already. That's because we simulate the log-committed callbacks to
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: fix use-after-free of buf_log_item in xlog_cil_build_lv_chain
  2026-06-04  9:42 [PATCH] xfs: fix use-after-free of buf_log_item in xlog_cil_build_lv_chain Gou Hao
@ 2026-06-10 12:28 ` Christoph Hellwig
  2026-06-11 11:49   ` Gou Hao
  2026-07-30 21:58   ` Dave Chinner
  0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2026-06-10 12:28 UTC (permalink / raw)
  To: Gou Hao
  Cc: cem, djwong, dchinner, linux-xfs, linux-kernel, niecheng1,
	zhanjun, gouhaojake, gouhao

On Thu, Jun 04, 2026 at 05:42:33PM +0800, Gou Hao wrote:
> xfs_buf_item_done() frees the buf_item via xfs_buf_item_relse() but
> does not remove the item from the CIL log_items list (li_cil). When the
> item is freed through an error/shutdown/abort path before the CIL push
> worker processes it, the freed memory remains linked in ctx->log_items.
> 
> The CIL push worker in xlog_cil_build_lv_chain() then dereferences
> the freed object via item->li_lv, triggering a KASAN slab-use-after-free.
> For details, see Link[1].

There's no reproducer there.  Do you have a local one?

> Add down_read() on xc_ctx_lock before list_del_init() in
> xfs_buf_item_done() to safely remove the item from the CIL list.  This
> uses the same lock that protects CIL list operations: insertions are
> done under xc_ctx_lock read-side (xlog_cil_insert_items) and removals
> under write-side (xlog_cil_build_lv_chain).  The read lock is safe here
> because xfs_buf_item_done() is always called in process context (workqueue
> or direct I/O wait) and cannot deadlock with the CIL push worker which
> holds the write lock during xlog_cil_build_lv_chain - the worker does not
> trigger metadata buffer I/O that would call xfs_buf_item_done().

This looks like a more general issue as we should never free anything
that is still on the CIL. I.e. it looks like we have even more issues
with the buf item state machine here :(

> 
> Reported-by: syzbot+598a791b31c498b63c6b@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6a069a95.050a0220.2921a.0006.GAE@google.com/T/ [1]
> Fixes: 816c330b605c ("xfs: factor out stale buffer item completion")
> Cc: stable@vger.kernel.org
> Suggested-by: Zhan Jun <zhanjun@uniontech.com>
> Signed-off-by: Gou Hao <gouhao@uniontech.com>
> ---
>  fs/xfs/xfs_buf_item.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 8487635579e5..75529dfd1170 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -1067,6 +1067,11 @@ void
>  xfs_buf_item_done(
>  	struct xfs_buf		*bp)
>  {
> +	if (bp->b_log_item->bli_item.li_log->l_cilp) {
> +		down_read(&bp->b_log_item->bli_item.li_log->l_cilp->xc_ctx_lock);
> +		list_del_init(&bp->b_log_item->bli_item.li_cil);
> +		up_read(&bp->b_log_item->bli_item.li_log->l_cilp->xc_ctx_lock);
> +	}
>  	/*
>  	 * If we are forcibly shutting down, this may well be off the AIL
>  	 * already. That's because we simulate the log-committed callbacks to
> -- 
> 2.20.1
> 
> 
---end quoted text---

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: fix use-after-free of buf_log_item in xlog_cil_build_lv_chain
  2026-06-10 12:28 ` Christoph Hellwig
@ 2026-06-11 11:49   ` Gou Hao
  2026-07-30 21:58   ` Dave Chinner
  1 sibling, 0 replies; 4+ messages in thread
From: Gou Hao @ 2026-06-11 11:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: cem, djwong, dchinner, linux-xfs, linux-kernel, niecheng1,
	zhanjun, gouhaojake


On 6/10/26 20:28, Christoph Hellwig wrote:
> On Thu, Jun 04, 2026 at 05:42:33PM +0800, Gou Hao wrote:
>> xfs_buf_item_done() frees the buf_item via xfs_buf_item_relse() but
>> does not remove the item from the CIL log_items list (li_cil). When the
>> item is freed through an error/shutdown/abort path before the CIL push
>> worker processes it, the freed memory remains linked in ctx->log_items.
>>
>> The CIL push worker in xlog_cil_build_lv_chain() then dereferences
>> the freed object via item->li_lv, triggering a KASAN slab-use-after-free.
>> For details, see Link[1].
> There's no reproducer there.  Do you have a local one?

No, I don't have one either. It's just based on code analysis.


>> Add down_read() on xc_ctx_lock before list_del_init() in
>> xfs_buf_item_done() to safely remove the item from the CIL list.  This
>> uses the same lock that protects CIL list operations: insertions are
>> done under xc_ctx_lock read-side (xlog_cil_insert_items) and removals
>> under write-side (xlog_cil_build_lv_chain).  The read lock is safe here
>> because xfs_buf_item_done() is always called in process context (workqueue
>> or direct I/O wait) and cannot deadlock with the CIL push worker which
>> holds the write lock during xlog_cil_build_lv_chain - the worker does not
>> trigger metadata buffer I/O that would call xfs_buf_item_done().
> This looks like a more general issue as we should never free anything
> that is still on the CIL. I.e. it looks like we have even more issues
> with the buf item state machine here :(
>
>> Reported-by: syzbot+598a791b31c498b63c6b@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/6a069a95.050a0220.2921a.0006.GAE@google.com/T/ [1]
>> Fixes: 816c330b605c ("xfs: factor out stale buffer item completion")
>> Cc: stable@vger.kernel.org
>> Suggested-by: Zhan Jun <zhanjun@uniontech.com>
>> Signed-off-by: Gou Hao <gouhao@uniontech.com>
>> ---
>>   fs/xfs/xfs_buf_item.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
>> index 8487635579e5..75529dfd1170 100644
>> --- a/fs/xfs/xfs_buf_item.c
>> +++ b/fs/xfs/xfs_buf_item.c
>> @@ -1067,6 +1067,11 @@ void
>>   xfs_buf_item_done(
>>   	struct xfs_buf		*bp)
>>   {
>> +	if (bp->b_log_item->bli_item.li_log->l_cilp) {
>> +		down_read(&bp->b_log_item->bli_item.li_log->l_cilp->xc_ctx_lock);
>> +		list_del_init(&bp->b_log_item->bli_item.li_cil);
>> +		up_read(&bp->b_log_item->bli_item.li_log->l_cilp->xc_ctx_lock);
>> +	}
>>   	/*
>>   	 * If we are forcibly shutting down, this may well be off the AIL
>>   	 * already. That's because we simulate the log-committed callbacks to
>> -- 
>> 2.20.1
>>
>>
> ---end quoted text---
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: fix use-after-free of buf_log_item in xlog_cil_build_lv_chain
  2026-06-10 12:28 ` Christoph Hellwig
  2026-06-11 11:49   ` Gou Hao
@ 2026-07-30 21:58   ` Dave Chinner
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2026-07-30 21:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Gou Hao, cem, djwong, dchinner, linux-xfs, linux-kernel,
	niecheng1, zhanjun, gouhaojake, gouhao

On Wed, Jun 10, 2026 at 05:28:47AM -0700, Christoph Hellwig wrote:
> On Thu, Jun 04, 2026 at 05:42:33PM +0800, Gou Hao wrote:
> > xfs_buf_item_done() frees the buf_item via xfs_buf_item_relse() but
> > does not remove the item from the CIL log_items list (li_cil). When the
> > item is freed through an error/shutdown/abort path before the CIL push
> > worker processes it, the freed memory remains linked in ctx->log_items.
> > 
> > The CIL push worker in xlog_cil_build_lv_chain() then dereferences
> > the freed object via item->li_lv, triggering a KASAN slab-use-after-free.
> > For details, see Link[1].
> 
> There's no reproducer there.  Do you have a local one?
> 
> > Add down_read() on xc_ctx_lock before list_del_init() in
> > xfs_buf_item_done() to safely remove the item from the CIL list.  This
> > uses the same lock that protects CIL list operations: insertions are
> > done under xc_ctx_lock read-side (xlog_cil_insert_items) and removals
> > under write-side (xlog_cil_build_lv_chain).  The read lock is safe here
> > because xfs_buf_item_done() is always called in process context (workqueue
> > or direct I/O wait) and cannot deadlock with the CIL push worker which
> > holds the write lock during xlog_cil_build_lv_chain - the worker does not
> > trigger metadata buffer I/O that would call xfs_buf_item_done().
> 
> This looks like a more general issue as we should never free anything
> that is still on the CIL. I.e. it looks like we have even more issues
> with the buf item state machine here :(

I finally found some time to look at this syzbot report and do some
analysis of it.  AFAICT, the BLI life cycle is solid.

Go have a look at the syzbot report. i.e. where KASAN reports that
the BLI has been freed from. It is freed from buffer read IO
completion. Now go and have a look at xfs_buf_ioend(): the read IO
completion does not -ever- access attached BLIs - even on IO failure
- let alone free them. Only the write IO completion path (i.e.
!XBF_READ) accesses the attached BLI.

IOWs, the KASAN trace is telling us we've got a read IO completion
*without* XBF_READ being set on the buffer, and that freeing the BLI
from this context is how we ended up with the CIL reference to the
BLI being removed incorrectly leading to the UAF.

I spent some time trying to come up with ways we could have a read
IO completion run without XBF_READ being set, and I cannot find it.
No combination of corruption errors, reverifier, readahead, stale,
and/or shutdown races appear to allow a READ IO without XBF_READ
being set on the buffer through to IO completion, even in manual
failure paths like xfs_buf_fail().

I can only conclude that this was caused by one of two things:
	- semaphores got broken unexpectedly; or
	- memory corruption from some other syzbot test that was
	  running at the same time trashed bp->b_flags whilst the
	  buffer was under IO.

Given that syzbot has only reported this 5 times in 12 hours only on
a 7.1-rc3 kernel, never before and never since, an external memory
corruption bug that has since been fixed seems like the most like
cause here.

IOWs, I think there's nothing in XFS to fix here, and the syzbot
report should simply be closed.

-Dave.
-- 
Dave Chinner
dgc@kernel.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-30 21:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04  9:42 [PATCH] xfs: fix use-after-free of buf_log_item in xlog_cil_build_lv_chain Gou Hao
2026-06-10 12:28 ` Christoph Hellwig
2026-06-11 11:49   ` Gou Hao
2026-07-30 21:58   ` Dave Chinner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox