The branch main has been updated by des:
URL:
https://cgit.FreeBSD.org/src/commit/?id=d481dcee72a05580c4c3af4a429b1c08fa846056
commit d481dcee72a05580c4c3af4a429b1c08fa846056
Author: Dag-Erling Smørgrav <d...@freebsd.org>
AuthorDate: 2023-02-20 21:28:53 +0000
Commit: Dag-Erling Smørgrav <d...@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
}
