public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Raphael Pinsonneault-Thibeault <rpthibeault@gmail.com>
Cc: cem@kernel.org, djwong@kernel.org, chandanbabu@kernel.org,
	bfoster@redhat.com, linux-xfs@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-kernel-mentees@lists.linux.dev, skhan@linuxfoundation.org,
	syzbot+9f6d080dece587cfdd4c@syzkaller.appspotmail.com
Subject: Re: [PATCH] xfs: ensure log recovery buffer is resized to avoid OOB
Date: Thu, 13 Nov 2025 09:19:50 +1100	[thread overview]
Message-ID: <aRUIBj3ntHM1rcfo@dread.disaster.area> (raw)
In-Reply-To: <20251112141032.2000891-3-rpthibeault@gmail.com>

On Wed, Nov 12, 2025 at 09:10:34AM -0500, Raphael Pinsonneault-Thibeault wrote:
> In xlog_do_recovery_pass(),
> commit 45cf976008dd ("xfs: fix log recovery buffer allocation for the legacy h_size fixup")
> added a fix to take the corrected h_size (from the xfsprogs bug
> workaround) into consideration for the log recovery buffer calculation.
> Without it, we would still allocate the buffer based on the incorrect
> on-disk size.
> 
> However, in a scenario similar to 45cf976008dd, syzbot creates a fuzzed
> record where xfs_has_logv2() but the xlog_rec_header h_version !=
> XLOG_VERSION_2.

We should abort journal recovery at that point because the record
header is corrupt and we can't trust it.

i.e. A filesytem with a version 2 log will only ever set XLOG_VERSION_2
in it's headers (and v1 will only ever set V_1), so if there is a
mismatch something has gone wrong and we should stop processing the
journal immediately.

Otherwise, stuff taht assumes the version flags are coherenti like
this...

> Meaning, we skip the log recover buffer calculation
> fix and allocate the buffer based on the incorrect on-disk size. Hence,
> a KASAN: slab-out-of-bounds read in xlog_do_recovery_pass() ->
> xlog_recover_process() -> xlog_cksum().

... goes wrong.

....

> Can xfs_has_logv2() and xlog_rec_header h_version ever disagree?

No. As per above, if they differ, either the journal or the
superblock has been corrupted and we need to abort processing with a
-EFSCORRUPTED error immediately.

That's the change that needs to be made here - xlog_valid_rec_header()
should validate that the header and sb log versions match, not just
that the record header only has "known" version bits set.

If we validate this up front, then the rest of the code can then
safely assume that xfs_has_logv2() and xlog_rec_header h_version are
coherent and correct and so won't be exposed to bugs related to an
undetected mismatch of various version fields...

-Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2025-11-12 22:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-12 14:10 [PATCH] xfs: ensure log recovery buffer is resized to avoid OOB Raphael Pinsonneault-Thibeault
2025-11-12 15:28 ` Christoph Hellwig
2025-11-12 18:18   ` [PATCH] xfs: reject log records with v2 size but v1 header version " Raphael Pinsonneault-Thibeault
2025-11-12 18:45     ` Darrick J. Wong
2025-11-13  6:55       ` Christoph Hellwig
2025-11-12 22:19 ` Dave Chinner [this message]
2025-11-13 19:01   ` [PATCH v3] xfs: validate log record version against superblock log version Raphael Pinsonneault-Thibeault
2025-11-18 20:19     ` Dave Chinner
2025-11-19 15:37       ` [PATCH v4] " Raphael Pinsonneault-Thibeault
2025-11-19 20:16         ` Dave Chinner
2025-11-20  6:57         ` Christoph Hellwig
2025-11-24 17:47           ` [PATCH v5] " Raphael Pinsonneault-Thibeault
2025-11-24 18:52             ` Darrick J. Wong
2025-11-25  6:31               ` Christoph Hellwig
2025-11-25 17:06                 ` Darrick J. Wong
2025-11-25  6:31             ` Christoph Hellwig

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=aRUIBj3ntHM1rcfo@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=cem@kernel.org \
    --cc=chandanbabu@kernel.org \
    --cc=djwong@kernel.org \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=rpthibeault@gmail.com \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+9f6d080dece587cfdd4c@syzkaller.appspotmail.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