* [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u"
@ 2022-12-27 5:55 Qu Wenruo
2022-12-27 5:55 ` [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-12-27 5:55 UTC (permalink / raw)
To: linux-btrfs
There is a regression caused by commit d729048be6ef ("btrfs-progs: stop
using btrfs_root_item_v0").
Just fix it and add a simple test case for it.
Qu Wenruo (2):
btrfs-progs: fix the wrong timestamp and UUID check for root items
btrfs-progs: misc-tests: add a test case to make sure uuid is
correctly reported
cmds/subvolume-list.c | 11 ++++++--
.../056-subvolume-list-uuid/test.sh | 28 +++++++++++++++++++
2 files changed, 37 insertions(+), 2 deletions(-)
create mode 100755 tests/misc-tests/056-subvolume-list-uuid/test.sh
--
2.39.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items
2022-12-27 5:55 [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" Qu Wenruo
@ 2022-12-27 5:55 ` Qu Wenruo
2022-12-27 6:36 ` Anand Jain
2022-12-28 12:04 ` Neal Gompa
2022-12-27 5:55 ` [PATCH 2/2] btrfs-progs: misc-tests: add a test case to make sure uuid is correctly reported Qu Wenruo
2023-01-02 13:02 ` [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" David Sterba
2 siblings, 2 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-12-27 5:55 UTC (permalink / raw)
To: linux-btrfs
[BUG]
Since commit d729048be6ef ("btrfs-progs: stop using
btrfs_root_item_v0"), "btrfs subvolume list" not longer correctly report
UUID nor timestamp, while older (btrfs-progs v6.0.2) still works
correct:
v6.0.2:
# btrfs subv list -u /mnt/btrfs/
ID 256 gen 12 top level 5 uuid ed4af580-d512-2644-b392-2a71aaeeb99e path subv1
ID 257 gen 13 top level 5 uuid a22ccba7-0a0a-a94f-af4b-5116ab58bb61 path subv2
v6.1:
# ./btrfs subv list -u /mnt/btrfs/
ID 256 gen 12 top level 5 uuid - path subv1
ID 257 gen 13 top level 5 uuid - path subv2
[CAUSE]
Commit d729048be6ef ("btrfs-progs: stop using btrfs_root_item_v0")
removed old btrfs_root_item_v0, but incorrectly changed the check for
v0 root item.
Now we will treat v0 root items as latest root items, causing possible
out-of-bound access. while treat current root items as older v0 root
items, ignoring the UUID nor timestamp.
[FIX]
Fix the bug by using correct checks, and add extra comments on the
branches.
Issue: #562
Fixes: d729048be6ef ("btrfs-progs: stop using btrfs_root_item_v0")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
cmds/subvolume-list.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
index 6d5ef509ae67..7cdb0402b8e5 100644
--- a/cmds/subvolume-list.c
+++ b/cmds/subvolume-list.c
@@ -870,14 +870,21 @@ static int list_subvol_search(int fd, struct rb_root *root_lookup)
ri = (struct btrfs_root_item *)(args.buf + off);
gen = btrfs_root_generation(ri);
flags = btrfs_root_flags(ri);
- if(sh.len <
- sizeof(struct btrfs_root_item)) {
+ if(sh.len >= sizeof(struct btrfs_root_item)) {
+ /*
+ * The new full btrfs_root_item with
+ * timestamp and UUID.
+ */
otime = btrfs_stack_timespec_sec(&ri->otime);
ogen = btrfs_root_otransid(ri);
memcpy(uuid, ri->uuid, BTRFS_UUID_SIZE);
memcpy(puuid, ri->parent_uuid, BTRFS_UUID_SIZE);
memcpy(ruuid, ri->received_uuid, BTRFS_UUID_SIZE);
} else {
+ /*
+ * The old v0 root item, which doesn't
+ * has timestamp nor UUID.
+ */
otime = 0;
ogen = 0;
memset(uuid, 0, BTRFS_UUID_SIZE);
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] btrfs-progs: misc-tests: add a test case to make sure uuid is correctly reported
2022-12-27 5:55 [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" Qu Wenruo
2022-12-27 5:55 ` [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items Qu Wenruo
@ 2022-12-27 5:55 ` Qu Wenruo
2022-12-28 12:05 ` Neal Gompa
2023-01-02 13:02 ` [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" David Sterba
2 siblings, 1 reply; 7+ messages in thread
From: Qu Wenruo @ 2022-12-27 5:55 UTC (permalink / raw)
To: linux-btrfs
The new test case will execute "btrfs subvolume list -u" on the newly
create btrfs.
Since the v0 root item is already deprecated for a long time, newly
created btrfs should be already using the new root item, thus "btrfs
subvolume list -u" should always report the correct uuid.
The test case relies on external program "uuidparse" which should be
provided by util-linux.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
.../056-subvolume-list-uuid/test.sh | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100755 tests/misc-tests/056-subvolume-list-uuid/test.sh
diff --git a/tests/misc-tests/056-subvolume-list-uuid/test.sh b/tests/misc-tests/056-subvolume-list-uuid/test.sh
new file mode 100755
index 000000000000..45f4f956c25f
--- /dev/null
+++ b/tests/misc-tests/056-subvolume-list-uuid/test.sh
@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# Make sure "btrfs subvolume list -u" shows uuid correctly
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+check_global_prereq uuidparse
+
+setup_root_helper
+prepare_test_dev
+
+tmp=$(_mktemp_dir list_uuid)
+
+run_check_mkfs_test_dev
+run_check_mount_test_dev
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subv1"
+run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list -u "$TEST_MNT" |\
+ cut -d\ -f9 > "$tmp/output"
+
+result=$(cat "$tmp/output" | uuidparse -o TYPE -n)
+rm -rf -- "$tmp"
+
+if [ "$result" == "invalid" ]; then
+ _fail "subvolume list failed to report uuid"
+fi
+run_check_umount_test_dev
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items
2022-12-27 5:55 ` [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items Qu Wenruo
@ 2022-12-27 6:36 ` Anand Jain
2022-12-28 12:04 ` Neal Gompa
1 sibling, 0 replies; 7+ messages in thread
From: Anand Jain @ 2022-12-27 6:36 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 12/27/22 13:55, Qu Wenruo wrote:
> [BUG]
> Since commit d729048be6ef ("btrfs-progs: stop using
> btrfs_root_item_v0"), "btrfs subvolume list" not longer correctly report
> UUID nor timestamp, while older (btrfs-progs v6.0.2) still works
> correct:
>
> v6.0.2:
> # btrfs subv list -u /mnt/btrfs/
> ID 256 gen 12 top level 5 uuid ed4af580-d512-2644-b392-2a71aaeeb99e path subv1
> ID 257 gen 13 top level 5 uuid a22ccba7-0a0a-a94f-af4b-5116ab58bb61 path subv2
>
> v6.1:
> # ./btrfs subv list -u /mnt/btrfs/
> ID 256 gen 12 top level 5 uuid - path subv1
> ID 257 gen 13 top level 5 uuid - path subv2
>
> [CAUSE]
> Commit d729048be6ef ("btrfs-progs: stop using btrfs_root_item_v0")
> removed old btrfs_root_item_v0, but incorrectly changed the check for
> v0 root item.
>
> Now we will treat v0 root items as latest root items, causing possible
> out-of-bound access. while treat current root items as older v0 root
> items, ignoring the UUID nor timestamp.
>
> [FIX]
> Fix the bug by using correct checks, and add extra comments on the
> branches.
>
> Issue: #562
> Fixes: d729048be6ef ("btrfs-progs: stop using btrfs_root_item_v0")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> cmds/subvolume-list.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
> index 6d5ef509ae67..7cdb0402b8e5 100644
> --- a/cmds/subvolume-list.c
> +++ b/cmds/subvolume-list.c
> @@ -870,14 +870,21 @@ static int list_subvol_search(int fd, struct rb_root *root_lookup)
> ri = (struct btrfs_root_item *)(args.buf + off);
> gen = btrfs_root_generation(ri);
> flags = btrfs_root_flags(ri);
> - if(sh.len <
> - sizeof(struct btrfs_root_item)) {
> + if(sh.len >= sizeof(struct btrfs_root_item)) {
> + /*
> + * The new full btrfs_root_item with
> + * timestamp and UUID.
> + */
> otime = btrfs_stack_timespec_sec(&ri->otime);
> ogen = btrfs_root_otransid(ri);
> memcpy(uuid, ri->uuid, BTRFS_UUID_SIZE);
> memcpy(puuid, ri->parent_uuid, BTRFS_UUID_SIZE);
> memcpy(ruuid, ri->received_uuid, BTRFS_UUID_SIZE);
> } else {
> + /*
> + * The old v0 root item, which doesn't
> + * has timestamp nor UUID.
> + */
Reviewed-by: Anand Jain <anand.jain@oracle.com>
> otime = 0;
> ogen = 0;
> memset(uuid, 0, BTRFS_UUID_SIZE);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items
2022-12-27 5:55 ` [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items Qu Wenruo
2022-12-27 6:36 ` Anand Jain
@ 2022-12-28 12:04 ` Neal Gompa
1 sibling, 0 replies; 7+ messages in thread
From: Neal Gompa @ 2022-12-28 12:04 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Dec 27, 2022 at 1:10 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> Since commit d729048be6ef ("btrfs-progs: stop using
> btrfs_root_item_v0"), "btrfs subvolume list" not longer correctly report
> UUID nor timestamp, while older (btrfs-progs v6.0.2) still works
> correct:
>
> v6.0.2:
> # btrfs subv list -u /mnt/btrfs/
> ID 256 gen 12 top level 5 uuid ed4af580-d512-2644-b392-2a71aaeeb99e path subv1
> ID 257 gen 13 top level 5 uuid a22ccba7-0a0a-a94f-af4b-5116ab58bb61 path subv2
>
> v6.1:
> # ./btrfs subv list -u /mnt/btrfs/
> ID 256 gen 12 top level 5 uuid - path subv1
> ID 257 gen 13 top level 5 uuid - path subv2
>
> [CAUSE]
> Commit d729048be6ef ("btrfs-progs: stop using btrfs_root_item_v0")
> removed old btrfs_root_item_v0, but incorrectly changed the check for
> v0 root item.
>
> Now we will treat v0 root items as latest root items, causing possible
> out-of-bound access. while treat current root items as older v0 root
> items, ignoring the UUID nor timestamp.
>
> [FIX]
> Fix the bug by using correct checks, and add extra comments on the
> branches.
>
> Issue: #562
> Fixes: d729048be6ef ("btrfs-progs: stop using btrfs_root_item_v0")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> cmds/subvolume-list.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
> index 6d5ef509ae67..7cdb0402b8e5 100644
> --- a/cmds/subvolume-list.c
> +++ b/cmds/subvolume-list.c
> @@ -870,14 +870,21 @@ static int list_subvol_search(int fd, struct rb_root *root_lookup)
> ri = (struct btrfs_root_item *)(args.buf + off);
> gen = btrfs_root_generation(ri);
> flags = btrfs_root_flags(ri);
> - if(sh.len <
> - sizeof(struct btrfs_root_item)) {
> + if(sh.len >= sizeof(struct btrfs_root_item)) {
> + /*
> + * The new full btrfs_root_item with
> + * timestamp and UUID.
> + */
> otime = btrfs_stack_timespec_sec(&ri->otime);
> ogen = btrfs_root_otransid(ri);
> memcpy(uuid, ri->uuid, BTRFS_UUID_SIZE);
> memcpy(puuid, ri->parent_uuid, BTRFS_UUID_SIZE);
> memcpy(ruuid, ri->received_uuid, BTRFS_UUID_SIZE);
> } else {
> + /*
> + * The old v0 root item, which doesn't
> + * has timestamp nor UUID.
> + */
> otime = 0;
> ogen = 0;
> memset(uuid, 0, BTRFS_UUID_SIZE);
> --
> 2.39.0
>
Resolves: rhbz#2156710
Reviewed-by: Neal Gompa <neal@gompa.dev>
--
真実はいつも一つ！/ Always, there's only one truth!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: misc-tests: add a test case to make sure uuid is correctly reported
2022-12-27 5:55 ` [PATCH 2/2] btrfs-progs: misc-tests: add a test case to make sure uuid is correctly reported Qu Wenruo
@ 2022-12-28 12:05 ` Neal Gompa
0 siblings, 0 replies; 7+ messages in thread
From: Neal Gompa @ 2022-12-28 12:05 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Dec 27, 2022 at 1:10 AM Qu Wenruo <wqu@suse.com> wrote:
>
> The new test case will execute "btrfs subvolume list -u" on the newly
> create btrfs.
>
> Since the v0 root item is already deprecated for a long time, newly
> created btrfs should be already using the new root item, thus "btrfs
> subvolume list -u" should always report the correct uuid.
>
> The test case relies on external program "uuidparse" which should be
> provided by util-linux.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> .../056-subvolume-list-uuid/test.sh | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100755 tests/misc-tests/056-subvolume-list-uuid/test.sh
>
> diff --git a/tests/misc-tests/056-subvolume-list-uuid/test.sh b/tests/misc-tests/056-subvolume-list-uuid/test.sh
> new file mode 100755
> index 000000000000..45f4f956c25f
> --- /dev/null
> +++ b/tests/misc-tests/056-subvolume-list-uuid/test.sh
> @@ -0,0 +1,28 @@
> +#!/bin/bash
> +#
> +# Make sure "btrfs subvolume list -u" shows uuid correctly
> +
> +source "$TEST_TOP/common"
> +
> +check_prereq mkfs.btrfs
> +check_prereq btrfs
> +check_global_prereq uuidparse
> +
> +setup_root_helper
> +prepare_test_dev
> +
> +tmp=$(_mktemp_dir list_uuid)
> +
> +run_check_mkfs_test_dev
> +run_check_mount_test_dev
> +run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subv1"
> +run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list -u "$TEST_MNT" |\
> + cut -d\ -f9 > "$tmp/output"
> +
> +result=$(cat "$tmp/output" | uuidparse -o TYPE -n)
> +rm -rf -- "$tmp"
> +
> +if [ "$result" == "invalid" ]; then
> + _fail "subvolume list failed to report uuid"
> +fi
> +run_check_umount_test_dev
> --
> 2.39.0
>
Reviewed-by: Neal Gompa <neal@gompa.dev>
--
真実はいつも一つ！/ Always, there's only one truth!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u"
2022-12-27 5:55 [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" Qu Wenruo
2022-12-27 5:55 ` [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items Qu Wenruo
2022-12-27 5:55 ` [PATCH 2/2] btrfs-progs: misc-tests: add a test case to make sure uuid is correctly reported Qu Wenruo
@ 2023-01-02 13:02 ` David Sterba
2 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-01-02 13:02 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Dec 27, 2022 at 01:55:06PM +0800, Qu Wenruo wrote:
> There is a regression caused by commit d729048be6ef ("btrfs-progs: stop
> using btrfs_root_item_v0").
>
> Just fix it and add a simple test case for it.
>
> Qu Wenruo (2):
> btrfs-progs: fix the wrong timestamp and UUID check for root items
> btrfs-progs: misc-tests: add a test case to make sure uuid is
> correctly reported
Added to devel, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-01-02 13:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-27 5:55 [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" Qu Wenruo
2022-12-27 5:55 ` [PATCH 1/2] btrfs-progs: fix the wrong timestamp and UUID check for root items Qu Wenruo
2022-12-27 6:36 ` Anand Jain
2022-12-28 12:04 ` Neal Gompa
2022-12-27 5:55 ` [PATCH 2/2] btrfs-progs: misc-tests: add a test case to make sure uuid is correctly reported Qu Wenruo
2022-12-28 12:05 ` Neal Gompa
2023-01-02 13:02 ` [PATCH 0/2] btrfs-progs: fix the uuid report in "btrfs subvolume list -u" David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox