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

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)?

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