The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=0118b0c8e58a438a931a5ce1bf8d7ae6208cc61b
commit 0118b0c8e58a438a931a5ce1bf8d7ae6208cc61b Author: Dag-Erling Smørgrav <[email protected]> AuthorDate: 2024-03-06 16:14:01 +0000 Commit: Dag-Erling Smørgrav <[email protected]> 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 }
