From nobody Mon Mar 11 12:35:08 2024 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Ttbp51HwDz5Ctp3; Mon, 11 Mar 2024 12:35:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Ttbp46rD9z4BsQ; Mon, 11 Mar 2024 12:35:08 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710160508; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2v9X75eRtpRGi+TFs9z1S+f2Z/z6GApYz0SVpAKnShU=; b=AiaUZ7WcOgr5S+Eugx86nfBLHr+KnhqnefNIpQD64x42frE6MiBFtuFv5nQO6SuwYKXQK+ ZdJxjpPb9JXMRu0f7FkzmpGT+Pknte7B7TthpCX4+fad4nqzWurITDfqak7aR8upmUTwpC FXcFTxIyWBXtjE+XvKa/CT6YQd+VTvm0dOITIXei7wt2a0XFc5bSS6b3Vnv/h9/FaCcSIO GaqSCDoQo1s5J6uDy5r/4iTwO0YLIJlpYXvAzJX0d+dLEwqUekIaT5XJvBWg4JwRqYplfC 1w7priAW5J7bWC0pNG3VCpzm+y5POn5gOWO42aYCJwuMxKpw3zjm/bDMESLEoQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1710160508; a=rsa-sha256; cv=none; b=evREWOGg6cnHUG8XbqO+XNzP8yeQT3SfFWxCD6fCdMfFtkqx03qqffCG6Vt9SsOR8OK1Gl ETEOcMbjrOxasPlzE1gV4pxXEM99/2f/HQZPDGuf8BRR1qUCpZugEMnr3E4wk124exqWK0 wL0oBXrRdBxNwII5F+xrXvkzXfh06Xkp7Z5t5ukcS4AkIJgZ94CLvaI63qVL2NGL9W85u5 ITFGTvmPPvm1vEctAzcq3m/mP51qakwrODgfhDd8XEfm5qIMs+ZBf/8A+ryHaH3OI7kluM 4n9t5UZzJV54/cuw1PYWL6Z6zTSC0Gr3jqsYlXbywBU09WIRnH30PeaNDdaxMA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710160508; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2v9X75eRtpRGi+TFs9z1S+f2Z/z6GApYz0SVpAKnShU=; b=dpcAAM+23cEkqR3FYQBt/zNaVII4tsJG0LA55JTI4LE+wzOIq4srCuAEcmSO6+Foso9E6A SxSsqqYy1kDkvYNnqgWBgUm5kqvcUvGREP/9Ikt9PPRa6r924IMYUmiwOG5PtylxyEAEZ3 WCBEFc4F2//KMgCdYhUlVByCwm3HOGmqf0N1JNnWc3Nzha1Rw03gVNU+NM8mT7G4B6KpTu bHX6w7Zz1FK1areLL+xiKBzT2f8oklTfK1EvgfeD0JbziMWMKDMI4wd/8s1+KLX3NRDbQT qLTC4C6jTQBjSmHouvRcAkHuDGhwZ9d3mZg9DIOnaYGk8KFqehdbyhsRydfxPw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Ttbp46CkkzT73; Mon, 11 Mar 2024 12:35:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 42BCZ8xu029001; Mon, 11 Mar 2024 12:35:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 42BCZ8wA028998; Mon, 11 Mar 2024 12:35:08 GMT (envelope-from git) Date: Mon, 11 Mar 2024 12:35:08 GMT Message-Id: <202403111235.42BCZ8wA028998@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= Subject: git: 08e799c0cc23 - stable/14 - tarfs: Fix two input validation issues. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: des X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 08e799c0cc235b8c5a98921653127a915a985220 Auto-Submitted: auto-generated The branch stable/14 has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=08e799c0cc235b8c5a98921653127a915a985220 commit 08e799c0cc235b8c5a98921653127a915a985220 Author: Dag-Erling Smørgrav AuthorDate: 2024-03-06 16:13:42 +0000 Commit: Dag-Erling Smørgrav CommitDate: 2024-03-11 12:19:06 +0000 tarfs: Fix two input validation issues. * Reject hard or soft links with an empty target path. Currently, a debugging kernel will hit an assertion in tarfs_lookup_path() while a non-debugging kernel will happily create a link to the mount root. * Use a temporary variable to store the result of the link target path, and copy it to tnp->other only once we have found it to be valid. Otherwise we error out after creating a reference to the target but before incrementing the target's reference count, which results in a use-after-free situation in the cleanup code. * Correctly return ENOENT from tarfs_lookup_path() if the requested path was not found and create_dirs is false. Luckily, existing callers did not rely solely on the return value. MFC after: 3 days PR: 277360 Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: sjg Differential Revision: https://reviews.freebsd.org/D44161 (cherry picked from commit 38b3683592d4c20a74f52a6e8e29368e6fa61858) tarfs: Improve validation of numeric fields. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: sjg, allanjude Differential Revision: https://reviews.freebsd.org/D44166 (cherry picked from commit 8427d94ce05682abb6c75e2a27c8c497962c0dc5) tarfs: Avoid overflow in exthdr calculation. MFC after: 3 days PR: 277420 Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D44202 (cherry picked from commit c291b7914e1db9469cc820abcb1f5dde7a6f7f28) tarfs: Remove unnecessary hack and obsolete comment. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: allanjude Differential Revision: https://reviews.freebsd.org/D44203 (cherry picked from commit e212f0c0666e7d3a24dce03b8c88920d14b80e47) 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 (cherry picked from commit 0118b0c8e58a438a931a5ce1bf8d7ae6208cc61b) tarfs: Factor out common test code. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: allanjude Differential Revision: https://reviews.freebsd.org/D44227 (cherry picked from commit 32b8aac6f9b77a1c4326083472d634e5de427547) tarfs: Fix checksum on 32-bit platforms. MFC after: 3 days Fixes: b56872332e47786afc09515a4daaf1388da4d73c Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: bapt Differential Revision: https://reviews.freebsd.org/D44261 (cherry picked from commit cbddb2f02c7687d1039abcffd931e94e481c11a5) --- sys/fs/tarfs/tarfs_vfsops.c | 222 ++++++++++++++++++++++----------------- tests/sys/fs/tarfs/Makefile | 2 +- tests/sys/fs/tarfs/tarfs_test.sh | 120 ++++++++++++++++++--- tests/sys/fs/tarfs/tarsum.c | 128 ++++++++++++++++++++++ 4 files changed, 357 insertions(+), 115 deletions(-) diff --git a/sys/fs/tarfs/tarfs_vfsops.c b/sys/fs/tarfs/tarfs_vfsops.c index 4489b41699ec..d1af7070e706 100644 --- a/sys/fs/tarfs/tarfs_vfsops.c +++ b/sys/fs/tarfs/tarfs_vfsops.c @@ -2,7 +2,7 @@ * SPDX-License-Identifier: BSD-2-Clause * * Copyright (c) 2013 Juniper Networks, Inc. - * Copyright (c) 2022-2023 Klara, Inc. + * Copyright (c) 2022-2024 Klara, Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -56,7 +56,7 @@ #include #include -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' @@ -113,58 +116,46 @@ static const char *tarfs_opts[] = { }; /* - * Reads a len-width signed octal number from strp. Returns the value. - * XXX Does not report errors. + * Reads a len-width signed octal number from strp. Returns 0 on success + * and non-zero on error. */ -static int64_t -tarfs_str2octal(const char *strp, size_t len) +static int +tarfs_str2octal(const char *strp, size_t len, int64_t *num) { int64_t val; size_t idx; int sign; - /* - * Skip leading spaces or tabs. - * XXX why? POSIX requires numeric fields to be 0-padded. - */ - for (idx = 0; idx < len; idx++) - if (strp[idx] != ' ' && strp[idx] != '\t') - break; - - if (idx == len) - return (0); - + idx = 0; if (strp[idx] == '-') { sign = -1; idx++; - } else + } else { sign = 1; + } val = 0; - for (; idx < len; idx++) { + for (; idx < len && strp[idx] != '\0' && strp[idx] != ' '; idx++) { if (strp[idx] < '0' || strp[idx] > '7') - break; + return (EINVAL); val <<= 3; - val += (strp[idx] - '0'); - - /* Truncate on overflow */ - if (val > INT64_MAX / 8) { - val = INT64_MAX; - break; - } + val += strp[idx] - '0'; + if (val > INT64_MAX / 8) + return (ERANGE); } - return (sign > 0) ? val : -val; + *num = val * sign; + return (0); } /* * Reads a len-byte extended numeric value from strp. The first byte has * bit 7 set to indicate the format; the remaining 7 bits + the (len - 1) * bytes that follow form a big-endian signed two's complement binary - * number. Returns the value. XXX Does not report errors. + * number. Returns 0 on success and non-zero on error; */ -static int64_t -tarfs_str2base256(const char *strp, size_t len) +static int +tarfs_str2base256(const char *strp, size_t len, int64_t *num) { int64_t val; size_t idx; @@ -183,37 +174,27 @@ tarfs_str2base256(const char *strp, size_t len) for (idx = 1; idx < len; idx++) { val <<= 8; val |= (0xff & (int64_t)strp[idx]); - - /* Truncate on overflow and underflow */ - if (val > INT64_MAX / 256) { - val = INT64_MAX; - break; - } else if (val < INT64_MAX / 256) { - val = INT64_MIN; - break; - } + if (val > INT64_MAX / 256 || val < INT64_MIN / 256) + return (ERANGE); } - return (val); + *num = val; + return (0); } /* * Read a len-byte numeric field from strp. If bit 7 of the first byte it * set, assume an extended numeric value (signed two's complement); * otherwise, assume a signed octal value. - * - * XXX practically no error checking or handling */ -static int64_t -tarfs_str2int64(const char *strp, size_t len) +static int +tarfs_str2int64(const char *strp, size_t len, int64_t *num) { - if (len < 1) - return (0); - + return (EINVAL); if ((strp[0] & 0x80) != 0) - return (tarfs_str2base256(strp, len)); - return (tarfs_str2octal(strp, len)); + return (tarfs_str2base256(strp, len, num)); + return (tarfs_str2octal(strp, len, num)); } /* @@ -225,21 +206,26 @@ tarfs_checksum(struct ustar_header *hdrp) { const unsigned char *ptr; int64_t checksum, hdrsum; - size_t idx; - 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); @@ -252,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); @@ -379,8 +366,10 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen, tnp = tarfs_lookup_node(parent, NULL, &cn); if (tnp == NULL) { do_lookup = false; - if (!create_dirs) + if (!create_dirs) { + error = ENOENT; break; + } } } name += cn.cn_namelen; @@ -451,7 +440,7 @@ tarfs_alloc_one(struct tarfs_mount *tmp, off_t *blknump) int64_t num; int endmarker = 0; char *namep, *sep; - struct tarfs_node *parent, *tnp; + struct tarfs_node *parent, *tnp, *other; size_t namelen = 0, linklen = 0, realsize = 0, sz; ssize_t res; dev_t rdev; @@ -519,42 +508,47 @@ again: } /* get standard attributes */ - num = tarfs_str2int64(hdrp->mode, sizeof(hdrp->mode)); - if (num < 0 || num > (S_IFMT|ALLPERMS)) { + if (tarfs_str2int64(hdrp->mode, sizeof(hdrp->mode), &num) != 0 || + num < 0 || num > (S_IFMT|ALLPERMS)) { TARFS_DPF(ALLOC, "%s: invalid file mode at %zu\n", __func__, TARFS_BLOCKSIZE * (blknum - 1)); mode = S_IRUSR; } else { mode = num & ALLPERMS; } - num = tarfs_str2int64(hdrp->uid, sizeof(hdrp->uid)); - if (num < 0 || num > UID_MAX) { - TARFS_DPF(ALLOC, "%s: UID out of range at %zu\n", + if (tarfs_str2int64(hdrp->uid, sizeof(hdrp->uid), &num) != 0 || + num < 0 || num > UID_MAX) { + TARFS_DPF(ALLOC, "%s: invalid UID at %zu\n", __func__, TARFS_BLOCKSIZE * (blknum - 1)); uid = tmp->root->uid; mode &= ~S_ISUID; } else { uid = num; } - num = tarfs_str2int64(hdrp->gid, sizeof(hdrp->gid)); - if (num < 0 || num > GID_MAX) { - TARFS_DPF(ALLOC, "%s: GID out of range at %zu\n", + if (tarfs_str2int64(hdrp->gid, sizeof(hdrp->gid), &num) != 0 || + num < 0 || num > GID_MAX) { + TARFS_DPF(ALLOC, "%s: invalid GID at %zu\n", __func__, TARFS_BLOCKSIZE * (blknum - 1)); gid = tmp->root->gid; mode &= ~S_ISGID; } else { gid = num; } - num = tarfs_str2int64(hdrp->size, sizeof(hdrp->size)); - if (num < 0) { - TARFS_DPF(ALLOC, "%s: negative size at %zu\n", + if (tarfs_str2int64(hdrp->size, sizeof(hdrp->size), &num) != 0 || + num < 0) { + TARFS_DPF(ALLOC, "%s: invalid size at %zu\n", + __func__, TARFS_BLOCKSIZE * (blknum - 1)); + error = EINVAL; + goto bad; + } + sz = num; + if (tarfs_str2int64(hdrp->mtime, sizeof(hdrp->mtime), &num) != 0) { + TARFS_DPF(ALLOC, "%s: invalid modification time at %zu\n", __func__, TARFS_BLOCKSIZE * (blknum - 1)); error = EINVAL; goto bad; - } else { - sz = num; } - mtime = tarfs_str2int64(hdrp->mtime, sizeof(hdrp->mtime)); + mtime = num; rdev = NODEV; TARFS_DPF(ALLOC, "%s: [%c] %zu @%jd %o %d:%d\n", __func__, hdrp->typeflag[0], sz, (intmax_t)mtime, mode, uid, gid); @@ -598,7 +592,8 @@ again: error = EINVAL; goto bad; } - if (line + len > exthdr + sz) { + if ((uintptr_t)line + len < (uintptr_t)line || + line + len > exthdr + sz) { TARFS_DPF(ALLOC, "%s: exthdr overflow\n", __func__); error = EINVAL; @@ -732,43 +727,82 @@ again: link = hdrp->linkname; linklen = strnlen(link, sizeof(hdrp->linkname)); } - error = tarfs_alloc_node(tmp, namep, sep - namep, VREG, - 0, 0, 0, 0, 0, 0, 0, NULL, 0, parent, &tnp); - if (error != 0) { + if (linklen == 0) { + TARFS_DPF(ALLOC, "%s: %.*s: link without target\n", + __func__, (int)namelen, name); + error = EINVAL; goto bad; } error = tarfs_lookup_path(tmp, link, linklen, NULL, - NULL, NULL, &tnp->other, false); - if (tnp->other == NULL || - tnp->other->type != VREG || - tnp->other->other != NULL) { - TARFS_DPF(ALLOC, "%s: %.*s: dead hard link to %.*s\n", + NULL, NULL, &other, false); + if (error != 0 || other == NULL || + other->type != VREG || other->other != NULL) { + TARFS_DPF(ALLOC, "%s: %.*s: invalid link to %.*s\n", __func__, (int)namelen, name, (int)linklen, link); error = EINVAL; goto bad; } - tnp->other->nlink++; + error = tarfs_alloc_node(tmp, namep, sep - namep, VREG, + 0, 0, 0, 0, 0, 0, 0, NULL, 0, parent, &tnp); + if (error == 0) { + tnp->other = other; + tnp->other->nlink++; + } break; case TAR_TYPE_SYMLINK: if (link == NULL) { link = hdrp->linkname; linklen = strnlen(link, sizeof(hdrp->linkname)); } + if (linklen == 0) { + TARFS_DPF(ALLOC, "%s: %.*s: link without target\n", + __func__, (int)namelen, name); + error = EINVAL; + goto bad; + } error = tarfs_alloc_node(tmp, namep, sep - namep, VLNK, 0, linklen, mtime, uid, gid, mode, flags, link, 0, parent, &tnp); break; case TAR_TYPE_BLOCK: - major = tarfs_str2int64(hdrp->major, sizeof(hdrp->major)); - minor = tarfs_str2int64(hdrp->minor, sizeof(hdrp->minor)); + if (tarfs_str2int64(hdrp->major, sizeof(hdrp->major), &num) != 0 || + num < 0 || num > INT_MAX) { + TARFS_DPF(ALLOC, "%s: %.*s: invalid device major\n", + __func__, (int)namelen, name); + error = EINVAL; + goto bad; + } + major = num; + if (tarfs_str2int64(hdrp->minor, sizeof(hdrp->minor), &num) != 0 || + num < 0 || num > INT_MAX) { + TARFS_DPF(ALLOC, "%s: %.*s: invalid device minor\n", + __func__, (int)namelen, name); + error = EINVAL; + goto bad; + } + minor = num; rdev = makedev(major, minor); error = tarfs_alloc_node(tmp, namep, sep - namep, VBLK, 0, 0, mtime, uid, gid, mode, flags, NULL, rdev, parent, &tnp); break; case TAR_TYPE_CHAR: - major = tarfs_str2int64(hdrp->major, sizeof(hdrp->major)); - minor = tarfs_str2int64(hdrp->minor, sizeof(hdrp->minor)); + if (tarfs_str2int64(hdrp->major, sizeof(hdrp->major), &num) != 0 || + num < 0 || num > INT_MAX) { + TARFS_DPF(ALLOC, "%s: %.*s: invalid device major\n", + __func__, (int)namelen, name); + error = EINVAL; + goto bad; + } + major = num; + if (tarfs_str2int64(hdrp->minor, sizeof(hdrp->minor), &num) != 0 || + num < 0 || num > INT_MAX) { + TARFS_DPF(ALLOC, "%s: %.*s: invalid device minor\n", + __func__, (int)namelen, name); + error = EINVAL; + goto bad; + } + minor = num; rdev = makedev(major, minor); error = tarfs_alloc_node(tmp, namep, sep - namep, VCHR, 0, 0, mtime, uid, gid, mode, flags, NULL, rdev, @@ -856,16 +890,8 @@ tarfs_alloc_mount(struct mount *mp, struct vnode *vp, tmp->vfs = mp; tmp->mtime = mtime; - /* - * XXX The decompression layer passes everything through the - * buffer cache, and the buffer cache wants to know our blocksize, - * but mnt_stat normally isn't populated until after we return, so - * we have to cheat a bit. - */ + /* Initialize I/O layer */ tmp->iosize = 1U << tarfs_ioshift; - mp->mnt_stat.f_iosize = tmp->iosize; - - /* Initialize decompression layer */ error = tarfs_io_init(tmp); if (error != 0) goto bad; diff --git a/tests/sys/fs/tarfs/Makefile b/tests/sys/fs/tarfs/Makefile index b16c6544d33f..72355a08a158 100644 --- a/tests/sys/fs/tarfs/Makefile +++ b/tests/sys/fs/tarfs/Makefile @@ -3,7 +3,7 @@ PACKAGE= tests TESTSDIR= ${TESTSBASE}/sys/fs/tarfs BINDIR= ${TESTSDIR} -PROGS+= mktar +PROGS+= mktar tarsum ATF_TESTS_SH+= tarfs_test diff --git a/tests/sys/fs/tarfs/tarfs_test.sh b/tests/sys/fs/tarfs/tarfs_test.sh index 15354aac501a..2a5dfc434201 100644 --- a/tests/sys/fs/tarfs/tarfs_test.sh +++ b/tests/sys/fs/tarfs/tarfs_test.sh @@ -2,7 +2,7 @@ #- # SPDX-License-Identifier: BSD-2-Clause # -# Copyright (c) 2023 Klara, Inc. +# Copyright (c) 2023-2024 Klara, Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions @@ -43,15 +43,27 @@ mktar() { "$(atf_get_srcdir)"/mktar ${TARFS_USE_GNU_TAR+-g} "$@" } +tarsum() { + "$(atf_get_srcdir)"/tarsum +} + +tarfs_setup() { + kldload -n tarfs || atf_skip "This test requires tarfs and could not load it" + mkdir "${mnt}" +} + +tarfs_cleanup() { + umount -f "${mnt}" 2>/dev/null || true +} + atf_test_case tarfs_basic cleanup tarfs_basic_head() { atf_set "descr" "Basic function test" atf_set "require.user" "root" } tarfs_basic_body() { + tarfs_setup local tarball="${PWD}/tarfs_test.tar.zst" - kldload -n tarfs || atf_skip "This test requires tarfs and could not load it" - mkdir "${mnt}" mktar "${tarball}" atf_check mount -rt tarfs "${tarball}" "${mnt}" atf_check -o match:"^${tarball} on ${mnt} \(tarfs," mount @@ -68,7 +80,7 @@ tarfs_basic_body() { atf_check -o inline:"3,40755\n" stat -f%l,%p "${mnt}" } tarfs_basic_cleanup() { - umount "${mnt}" || true + tarfs_cleanup } atf_test_case tarfs_basic_gnu cleanup @@ -91,8 +103,7 @@ tarfs_notdir_device_head() { atf_set "require.user" "root" } tarfs_notdir_device_body() { - kldload -n tarfs || atf_skip "This test requires tarfs and could not load it" - mkdir "${mnt}" + tarfs_setup atf_check mknod d b 0xdead 0xbeef tar -cf tarfs_notdir.tar d rm d @@ -103,7 +114,7 @@ tarfs_notdir_device_body() { mount -rt tarfs tarfs_notdir.tar "${mnt}" } tarfs_notdir_device_cleanup() { - umount "${mnt}" || true + tarfs_cleanup } atf_test_case tarfs_notdir_device_gnu cleanup @@ -126,8 +137,7 @@ tarfs_notdir_dot_head() { atf_set "require.user" "root" } tarfs_notdir_dot_body() { - kldload -n tarfs || atf_skip "This test requires tarfs and could not load it" - mkdir "${mnt}" + tarfs_setup echo "hello" >d tar -cf tarfs_notdir.tar d rm d @@ -138,7 +148,7 @@ tarfs_notdir_dot_body() { mount -rt tarfs tarfs_notdir.tar "${mnt}" } tarfs_notdir_dot_cleanup() { - umount "${mnt}" || true + tarfs_cleanup } atf_test_case tarfs_notdir_dot_gnu cleanup @@ -161,8 +171,7 @@ tarfs_notdir_dotdot_head() { atf_set "require.user" "root" } tarfs_notdir_dotdot_body() { - kldload -n tarfs || atf_skip "This test requires tarfs and could not load it" - mkdir "${mnt}" + tarfs_setup echo "hello" >d tar -cf tarfs_notdir.tar d rm d @@ -173,7 +182,7 @@ tarfs_notdir_dotdot_body() { mount -rt tarfs tarfs_notdir.tar "${mnt}" } tarfs_notdir_dotdot_cleanup() { - umount "${mnt}" || true + tarfs_cleanup } atf_test_case tarfs_notdir_dotdot_gnu cleanup @@ -196,8 +205,7 @@ tarfs_notdir_file_head() { atf_set "require.user" "root" } tarfs_notdir_file_body() { - kldload -n tarfs || atf_skip "This test requires tarfs and could not load it" - mkdir "${mnt}" + tarfs_setup echo "hello" >d tar -cf tarfs_notdir.tar d rm d @@ -208,7 +216,7 @@ tarfs_notdir_file_body() { mount -rt tarfs tarfs_notdir.tar "${mnt}" } tarfs_notdir_file_cleanup() { - umount "${mnt}" || true + tarfs_cleanup } atf_test_case tarfs_notdir_file_gnu cleanup @@ -225,6 +233,82 @@ tarfs_notdir_file_gnu_cleanup() { tarfs_notdir_file_cleanup } +atf_test_case tarfs_emptylink cleanup +tarfs_emptylink_head() { + atf_set "descr" "Regression test for PR 277360: empty link target" + atf_set "require.user" "root" +} +tarfs_emptylink_body() { + tarfs_setup + touch z + ln -f z hard + ln -fs z soft + tar -cf - z hard soft | dd bs=512 skip=1 | tr z '\0' | \ + tarsum >> tarfs_emptylink.tar + atf_check -s not-exit:0 -e match:"Invalid" \ + mount -rt tarfs tarfs_emptylink.tar "${mnt}" +} +tarfs_emptylink_cleanup() { + tarfs_cleanup +} + +atf_test_case tarfs_linktodir cleanup +tarfs_linktodir_head() { + atf_set "descr" "Regression test for PR 277360: link to directory" + atf_set "require.user" "root" +} +tarfs_linktodir_body() { + tarfs_setup + mkdir d + tar -cf - d | dd bs=512 count=1 > tarfs_linktodir.tar + rmdir d + touch d + ln -f d link + tar -cf - d link | dd bs=512 skip=1 >> tarfs_linktodir.tar + atf_check -s not-exit:0 -e match:"Invalid" \ + mount -rt tarfs tarfs_linktodir.tar "${mnt}" +} +tarfs_linktodir_cleanup() { + tarfs_cleanup +} + +atf_test_case tarfs_linktononexistent cleanup +tarfs_linktononexistent_head() { + atf_set "descr" "Regression test for PR 277360: link to nonexistent target" + atf_set "require.user" "root" +} +tarfs_linktononexistent_body() { + tarfs_setup + touch f + ln -f f link + tar -cf - f link | dd bs=512 skip=1 >> tarfs_linktononexistent.tar + atf_check -s not-exit:0 -e match:"Invalid" \ + 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() { + tarfs_setup + 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() { + tarfs_cleanup +} + atf_init_test_cases() { atf_add_test_case tarfs_basic atf_add_test_case tarfs_basic_gnu @@ -236,4 +320,8 @@ atf_init_test_cases() { atf_add_test_case tarfs_notdir_dotdot_gnu atf_add_test_case tarfs_notdir_file atf_add_test_case tarfs_notdir_file_gnu + atf_add_test_case tarfs_emptylink + atf_add_test_case tarfs_linktodir + atf_add_test_case tarfs_linktononexistent + atf_add_test_case tarfs_checksum } diff --git a/tests/sys/fs/tarfs/tarsum.c b/tests/sys/fs/tarfs/tarsum.c new file mode 100644 index 000000000000..73ead2230a5e --- /dev/null +++ b/tests/sys/fs/tarfs/tarsum.c @@ -0,0 +1,128 @@ +/*- + * Copyright (c) 2024 Klara, Inc. + * + * SPDX-License-Identifier: BSD-2-Clause + * + * This program reads a tarball from stdin, recalculates the checksums of + * all ustar records within it, and writes the result to stdout. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +static bool opt_v; + +static int +verbose(const char *fmt, ...) +{ + va_list ap; + int ret; + + if (!opt_v) + return (0); + va_start(ap, fmt); + ret = vfprintf(stderr, fmt, ap); + va_end(ap); + return (ret); +} + +static void +tarsum(FILE *in, const char *ifn, FILE *out, const char *ofn) +{ + union { + uint8_t bytes[512]; + struct { + uint8_t prelude[148]; + char checksum[8]; + uint8_t interlude[101]; + char magic[6]; + char version[2]; + char postlude[]; + }; + } ustar; + unsigned long sum; + off_t offset = 0; + ssize_t ret; + size_t i; + + for (;;) { + if ((ret = fread(&ustar, 1, sizeof(ustar), in)) < 0) + err(1, "%s", ifn); + else if (ret == 0) + break; + else if ((size_t)ret < sizeof(ustar)) + errx(1, "%s: Short read", ifn); + if (strcmp(ustar.magic, "ustar") == 0 && + ustar.version[0] == '0' && ustar.version[1] == '0') { + verbose("header found at offset %#lx\n", offset); + verbose("current checksum %.*s\n", + (int)sizeof(ustar.checksum), ustar.checksum); + memset(ustar.checksum, ' ', sizeof(ustar.checksum)); + for (sum = i = 0; i < sizeof(ustar); i++) + sum += ustar.bytes[i]; + verbose("calculated checksum %#lo\n", sum); + sprintf(ustar.checksum, "%#lo", sum); + } + if ((ret = fwrite(&ustar, 1, sizeof(ustar), out)) < 0) + err(1, "%s", ofn); + else if ((size_t)ret < sizeof(ustar)) + errx(1, "%s: Short write", ofn); + offset += sizeof(ustar); + } + verbose("%lu bytes processed\n", offset); +} + +static void +usage(void) +{ + fprintf(stderr, "usage: tarsum [-v] [-o output] [input]\n"); + exit(1); +} + +int +main(int argc, char *argv[]) +{ + const char *ifn, *ofn = NULL; + FILE *in, *out; + int opt; + + while ((opt = getopt(argc, argv, "o:v")) != -1) { + switch (opt) { + case 'o': + ofn = optarg; + break; + case 'v': + opt_v = true; + break; + default: + usage(); + } + } + argc -= optind; + argv += optind; + if (argc == 0 || strcmp(*argv, "-") == 0) { + ifn = "stdin"; + in = stdin; + } else if (argc == 1) { + ifn = *argv; + if ((in = fopen(ifn, "rb")) == NULL) + err(1, "%s", ifn); + } else { + usage(); + } + if (ofn == NULL || strcmp(ofn, "-") == 0) { + ofn = "stdout"; + out = stdout; + } else { + if ((out = fopen(ofn, "wb")) == NULL) + err(1, "%s", ofn); + } + tarsum(in, ifn, out, ofn); + return (0); +}