* [PATCH] btrfs-progs: receive: fix btrfs_mount_root substring bug
@ 2020-11-17 0:58 Boris Burkov
2021-02-19 15:30 ` David Sterba
0 siblings, 1 reply; 2+ messages in thread
From: Boris Burkov @ 2020-11-17 0:58 UTC (permalink / raw)
To: linux-btrfs, kernel-team
The current mount detection code in btrfs receive is not quite perfect.
For example, suppose /tmp is mounted as a tmpfs. In that case,
btrfs receive /tmp2 will find /tmp as the longest mount that matches a
prefix of /tmp2 and blow up because it is not a btrfs filesystem, even
if /tmp2 is just a directory in / mounted as btrfs.
Fix this by replacing the substring check with a dirname recursion to
only check the directories in the path of the dir, rather than every
substring.
Add a new test for this case.
Signed-off-by: Boris Burkov <boris@bur.io>
---
common/path-utils.c | 32 +++++++++++++++++
common/path-utils.h | 1 +
common/utils.c | 5 ++-
tests/misc-tests/042-recv-mount-type/test.sh | 36 ++++++++++++++++++++
4 files changed, 71 insertions(+), 3 deletions(-)
create mode 100755 tests/misc-tests/042-recv-mount-type/test.sh
diff --git a/common/path-utils.c b/common/path-utils.c
index 175da572..ee479cc7 100644
--- a/common/path-utils.c
+++ b/common/path-utils.c
@@ -29,6 +29,7 @@
#include <string.h>
#include <errno.h>
#include <ctype.h>
+#include <libgen.h>
#include "common/path-utils.h"
/*
@@ -374,6 +375,37 @@ int path_is_dir(const char *path)
return !!S_ISDIR(st.st_mode);
}
+/*
+ * Test if a path is recursively contained in parent
+ * Assumes parent and path are null terminated absolute paths
+ * Returns:
+ * 0 - path not contained in parent
+ * 1 - path contained in parent
+ * < 0 - error
+ * e.g. (/, /foo) -> 1
+ * (/foo, /) -> 0
+ * (/foo, /foo/bar/baz) -> 1
+ */
+int path_is_in_dir(const char *parent, const char *path)
+{
+ char *tmp = strdup(path);
+ char *curr_dir = tmp;
+ int ret;
+
+ while (strcmp(parent, curr_dir) != 0) {
+ if (strcmp(curr_dir, "/") == 0) {
+ ret = 0;
+ goto out;
+ }
+ curr_dir = dirname(curr_dir);
+ }
+ ret = 1;
+
+out:
+ free(tmp);
+ return ret;
+}
+
/*
* Copy a path argument from SRC to DEST and check the SRC length if it's at
* most PATH_MAX and fits into DEST. DESTLEN is supposed to be exact size of
diff --git a/common/path-utils.h b/common/path-utils.h
index 0cabcb7d..8e9ddadf 100644
--- a/common/path-utils.h
+++ b/common/path-utils.h
@@ -35,6 +35,7 @@ int path_is_reg_file(const char *path);
int path_is_dir(const char *path);
int is_same_loop_file(const char *a, const char *b);
int path_is_reg_or_block_device(const char *filename);
+int path_is_in_dir(const char *parent, const char *path);
int test_issubvolname(const char *name);
diff --git a/common/utils.c b/common/utils.c
index c47ce29b..9bf28cc6 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1267,9 +1267,8 @@ int find_mount_root(const char *path, char **mount_root)
return -errno;
while ((ent = getmntent(mnttab))) {
- len = strlen(ent->mnt_dir);
- if (strncmp(ent->mnt_dir, path, len) == 0) {
- /* match found and use the latest match */
+ if (path_is_in_dir(ent->mnt_dir, path)) {
+ len = strlen(ent->mnt_dir);
if (longest_matchlen <= len) {
free(longest_match);
longest_matchlen = len;
diff --git a/tests/misc-tests/042-recv-mount-type/test.sh b/tests/misc-tests/042-recv-mount-type/test.sh
new file mode 100755
index 00000000..a30742a8
--- /dev/null
+++ b/tests/misc-tests/042-recv-mount-type/test.sh
@@ -0,0 +1,36 @@
+#!/bin/bash
+#
+# Test some scenarios around the mount point we do receive onto.
+# Should fail in a non-btrfs filesystem, but succeed if a non btrfs
+# filesystem is the longest mounted substring of the target, but not
+# the actual containing mount.
+#
+# This is a regression test for
+# "btrfs-progs: receive: fix btrfs_mount_root substring bug"
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+run_check_mkfs_test_dev
+run_check_mount_test_dev
+
+cd "$TEST_MNT"
+run_check $SUDO_HELPER mkdir "foo" "foobar"
+run_check $SUDO_HELPER mount -t tmpfs tmpfs "foo"
+run_check $SUDO_HELPER mkdir "foo/bar"
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "subvol"
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r "subvol" "snap"
+run_check $SUDO_HELPER "$TOP/btrfs" send -f send.data "snap"
+run_mustfail "no receive on tmpfs" $SUDO_HELPER "$TOP/btrfs" receive -f send.data "./foo"
+run_mustfail "no receive on tmpfs" $SUDO_HELPER "$TOP/btrfs" receive -f send.data "./foo/bar"
+run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "./foobar"
+run_check_umount_test_dev "foo"
+
+cd ..
+run_check_umount_test_dev
--
2.24.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] btrfs-progs: receive: fix btrfs_mount_root substring bug
2020-11-17 0:58 [PATCH] btrfs-progs: receive: fix btrfs_mount_root substring bug Boris Burkov
@ 2021-02-19 15:30 ` David Sterba
0 siblings, 0 replies; 2+ messages in thread
From: David Sterba @ 2021-02-19 15:30 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs, kernel-team
On Mon, Nov 16, 2020 at 04:58:20PM -0800, Boris Burkov wrote:
> The current mount detection code in btrfs receive is not quite perfect.
> For example, suppose /tmp is mounted as a tmpfs. In that case,
> btrfs receive /tmp2 will find /tmp as the longest mount that matches a
> prefix of /tmp2 and blow up because it is not a btrfs filesystem, even
> if /tmp2 is just a directory in / mounted as btrfs.
>
> Fix this by replacing the substring check with a dirname recursion to
> only check the directories in the path of the dir, rather than every
> substring.
>
> Add a new test for this case.
>
> Signed-off-by: Boris Burkov <boris@bur.io>
Added to devel, thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-19 15:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 0:58 [PATCH] btrfs-progs: receive: fix btrfs_mount_root substring bug Boris Burkov
2021-02-19 15:30 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.