Received-SPF: pass (google.com: domain of linux-kernel+bounces-158628-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 25 Apr 2024 09:57:54 -0400
From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Sam Sun <samsun1006219@gmail.com>, linux-kernel@vger.kernel.org,
	linux-xfs@vger.kernel.org, djwong@kernel.org,
	chandan.babu@oracle.com, syzkaller-bugs@googlegroups.com,
	xrivendell7@gmail.com
Subject: Re: [Linux kernel bug] KASAN: slab-out-of-bounds Read in xlog_cksum
Message-ID: <ZiphYrREkQvxkE-U@bfoster>
References: <CAEkJfYO++C-pxyqzfoXFKEvmMQEnrgkQ2QcG6radAWJMqdXQCQ@mail.gmail.com>
 <ZipWt03PhXs2Yc84@infradead.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZipWt03PhXs2Yc84@infradead.org>

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;
>  	}
>