git: 0118b0c8e58a - main - tarfs: Fix checksum calculation.

The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=0118b0c8e58a438a931a5ce1bf8d7ae6208cc61b

commit 0118b0c8e58a438a931a5ce1bf8d7ae6208cc61b
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-03-06 16:14:01 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-03-06 16:14:01 +0000

    tarfs: Fix checksum calculation.
    
    The checksum code assumed that struct ustar_header filled an entire
    block and calculcated the checksum based on the size of the structure.
    The header is in fact only 500 bytes long while the checksum covers
    the entire block (“logical record” in POSIX terms).  Add padding and
    an assertion, and clean up the checksum code.
    
    MFC after:      3 days
    Sponsored by:   Juniper Networks, Inc.
    Sponsored by:   Klara, Inc.
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D44226
---
 sys/fs/tarfs/tarfs_vfsops.c      | 31 ++++++++++++++++++++-----------
 tests/sys/fs/tarfs/tarfs_test.sh | 22 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/sys/fs/tarfs/tarfs_vfsops.c b/sys/fs/tarfs/tarfs_vfsops.c
index 33cc019028e6..ce896c5841c0 100644
--- a/sys/fs/tarfs/tarfs_vfsops.c
+++ b/sys/fs/tarfs/tarfs_vfsops.c
@@ -56,7 +56,7 @@
 #include <fs/tarfs/tarfs.h>
 #include <fs/tarfs/tarfs_dbg.h>
 
-CTASSERT(ZERO_REGION_SIZE > TARFS_BLOCKSIZE);
+CTASSERT(ZERO_REGION_SIZE >= TARFS_BLOCKSIZE);
 
 struct ustar_header {
 	char	name[100];		/* File name */
@@ -75,8 +75,11 @@ struct ustar_header {
 	char	major[8];		/* Device major number */
 	char	minor[8];		/* Device minor number */
 	char	prefix[155];		/* Path prefix */
+	char	_pad[12];
 };
 
+CTASSERT(sizeof(struct ustar_header) == TARFS_BLOCKSIZE);
+
 #define	TAR_EOF			((off_t)-1)
 
 #define	TAR_TYPE_FILE		'0'
@@ -202,22 +205,27 @@ static boolean_t
 tarfs_checksum(struct ustar_header *hdrp)
 {
 	const unsigned char *ptr;
-	int64_t checksum, hdrsum;
-	size_t idx;
+	unsigned long checksum, hdrsum;
 
-	hdrsum = tarfs_str2int64(hdrp->checksum, sizeof(hdrp->checksum));
-	TARFS_DPF(CHECKSUM, "%s: header checksum %lx\n", __func__, hdrsum);
+	if (tarfs_str2int64(hdrp->checksum, sizeof(hdrp->checksum), &hdrsum) != 0) {
+		TARFS_DPF(CHECKSUM, "%s: invalid header checksum \"%.*s\"\n",
+		    __func__, (int)sizeof(hdrp->checksum), hdrp->checksum);
+		return (false);
+	}
+	TARFS_DPF(CHECKSUM, "%s: header checksum \"%.*s\" = %#lo\n", __func__,
+	    (int)sizeof(hdrp->checksum), hdrp->checksum, hdrsum);
 
 	checksum = 0;
 	for (ptr = (const unsigned char *)hdrp;
 	     ptr < (const unsigned char *)hdrp->checksum; ptr++)
 		checksum += *ptr;
-	for (idx = 0; idx < sizeof(hdrp->checksum); idx++)
+	for (;
+	     ptr < (const unsigned char *)hdrp->typeflag; ptr++)
 		checksum += 0x20;
-	for (ptr = (const unsigned char *)hdrp->typeflag;
+	for (;
 	     ptr < (const unsigned char *)(hdrp + 1); ptr++)
 		checksum += *ptr;
-	TARFS_DPF(CHECKSUM, "%s: calc unsigned checksum %lx\n", __func__,
+	TARFS_DPF(CHECKSUM, "%s: calc unsigned checksum %#lo\n", __func__,
 	    checksum);
 	if (hdrsum == checksum)
 		return (true);
@@ -230,12 +238,13 @@ tarfs_checksum(struct ustar_header *hdrp)
 	for (ptr = (const unsigned char *)hdrp;
 	     ptr < (const unsigned char *)&hdrp->checksum; ptr++)
 		checksum += *((const signed char *)ptr);
-	for (idx = 0; idx < sizeof(hdrp->checksum); idx++)
+	for (;
+	     ptr < (const unsigned char *)&hdrp->typeflag; ptr++)
 		checksum += 0x20;
-	for (ptr = (const unsigned char *)&hdrp->typeflag;
+	for (;
 	     ptr < (const unsigned char *)(hdrp + 1); ptr++)
 		checksum += *((const signed char *)ptr);
-	TARFS_DPF(CHECKSUM, "%s: calc signed checksum %lx\n", __func__,
+	TARFS_DPF(CHECKSUM, "%s: calc signed checksum %#lo\n", __func__,
 	    checksum);
 	if (hdrsum == checksum)
 		return (true);
diff --git a/tests/sys/fs/tarfs/tarfs_test.sh b/tests/sys/fs/tarfs/tarfs_test.sh
index 6f45062c18d9..46aefd832d5d 100644
--- a/tests/sys/fs/tarfs/tarfs_test.sh
+++ b/tests/sys/fs/tarfs/tarfs_test.sh
@@ -285,6 +285,27 @@ tarfs_linktononexistent_body() {
 		  mount -rt tarfs tarfs_linktononexistent.tar "${mnt}"
 }
 tarfs_linktononexistent_cleanup() {
+	tarfs_cleanup
+}
+
+atf_test_case tarfs_checksum cleanup
+tarfs_checksum_head() {
+	atf_set "descr" "Verify that the checksum covers header padding"
+	atf_set "require.user" "root"
+}
+tarfs_checksum_body() {
+	kldload -n tarfs || atf_skip "This test requires tarfs and could not load it"
+	mkdir "${mnt}"
+	touch f
+	tar -cf tarfs_checksum.tar f
+	truncate -s 500 tarfs_checksum.tar
+	printf "\1\1\1\1\1\1\1\1\1\1\1\1" >> tarfs_checksum.tar
+	dd if=/dev/zero bs=512 count=2 >> tarfs_checksum.tar
+	hexdump -C tarfs_checksum.tar
+	atf_check -s not-exit:0 -e match:"Invalid" \
+		  mount -rt tarfs tarfs_checksum.tar "${mnt}"
+}
+tarfs_checksum_cleanup() {
 	umount "${mnt}" || true
 }
 
@@ -302,4 +323,5 @@ atf_init_test_cases() {
 	atf_add_test_case tarfs_emptylink
 	atf_add_test_case tarfs_linktodir
 	atf_add_test_case tarfs_linktononexistent
+	atf_add_test_case tarfs_checksum
 }