git: d481dcee72a0 - main - tarfs: Really prevent descending into a non-directory.

The branch main has been updated by des:

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

commit d481dcee72a05580c4c3af4a429b1c08fa846056
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2023-02-20 21:28:53 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2023-02-20 21:29:19 +0000

    tarfs: Really prevent descending into a non-directory.
    
    The previous fix was incorrect: we need to verify that the current node, if it exists, is not a directory, but we were checking the parent node instead.  Address this, add more tests, and fix the test cleanup routines.
    
    PR:             269519, 269561
    Fixes:          ae6cff89738b
    Sponsored by:   Juniper Networks, Inc.
    Sponsored by:   Klara, Inc.
    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D38645
---
 sys/fs/tarfs/tarfs_vfsops.c      | 17 ++++----
 tests/sys/fs/tarfs/tarfs_test.sh | 87 ++++++++++++++++++++++++++++++++++------
 2 files changed, 85 insertions(+), 19 deletions(-)

diff --git a/sys/fs/tarfs/tarfs_vfsops.c b/sys/fs/tarfs/tarfs_vfsops.c
index b3c30e884a9d..059608ea60a5 100644
--- a/sys/fs/tarfs/tarfs_vfsops.c
+++ b/sys/fs/tarfs/tarfs_vfsops.c
@@ -304,8 +304,8 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen,
 	if (tnp == NULL)
 		panic("%s: root node not yet created", __func__);
 
-	TARFS_DPF(LOOKUP, "%s: Full path: %.*s\n", __func__, (int)namelen,
-	    name);
+	TARFS_DPF(LOOKUP, "%s: full path: %.*s\n", __func__,
+	    (int)namelen, name);
 
 	sep = NULL;
 	for (;;) {
@@ -320,8 +320,10 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen,
 			break;
 		}
 
-		/* we're not at the end, so parent must be a directory */
-		if (parent->type != VDIR) {
+		/* we're not at the end, so we must be in a directory */
+		if (tnp != NULL && tnp->type != VDIR) {
+			TARFS_DPF(LOOKUP, "%s: %.*s is not a directory\n", __func__,
+			    (int)tnp->namelen, tnp->name);
 			error = ENOTDIR;
 			break;
 		}
@@ -365,8 +367,9 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen,
 		tnp = NULL;
 		cn.cn_nameptr = name;
 		cn.cn_namelen = len;
-		TARFS_DPF(LOOKUP, "%s: Search: %.*s\n", __func__,
-		    (int)cn.cn_namelen, cn.cn_nameptr);
+		TARFS_DPF(LOOKUP, "%s: looking up %.*s in %.*s/\n", __func__,
+		    (int)cn.cn_namelen, cn.cn_nameptr,
+		    (int)parent->namelen, parent->name);
 		if (do_lookup) {
 			tnp = tarfs_lookup_node(parent, NULL, &cn);
 			if (tnp == NULL) {
@@ -379,7 +382,7 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen,
 		namelen -= cn.cn_namelen;
 	}
 
-	TARFS_DPF(LOOKUP, "%s: Parent %p, node %p\n", __func__, parent, tnp);
+	TARFS_DPF(LOOKUP, "%s: parent %p node %p\n", __func__, parent, tnp);
 
 	if (retparent)
 		*retparent = parent;
diff --git a/tests/sys/fs/tarfs/tarfs_test.sh b/tests/sys/fs/tarfs/tarfs_test.sh
index 634b6be3dd08..15319e249034 100644
--- a/tests/sys/fs/tarfs/tarfs_test.sh
+++ b/tests/sys/fs/tarfs/tarfs_test.sh
@@ -27,12 +27,12 @@
 #
 
 mktar="$(dirname $(realpath "$0"))"/mktar
-mnt="$(realpath ${TMPDIR:-/tmp})/mnt.$$"
+mnt="$(realpath ${TMPDIR:-/tmp})/mnt"
 
 # expected SHA256 checksum of file contained in test tarball
 sum=4da2143234486307bb44eaa610375301781a577d1172f362b88bb4b1643dee62
 
-atf_test_case tarfs_test
+atf_test_case tarfs_basic cleanup
 tarfs_basic_head() {
 	atf_set "descr" "Basic function test"
 	atf_set "require.user" "root"
@@ -50,27 +50,90 @@ tarfs_basic_cleanup() {
 	umount "${mnt}"
 }
 
-atf_test_case tarfs_notdir
-tarfs_notdir_head() {
-	atf_set "descr" "Regression test for PR 269519"
+atf_test_case tarfs_notdir_device cleanup
+tarfs_notdir_device_head() {
+	atf_set "descr" "Regression test for PR 269519 and 269561"
 	atf_set "require.user" "root"
 }
-tarfs_notdir_body() {
+tarfs_notdir_device_body() {
+	mkdir "${mnt}"
+	atf_check mknod d b 0xdead 0xbeef
+	tar cf tarfs_notdir.tar d
+	rm d
+	mkdir d
+	echo "boom" >d/f
+	tar rf tarfs_notdir.tar d/f
+	atf_check -s not-exit:0 -e match:"Invalid" \
+	    mount -rt tarfs tarfs_notdir.tar "${mnt}"
+}
+tarfs_notdir_device_cleanup() {
+	umount "${mnt}" || true
+}
+
+atf_test_case tarfs_notdir_dot cleanup
+tarfs_notdir_dot_head() {
+	atf_set "descr" "Regression test for PR 269519 and 269561"
+	atf_set "require.user" "root"
+}
+tarfs_notdir_dot_body() {
 	mkdir "${mnt}"
 	echo "hello" >d
 	tar cf tarfs_notdir.tar d
 	rm d
-	mkdir -p d/s
-	echo "world" >d/s/f
-	tar rf tarfs_notdir.tar d/s/f
+	mkdir d
+	echo "world" >d/f
+	tar rf tarfs_notdir.tar d/./f
 	atf_check -s not-exit:0 -e match:"Invalid" \
 	    mount -rt tarfs tarfs_notdir.tar "${mnt}"
 }
-tarfs_notdir_cleanup() {
-	umount "${mnt}"
+tarfs_notdir_dot_cleanup() {
+	umount "${mnt}" || true
+}
+
+atf_test_case tarfs_notdir_dotdot cleanup
+tarfs_notdir_dotdot_head() {
+	atf_set "descr" "Regression test for PR 269519 and 269561"
+	atf_set "require.user" "root"
+}
+tarfs_notdir_dotdot_body() {
+	mkdir "${mnt}"
+	echo "hello" >d
+	tar cf tarfs_notdir.tar d
+	rm d
+	mkdir d
+	echo "world" >f
+	tar rf tarfs_notdir.tar d/../f
+	atf_check -s not-exit:0 -e match:"Invalid" \
+	    mount -rt tarfs tarfs_notdir.tar "${mnt}"
+}
+tarfs_notdir_dotdot_cleanup() {
+	umount "${mnt}" || true
+}
+
+atf_test_case tarfs_notdir_file cleanup
+tarfs_notdir_file_head() {
+	atf_set "descr" "Regression test for PR 269519 and 269561"
+	atf_set "require.user" "root"
+}
+tarfs_notdir_file_body() {
+	mkdir "${mnt}"
+	echo "hello" >d
+	tar cf tarfs_notdir.tar d
+	rm d
+	mkdir d
+	echo "world" >d/f
+	tar rf tarfs_notdir.tar d/f
+	atf_check -s not-exit:0 -e match:"Invalid" \
+	    mount -rt tarfs tarfs_notdir.tar "${mnt}"
+}
+tarfs_notdir_file_cleanup() {
+	umount "${mnt}" || true
 }
 
 atf_init_test_cases() {
 	atf_add_test_case tarfs_basic
-	atf_add_test_case tarfs_notdir
+	atf_add_test_case tarfs_notdir_device
+	atf_add_test_case tarfs_notdir_dot
+	atf_add_test_case tarfs_notdir_dotdot
+	atf_add_test_case tarfs_notdir_file
 }