Re: [Linux kernel bug] KASAN: slab-out-of-bounds Read in xlog_cksum

[Brian Foster]

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