From nobody Thu Feb 9 17:41:38 2023 X-Original-To: dev-commits-src-main@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 4PCPLV2sspz3mxyZ; Thu, 9 Feb 2023 17:41:38 +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 4PCPLV1rv5z4YWy; Thu, 9 Feb 2023 17:41:38 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1675964498; 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=/gehXv+JlJzprYXbtGCdIE9bBn5KsGtbB72A3WDCN60=; b=omo1Rdv/LEgzNo74+Yb6KAXSzQ9t8gwObjlorfVSe0DXuiQ2rfImRfhsuiBp2171V3fuv+ AEh4hzMQNGCmH/R9hFyK8zEGtBRNY0DHxGK222szFOpHadhyPJuz14+iRP2s+2zXQd7nIb j5zH33letNfr1EIboRSFw4D/HYaNPBW2T+ulz0bsp1AVfvi7Fc57UP1AyZRJyjnXK1AxoB fwuiOwULqf+mc8Xst0897kXNP0NTvG4OAr92vknWxdq1kcnqnMdG0NolW6YikJEmxQSZ10 +olNcysTThxJTmeJq8LwsPiis/5F3ZB1rd6r3ut7tdCXy57NIoqbqWGX3duaIg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1675964498; 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=/gehXv+JlJzprYXbtGCdIE9bBn5KsGtbB72A3WDCN60=; b=mKNiFugjN+3E6PN+YJSJDWakHWn0G05GKJVWryP4ekEMeIPF4m92m8S40guqKuTkRE7rJB uJ/7ZQDKiWaxfPwpy4HCYiVx95L/tJQqU0QQVqhHesnXDpdyHxQUSkkLNOu0BqmAenYN0s 20WAXi2go5ySjIxUHlPQwoiKCwLIPtAs4xTNYKJp1jL1PjWDNgpTQuv77eOeLJyOAtUlGO WtxTkU4J6Cx198vXk//zHYuGhUAJcZk3MKo0y98OK6qwH5eI5LryUf3iUzhnFZB8d/8TLw /TMWIOTtlgN4UUqu3+FhVZ0ezPQqTn/IcLbzxfXbQ15OtyV4NZ76KIj70/5Srg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1675964498; a=rsa-sha256; cv=none; b=rD3MCZE3Uy8r6MySmhJ+UXJoG5W60rf9kgVtXu7ect55KpaVFU06ENXzQx6Ju/M5NS8k9t HzHvu3UUmFnuE4iMU0gi2qDOa2y0s6qI3AkHoSxMg2pS5Y/X4sDTbMjY27n6aPxAY4TmBV QAe6pdsnKE5F6pqpV5cXNTHBrXgApFp6grf9+yeFV7wqkZ/lWNcH3/9W4PYmR/vPCSX7cs gTqBYQuDYKnv3ALvP/neqN16/5m8/MK3d+/KrqQTrqlsyXzHAaiKusUgOzvmIsgQa1oBV5 24Zi1si8Rp62zCQO+qG5BoZAW+uYwdMnQWWpCKd4INmDov/cdMcwFEzqursW0A== 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 4PCPLV0t8CzmG5; Thu, 9 Feb 2023 17:41:38 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 319HfcBp035310; Thu, 9 Feb 2023 17:41:38 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 319Hfcbd035309; Thu, 9 Feb 2023 17:41:38 GMT (envelope-from git) Date: Thu, 9 Feb 2023 17:41:38 GMT Message-Id: <202302091741.319Hfcbd035309@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: =?utf-8?Q?Dag-Erling=20Sm=C3=B8rgrav?= Subject: git: ce6a0c776b70 - main - tarfs: Fix issues revealed by static analysis and testing. List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: ce6a0c776b702f063d4f200de34bfeaddcbb3cb7 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=ce6a0c776b702f063d4f200de34bfeaddcbb3cb7 commit ce6a0c776b702f063d4f200de34bfeaddcbb3cb7 Author: Dag-Erling Smørgrav AuthorDate: 2023-02-09 17:35:28 +0000 Commit: Dag-Erling Smørgrav CommitDate: 2023-02-09 17:35:47 +0000 tarfs: Fix issues revealed by static analysis and testing. * tarfs_alloc_mount(): Remove an unnecessary null check (CID 1504505) and an unused variable. * tarfs_alloc_one(): Verify that the file size is not negative (CID 1504506). While there, also validate the mode, owner and group. * tarfs_vget(), tarfs_zio_init(): Explicitly ignore return value from getnewvnode(), which cannot fail (CID 1504508) * tarfs_lookup_path(): Fix a case where a specially-crafted tarball could trigger a null pointer dereference by first descending into, and then backing out of, a previously unknown directory. (CID 1504515) * mktar: Construct a tarball that triggers the aforementioned null pointer dereference. Reported by: Coverity Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: imp, kib Differential Revision: https://reviews.freebsd.org/D38463 --- sys/fs/tarfs/tarfs_io.c | 2 +- sys/fs/tarfs/tarfs_vfsops.c | 88 +++++++++++++++++++++++++++------------------ sys/fs/tarfs/tarfs_vnops.c | 2 +- tests/sys/fs/tarfs/mktar.c | 43 ++++++++++++++++++++-- 4 files changed, 96 insertions(+), 39 deletions(-) diff --git a/sys/fs/tarfs/tarfs_io.c b/sys/fs/tarfs/tarfs_io.c index c185de8beef1..8837681ac5f0 100644 --- a/sys/fs/tarfs/tarfs_io.c +++ b/sys/fs/tarfs/tarfs_io.c @@ -630,7 +630,7 @@ tarfs_zio_init(struct tarfs_mount *tmp, off_t i, off_t o) zio->idx[zio->curidx].o = zio->opos = o; tmp->zio = zio; TARFS_DPF(ALLOC, "%s: allocated zio index\n", __func__); - getnewvnode("tarfsz", tmp->vfs, &tarfs_znodeops, &zvp); + (void)getnewvnode("tarfsz", tmp->vfs, &tarfs_znodeops, &zvp); zvp->v_data = zio; zvp->v_type = VREG; zvp->v_mount = tmp->vfs; diff --git a/sys/fs/tarfs/tarfs_vfsops.c b/sys/fs/tarfs/tarfs_vfsops.c index a45f005a2bd1..138a57c22e7f 100644 --- a/sys/fs/tarfs/tarfs_vfsops.c +++ b/sys/fs/tarfs/tarfs_vfsops.c @@ -289,7 +289,7 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen, char **endp, char **sepp, struct tarfs_node **retparent, struct tarfs_node **retnode, boolean_t create_dirs) { - struct componentname cn; + struct componentname cn = { }; struct tarfs_node *parent, *tnp; char *sep; size_t len; @@ -304,8 +304,6 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen, if (tnp == NULL) panic("%s: root node not yet created", __func__); - bzero(&cn, sizeof(cn)); - TARFS_DPF(LOOKUP, "%s: Full path: %.*s\n", __func__, (int)namelen, name); @@ -329,24 +327,21 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen, /* nothing */ ; /* check for . and .. */ - if (name[0] == '.' && len <= 2) { - if (len == 1) { - /* . */ - name += len; - namelen -= len; - continue; - } else if (name[1] == '.') { - /* .. */ - if (tnp == tmp->root) { - error = EINVAL; - break; - } - tnp = tnp->parent; - parent = tnp->parent; - name += len; - namelen -= len; - continue; + if (name[0] == '.' && len == 1) { + name += len; + namelen -= len; + continue; + } + if (name[0] == '.' && name[1] == '.' && len == 2) { + if (tnp == tmp->root) { + error = EINVAL; + break; } + tnp = parent; + parent = tnp->parent; + name += len; + namelen -= len; + continue; } /* create parent if necessary */ @@ -441,6 +436,7 @@ tarfs_alloc_one(struct tarfs_mount *tmp, off_t *blknump) struct sbuf *namebuf = NULL; char *exthdr = NULL, *name = NULL, *link = NULL; off_t blknum = *blknump; + int64_t num; int endmarker = 0; char *namep, *sep; struct tarfs_node *parent, *tnp; @@ -511,10 +507,41 @@ again: } /* get standard attributes */ - mode = tarfs_str2int64(hdrp->mode, sizeof(hdrp->mode)); - uid = tarfs_str2int64(hdrp->uid, sizeof(hdrp->uid)); - gid = tarfs_str2int64(hdrp->gid, sizeof(hdrp->gid)); - sz = tarfs_str2int64(hdrp->size, sizeof(hdrp->size)); + num = tarfs_str2int64(hdrp->mode, sizeof(hdrp->mode)); + if (num < 0 || num > ALLPERMS) { + TARFS_DPF(ALLOC, "%s: invalid file mode at %zu\n", + __func__, TARFS_BLOCKSIZE * (blknum - 1)); + mode = S_IRUSR; + } else { + mode = num; + } + 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", + __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", + __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", + __func__, TARFS_BLOCKSIZE * (blknum - 1)); + error = EINVAL; + goto bad; + } else { + sz = num; + } mtime = tarfs_str2int64(hdrp->mtime, sizeof(hdrp->mtime)); rdev = NODEV; TARFS_DPF(ALLOC, "%s: [%c] %zu @%jd %o %d:%d\n", __func__, @@ -777,7 +804,6 @@ tarfs_alloc_mount(struct mount *mp, struct vnode *vp, { struct vattr va; struct thread *td = curthread; - char *fullpath; struct tarfs_mount *tmp; struct tarfs_node *root; off_t blknum; @@ -788,7 +814,6 @@ tarfs_alloc_mount(struct mount *mp, struct vnode *vp, ASSERT_VOP_LOCKED(vp, __func__); tmp = NULL; - fullpath = NULL; TARFS_DPF(ALLOC, "%s: Allocating tarfs mount structure for vp %p\n", __func__, vp); @@ -802,8 +827,7 @@ tarfs_alloc_mount(struct mount *mp, struct vnode *vp, mtime = va.va_mtime.tv_sec; /* Allocate and initialize tarfs mount structure */ - tmp = (struct tarfs_mount *)malloc(sizeof(struct tarfs_mount), - M_TARFSMNT, M_WAITOK | M_ZERO); + tmp = malloc(sizeof(*tmp), M_TARFSMNT, M_WAITOK | M_ZERO); TARFS_DPF(ALLOC, "%s: Allocated mount structure\n", __func__); mp->mnt_data = tmp; @@ -848,9 +872,7 @@ tarfs_alloc_mount(struct mount *mp, struct vnode *vp, return (0); bad: - if (tmp != NULL) - tarfs_free_mount(tmp); - free(fullpath, M_TEMP); + tarfs_free_mount(tmp); return (error); } @@ -1104,9 +1126,7 @@ tarfs_vget(struct mount *mp, ino_t ino, int lkflags, struct vnode **vpp) if (tnp == NULL) return (ENOENT); - error = getnewvnode("tarfs", mp, &tarfs_vnodeops, &vp); - if (error != 0) - goto bad; + (void)getnewvnode("tarfs", mp, &tarfs_vnodeops, &vp); TARFS_DPF(FS, "%s: allocated vnode\n", __func__); vp->v_data = tnp; vp->v_type = tnp->type; diff --git a/sys/fs/tarfs/tarfs_vnops.c b/sys/fs/tarfs/tarfs_vnops.c index 266002bca7b2..99ff39d41271 100644 --- a/sys/fs/tarfs/tarfs_vnops.c +++ b/sys/fs/tarfs/tarfs_vnops.c @@ -250,7 +250,7 @@ tarfs_lookup(struct vop_cachedlookup_args *ap) static int tarfs_readdir(struct vop_readdir_args *ap) { - struct dirent cde; + struct dirent cde = { }; struct tarfs_node *current, *tnp; struct vnode *vp; struct uio *uio; diff --git a/tests/sys/fs/tarfs/mktar.c b/tests/sys/fs/tarfs/mktar.c index e1b1183af114..9b3d7910a12c 100644 --- a/tests/sys/fs/tarfs/mktar.c +++ b/tests/sys/fs/tarfs/mktar.c @@ -41,6 +41,8 @@ #define PROGNAME "mktar" #define SUBDIRNAME "directory" +#define EMPTYDIRNAME "empty" +#define NORMALFILENAME "file" #define SPARSEFILENAME "sparse_file" #define HARDLINKNAME "hard_link" #define SHORTLINKNAME "short_link" @@ -61,6 +63,25 @@ static void verbose(const char *fmt, ...) fprintf(stderr, "\n"); } +static void +mknormalfile(const char *filename, mode_t mode) +{ + char buf[512]; + ssize_t res; + int fd; + + if ((fd = open(filename, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) + err(1, "%s", filename); + for (unsigned int i = 0; i < sizeof(buf); i++) + buf[i] = 32 + i % 64; + res = write(fd, buf, sizeof(buf)); + if (res < 0) + err(1, "%s", filename); + if (res != sizeof(buf)) + errx(1, "%s: short write", filename); + close(fd); +} + static void mksparsefile(const char *filename, mode_t mode) { @@ -68,7 +89,7 @@ mksparsefile(const char *filename, mode_t mode) ssize_t res; int fd; - if ((fd = open(filename, O_RDWR|O_CREAT|O_TRUNC, mode)) < 0) + if ((fd = open(filename, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) err(1, "%s", filename); for (unsigned int i = 33; i <= 126; i++) { memset(buf, i, sizeof(buf)); @@ -106,6 +127,15 @@ mktar(void) if (mkdir(SUBDIRNAME, 0755) != 0) err(1, "%s", SUBDIRNAME); + /* create a second subdirectory which will remain empty */ + verbose("mkdir %s", EMPTYDIRNAME); + if (mkdir(EMPTYDIRNAME, 0755) != 0) + err(1, "%s", EMPTYDIRNAME); + + /* create a normal file */ + verbose("creating %s", NORMALFILENAME); + mknormalfile(NORMALFILENAME, 0644); + /* create a sparse file */ verbose("creating %s", SPARSEFILENAME); mksparsefile(SPARSEFILENAME, 0644); @@ -198,7 +228,12 @@ main(int argc, char *argv[]) #if 0 "--options", "zstd:frame-per-file", #endif - ".", + "./" EMPTYDIRNAME "/../" NORMALFILENAME, + "./" SPARSEFILENAME, + "./" HARDLINKNAME, + "./" SHORTLINKNAME, + "./" SUBDIRNAME, + "./" LONGLINKNAME, NULL); err(1, "execlp()"); } @@ -222,7 +257,9 @@ main(int argc, char *argv[]) (void)unlink(HARDLINKNAME); verbose("rm %s", SPARSEFILENAME); (void)unlink(SPARSEFILENAME); - verbose("rm %s", SUBDIRNAME); + verbose("rmdir %s", EMPTYDIRNAME); + (void)rmdir(EMPTYDIRNAME); + verbose("rmdir %s", SUBDIRNAME); (void)rmdir(SUBDIRNAME); verbose("cd -"); exit(0);