[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZiphYrREkQvxkE-U@bfoster>
Date: Thu, 25 Apr 2024 09:57:54 -0400
From: Brian Foster <bfoster@...hat.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: Sam Sun <samsun1006219@...il.com>, linux-kernel@...r.kernel.org,
linux-xfs@...r.kernel.org, djwong@...nel.org,
chandan.babu@...cle.com, syzkaller-bugs@...glegroups.com,
xrivendell7@...il.com
Subject: Re: [Linux kernel bug] KASAN: slab-out-of-bounds Read in xlog_cksum
On Thu, Apr 25, 2024 at 06:12:23AM -0700, Christoph Hellwig wrote:
> This triggers the workaround for really old xfsprogs putting in a
> bogus h_size:
>
> [ 12.101992] XFS (loop0): invalid iclog size (0 bytes), using lsunit (65536 bytes)
>
> but then calculates the log recovery buffer size based on the actual
> on-disk h_size value. The patch below open codes xlog_logrec_hblks and
> fixes this particular reproducer. But I wonder if we should limit the
> workaround. Brian, you don't happpen to remember how old xfsprogs had
> to be to require your workaround (commit a70f9fe52daa8)?
>
No, but a little digging turns up xfsprogs commit 20fbd4593ff2 ("libxfs:
format the log with valid log record headers"), which I think is what
you're looking for..? That went in around v4.5 or so, so I suppose
anything earlier than that is affected.
Brian
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index b445e8ce4a7d21..b3ea546508dc93 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2999,7 +2999,7 @@ xlog_do_recovery_pass(
> int error = 0, h_size, h_len;
> int error2 = 0;
> int bblks, split_bblks;
> - int hblks, split_hblks, wrapped_hblks;
> + int hblks = 1, split_hblks, wrapped_hblks;
> int i;
> struct hlist_head rhash[XLOG_RHASH_SIZE];
> LIST_HEAD (buffer_list);
> @@ -3055,14 +3055,16 @@ xlog_do_recovery_pass(
> if (error)
> goto bread_err1;
>
> - hblks = xlog_logrec_hblks(log, rhead);
> - if (hblks != 1) {
> - kvfree(hbp);
> - hbp = xlog_alloc_buffer(log, hblks);
> + if ((rhead->h_version & cpu_to_be32(XLOG_VERSION_2)) &&
> + h_size > XLOG_HEADER_CYCLE_SIZE) {
> + hblks = DIV_ROUND_UP(h_size, XLOG_HEADER_CYCLE_SIZE);
> + if (hblks > 1) {
> + kvfree(hbp);
> + hbp = xlog_alloc_buffer(log, hblks);
> + }
> }
> } else {
> ASSERT(log->l_sectBBsize == 1);
> - hblks = 1;
> hbp = xlog_alloc_buffer(log, 1);
> h_size = XLOG_BIG_RECORD_BSIZE;
> }
>
Powered by blists - more mailing lists