From: "Darrick J. Wong" <djwong@kernel.org>
To: Yousef Alhouseen <alhouseenyousef@gmail.com>
Cc: Carlos Maiolino <cem@kernel.org>,
linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org,
syzbot+b7dfbed0c6c2b5e9fd34@syzkaller.appspotmail.com
Subject: Re: [PATCH] xfs: initialize first bad log block in head verification
Date: Mon, 29 Jun 2026 13:13:50 -0700 [thread overview]
Message-ID: <20260629201350.GC6078@frogsfrogsfrogs> (raw)
In-Reply-To: <20260628092513.39620-1-alhouseenyousef@gmail.com>
On Sun, Jun 28, 2026 at 11:25:13AM +0200, Yousef Alhouseen wrote:
> xlog_do_recovery_pass() only writes first_bad when it reaches the common
> error exit after processing a log record. An earlier CRC or corruption
> failure can therefore return without initializing the out-parameter.
>
> xlog_verify_head() tests first_bad on those errors and may then use its
> uninitialized stack value as a log block number while searching for the
> last good record. Initialize it to zero, matching xlog_verify_tail(), so
> an error without a recorded bad block is returned directly.
>
> Fixes: 7088c4136fa1 ("xfs: detect and trim torn writes during log recovery")
> Reported-by: syzbot+b7dfbed0c6c2b5e9fd34@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b7dfbed0c6c2b5e9fd34
> Cc: stable@vger.kernel.org
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
> fs/xfs/xfs_log_recover.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 09e6678ca487..d8125f3add4b 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1028,7 +1028,7 @@ xlog_verify_head(
> {
> struct xlog_rec_header *tmp_rhead;
> char *tmp_buffer;
> - xfs_daddr_t first_bad;
> + xfs_daddr_t first_bad = 0;
Why is it safe to set this to the first daddr of the log? Is it
possible that a filesystem could have a log record starting near the end
of the log which wrapped around, and later suffered a CRC corruption in
daddr 0?
xfs_daddr_t already defines an explicit null value (XFS_BUF_DADDR_NULL);
wouldn't it be /much/ safer to set that here and update the if test body
later?
--D
> xfs_daddr_t tmp_rhead_blk;
> int found;
> int error;
> --
> 2.54.0
>
>
next prev parent reply other threads:[~2026-06-29 20:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-28 9:25 [PATCH] xfs: initialize first bad log block in head verification Yousef Alhouseen
2026-06-29 20:13 ` Darrick J. Wong [this message]
2026-06-30 10:01 ` Yousef Alhouseen
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=20260629201350.GC6078@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=alhouseenyousef@gmail.com \
--cc=cem@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=syzbot+b7dfbed0c6c2b5e9fd34@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.