* [PATCH] xfs: validate buffer log item before reordering
@ 2026-09-13 11:45 Weiming Shi
2026-09-13 23:49 ` Dave Chinner
0 siblings, 1 reply; 3+ messages in thread
From: Weiming Shi @ 2026-09-13 11:45 UTC (permalink / raw)
To: Carlos Maiolino
Cc: linux-xfs, linux-kernel, Chandan Babu R, Darrick J . Wong,
Xiang Mei, Weiming Shi, co+af981e62f5c7171a, stable
Log recovery reorders transaction items before buffer item pass1 validates
the format of region 0. A corrupt log can therefore supply a four-byte
region containing only blf_type and blf_size. xlog_recover_buf_reorder()
then reads blf_flags immediately past the allocation:
BUG: KASAN: slab-out-of-bounds in xlog_recover_buf_reorder
Read of size 2 at addr ffff88800e40e364 by task poc/133
Call Trace:
kasan_report mm/kasan/report.c:595
xlog_recover_buf_reorder fs/xfs/xfs_buf_item_recover.c:164
xlog_recover_reorder_trans fs/xfs/xfs_log_recover.c:1929
xlog_recover_commit_trans fs/xfs/xfs_log_recover.c:2053
xlog_recovery_process_trans fs/xfs/xfs_log_recover.c:2319
xlog_recover_process_data fs/xfs/xfs_log_recover.c:2510
xlog_do_recovery_pass fs/xfs/xfs_log_recover.c:3253
xlog_do_log_recovery fs/xfs/xfs_log_recover.c:3340
xlog_do_recover fs/xfs/xfs_log_recover.c:3377
xlog_recover fs/xfs/xfs_log_recover.c:3502
xfs_log_mount fs/xfs/xfs_log.c:617
xfs_mountfs fs/xfs/xfs_mount.c:1031
The buggy address is located 0 bytes to the right of
allocated 4-byte region [ffff88800e40e360, ffff88800e40e364)
Validate region 0 before inspecting the flags. Keep a malformed item on
the regular item list so that xlog_recover_buf_commit_pass1() reports the
corrupt log through the existing error path.
Fixes: 86ffa471d9ce ("xfs: refactor log recovery item sorting into a generic dispatch structure")
Reported-by: co+af981e62f5c7171a@bugs.sh
Closes: https://lore.kernel.org/all/aqZALi7GdVprcNOh@cronus.toxiclabs.cc/
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
fs/xfs/xfs_buf_item_recover.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 57929f115055..70e69ec731ac 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -161,6 +161,10 @@ xlog_recover_buf_reorder(
{
struct xfs_buf_log_format *buf_f = item->ri_buf[0].iov_base;
+ /* A short region 0 is rejected by xlog_recover_buf_commit_pass1. */
+ if (!xfs_buf_log_check_iovec(&item->ri_buf[0]))
+ return XLOG_REORDER_ITEM_LIST;
+
if (buf_f->blf_flags & XFS_BLF_CANCEL)
return XLOG_REORDER_CANCEL_LIST;
if (buf_f->blf_flags & XFS_BLF_INODE_BUF)
--
2.55.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: validate buffer log item before reordering
2026-09-13 11:45 [PATCH] xfs: validate buffer log item before reordering Weiming Shi
@ 2026-09-13 23:49 ` Dave Chinner
2026-09-14 1:49 ` Weiming Shi
0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2026-09-13 23:49 UTC (permalink / raw)
To: Weiming Shi
Cc: Carlos Maiolino, linux-xfs, linux-kernel, Chandan Babu R,
Darrick J . Wong, Xiang Mei, co+af981e62f5c7171a, stable
On Sun, Sep 13, 2026 at 07:45:30PM +0800, Weiming Shi wrote:
> Log recovery reorders transaction items before buffer item pass1 validates
> the format of region 0. A corrupt log can therefore supply a four-byte
> region containing only blf_type and blf_size. xlog_recover_buf_reorder()
> then reads blf_flags immediately past the allocation:
>
> BUG: KASAN: slab-out-of-bounds in xlog_recover_buf_reorder
> Read of size 2 at addr ffff88800e40e364 by task poc/133
> Call Trace:
> kasan_report mm/kasan/report.c:595
> xlog_recover_buf_reorder fs/xfs/xfs_buf_item_recover.c:164
> xlog_recover_reorder_trans fs/xfs/xfs_log_recover.c:1929
> xlog_recover_commit_trans fs/xfs/xfs_log_recover.c:2053
> xlog_recovery_process_trans fs/xfs/xfs_log_recover.c:2319
> xlog_recover_process_data fs/xfs/xfs_log_recover.c:2510
> xlog_do_recovery_pass fs/xfs/xfs_log_recover.c:3253
> xlog_do_log_recovery fs/xfs/xfs_log_recover.c:3340
> xlog_do_recover fs/xfs/xfs_log_recover.c:3377
> xlog_recover fs/xfs/xfs_log_recover.c:3502
> xfs_log_mount fs/xfs/xfs_log.c:617
> xfs_mountfs fs/xfs/xfs_mount.c:1031
> The buggy address is located 0 bytes to the right of
> allocated 4-byte region [ffff88800e40e360, ffff88800e40e364)
>
> Validate region 0 before inspecting the flags. Keep a malformed item on
> the regular item list so that xlog_recover_buf_commit_pass1() reports the
> corrupt log through the existing error path.
>
> Fixes: 86ffa471d9ce ("xfs: refactor log recovery item sorting into a generic dispatch structure")
> Reported-by: co+af981e62f5c7171a@bugs.sh
> Closes: https://lore.kernel.org/all/aqZALi7GdVprcNOh@cronus.toxiclabs.cc/
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
That's pretty poor form - taking a bug reported by someone else and
then reposting their proposed fix as if it was your own before
they've even had a chance to respond to the maintainer's comments.
And that's even before I take into account that the proposed fix is
an extremely poor one. It basically punts the broken log item down
the road where some future processing operation happens to catch it
and abort journal recovery with EFSCORRUPTED before bad things
happen.
i.e. you didn't review it, nor did your directions to the LLM that
"helped" you direct it to determine if this was the right way to fix
this issue.
To compound this all, I've previously asked you directly to stop
sending patches that add random verification checks to random parts
of log recovery and instead spend time on actually addressing the
root cause of these issues. i.e. that we lack robust on-disk format
verification of the journal contents.
Last week, when someone else posted a random 'log vector is broken'
hack, I said the same thing (yet again!) and posted the design doc I
wrote a while back to provide robust, generic log item validation
for the entire journal:
https://lore.kernel.org/linux-xfs/ap8ucHIw-pKLhh9c@dread/
I posted a link to the git repo where I've started this work further
down that discussion:
| Ok, I just posted my current WIP to the log-verification-1 branch
| in my kernel.org repo
| (https://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git)
That's the work we need to do to get rid of -all- the journal
corruption issues in one go; once we have that in place then almost
all corruptions will be caught long before any of the recovery
parsing code can trip over it....
-Dave.
--
Dave Chinner
dgc@kernel.org
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: validate buffer log item before reordering
2026-09-13 23:49 ` Dave Chinner
@ 2026-09-14 1:49 ` Weiming Shi
0 siblings, 0 replies; 3+ messages in thread
From: Weiming Shi @ 2026-09-14 1:49 UTC (permalink / raw)
To: Dave Chinner
Cc: Carlos Maiolino, linux-xfs, linux-kernel, Chandan Babu R,
Darrick J . Wong, Xiang Mei, co+af981e62f5c7171a, stable
Dave Chinner <dgc@kernel.org> 于2026年9月14日周一 07:49写道:
>
> On Sun, Sep 13, 2026 at 07:45:30PM +0800, Weiming Shi wrote:
> > Log recovery reorders transaction items before buffer item pass1 validates
> > the format of region 0. A corrupt log can therefore supply a four-byte
> > region containing only blf_type and blf_size. xlog_recover_buf_reorder()
> > then reads blf_flags immediately past the allocation:
> >
> > BUG: KASAN: slab-out-of-bounds in xlog_recover_buf_reorder
> > Read of size 2 at addr ffff88800e40e364 by task poc/133
> > Call Trace:
> > kasan_report mm/kasan/report.c:595
> > xlog_recover_buf_reorder fs/xfs/xfs_buf_item_recover.c:164
> > xlog_recover_reorder_trans fs/xfs/xfs_log_recover.c:1929
> > xlog_recover_commit_trans fs/xfs/xfs_log_recover.c:2053
> > xlog_recovery_process_trans fs/xfs/xfs_log_recover.c:2319
> > xlog_recover_process_data fs/xfs/xfs_log_recover.c:2510
> > xlog_do_recovery_pass fs/xfs/xfs_log_recover.c:3253
> > xlog_do_log_recovery fs/xfs/xfs_log_recover.c:3340
> > xlog_do_recover fs/xfs/xfs_log_recover.c:3377
> > xlog_recover fs/xfs/xfs_log_recover.c:3502
> > xfs_log_mount fs/xfs/xfs_log.c:617
> > xfs_mountfs fs/xfs/xfs_mount.c:1031
> > The buggy address is located 0 bytes to the right of
> > allocated 4-byte region [ffff88800e40e360, ffff88800e40e364)
> >
> > Validate region 0 before inspecting the flags. Keep a malformed item on
> > the regular item list so that xlog_recover_buf_commit_pass1() reports the
> > corrupt log through the existing error path.
> >
> > Fixes: 86ffa471d9ce ("xfs: refactor log recovery item sorting into a generic dispatch structure")
> > Reported-by: co+af981e62f5c7171a@bugs.sh
> > Closes: https://lore.kernel.org/all/aqZALi7GdVprcNOh@cronus.toxiclabs.cc/
> > Cc: stable@vger.kernel.org
> > Assisted-by: Codex:gpt-5
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>
> That's pretty poor form - taking a bug reported by someone else and
> then reposting their proposed fix as if it was your own before
> they've even had a chance to respond to the maintainer's comments.
>
> And that's even before I take into account that the proposed fix is
> an extremely poor one. It basically punts the broken log item down
> the road where some future processing operation happens to catch it
> and abort journal recovery with EFSCORRUPTEDEFSCORRUPTED before bad things
> happen.
>
> i.e. you didn't review it, nor did your directions to the LLM that
> "helped" you direct it to determine if this was the right way to fix
> this issue.
>
> To compound this all, I've previously asked you directly to stop
> sending patches that add random verification checks to random parts
> of log recovery and instead spend time on actually addressing the
> root cause of these issues. i.e. that we lack robust on-disk format
> verification of the journal contents.
>
> Last week, when someone else posted a random 'log vector is broken'
> hack, I said the same thing (yet again!) and posted the design doc I
> wrote a while back to provide robust, generic log item validation
> for the entire journal:
>
> https://lore.kernel.org/linux-xfs/ap8ucHIw-pKLhh9c@dread/
>
> I posted a link to the git repo where I've started this work further
> down that discussion:
>
> | Ok, I just posted my current WIP to the log-verification-1 branch
> | in my kernel.org repo
> | (https://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git)
>
> That's the work we need to do to get rid of -all- the journal
> corruption issues in one go; once we have that in place then almost
> all corruptions will be caught long before any of the recovery
> parsing code can trip over it....
>
> -Dave.
> --
> Dave Chinner
> dgc@kernel.org
You’re right. I’m sorry that I overlooked the ongoing effort to
address these journal corruption issues systematically and instead
started from the bugs.sh patch, which merely stops this particular
crash rather than addressing the underlying problem properly.
I did analyze the crash, test the patch, and check that it handled
this reproducer; I did not send it without review or testing. However,
I understand that this does not make it a good-quality or appropriate
fix, and I’m sorry for submitting it.
I’ll follow your log-verification work and would be happy to test
whether it also addresses this issue.
Thanks,
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-14 1:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-13 11:45 [PATCH] xfs: validate buffer log item before reordering Weiming Shi
2026-09-13 23:49 ` Dave Chinner
2026-09-14 1:49 ` Weiming Shi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®