[RFC PATCH v2 6/8] btrfs-progs: sub list: Allow normal user to call "subvolume list/show"
Allow normal user to call "subvolume list/show" by using 3 new
unprivileged ioctls (BTRFS_IOC_GET_SUBVOL_INFO,
BTRFS_IOC_GET_SUBVOL_ROOTREF and BTRFS_IOC_INO_LOOKUP_USER).
Note that for root, "subvolume list" returns all the subvolume in the
filesystem by default, but for normal user, it returns subvolumes
which exist under the specified path (including the path itself).
The specified path itself is not needed to be a subvolume.
If the subvolume cannot be opened but the parent directory can be,
the information other than name or id would be zeroed out.
Also, for normal user, snapshot filed of "subvolume show" just lists
the snapshots under the specified subvolume.
Signed-off-by: Tomohiro Misono
---
btrfs-list.c | 326 +--
cmds-subvolume.c | 13 +++
2 files changed, 332 insertions(+), 7 deletions(-)
diff --git a/btrfs-list.c b/btrfs-list.c
index 2e82dc5c..ef97d2db 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -33,6 +33,7 @@
#include
#include "btrfs-list.h"
#include "rbtree-utils.h"
+#include
#define BTRFS_LIST_NFILTERS_INCREASE (2 * BTRFS_LIST_FILTER_MAX)
#define BTRFS_LIST_NCOMPS_INCREASE (2 * BTRFS_LIST_COMP_MAX)
@@ -549,6 +550,9 @@ static int resolve_root(struct root_lookup *rl, struct
root_info *ri,
int len = 0;
struct root_info *found;
+ if (ri->full_path != NULL)
+ return 0;
+
/*
* we go backwards from the root_info object and add pathnames
* from parent directories as we go.
@@ -672,6 +676,50 @@ static int lookup_ino_path(int fd, struct root_info *ri)
return 0;
}
+/* user version of lookup_ino_path which also cheks the access right */
+static int lookup_ino_path_user(int fd, struct root_info *ri)
+{
+ struct btrfs_ioctl_ino_lookup_user_args args;
+ int ret = 0;
+
+ if (ri->path)
+ return 0;
+ if (!ri->ref_tree)
+ return -ENOENT;
+
+ memset(&args, 0, sizeof(args));
+ args.dirid = ri->dir_id;
+ args.subvolid = ri->root_id;
+
+ ret = ioctl(fd, BTRFS_IOC_INO_LOOKUP_USER, &args);
+ if (ret < 0) {
+ if (errno == ENOENT) {
+ ri->ref_tree = 0;
+ return -ENOENT;
+ }
+ if (errno != EACCES) {
+ error("failed to lookup path for root %llu: %m",
+ (unsigned long long)ri->ref_tree);
+ return ret;
+ } else {
+ return -EACCES;
+ }
+ }
+
+ ri->path = malloc(strlen(args.path) + strlen(args.name) + 1);
+ if (!ri->path)
+ return -ENOMEM;
+ strcpy(ri->path, args.path);
+
+ ri->name = malloc(strlen(args.name) + 1);
+ if (!ri->name)
+ return -ENOMEM;
+ strcpy(ri->name, args.name);
+
+ strcat(ri->path, ri->name);
+ return ret;
+}
+
/* finding the generation for a given path is a two step process.
* First we use the inode lookup routine to find out the root id
*
@@ -1303,6 +1351,20 @@ static void filter_and_sort_subvol(struct root_lookup
*all_subvols,
while (n) {
entry = rb_entry(n, struct root_info, rb_node);
+ /*
+* If list_subvol_user() is used, there may be
+* entries which have been skipped for search.
+* Just remove these entries here.
+*/
+ if (!entry->path) {
+ struct rb_node *prev = rb_prev(n);
+
+ rb_erase(n, &all_subvols->root);
+ free_root_info(n);
+ n = prev;
+ continue;
+ }
+
ret = resolve_root(all_subvols, entry, top_id);
if (ret == -ENOENT) {
if (entry->root_id != BTRFS_FS_TREE_OBJECTID) {
@@ -1343,6 +1405,243 @@ static int list_subvol_fill_paths(int fd, struct
root_lookup *root_lookup)
return 0;
}
+static int fill_subvol_info(struct root_lookup *root_lookup, int fd)
+{
+ struct btrfs_ioctl_get_subvol_info_args subvol_info;
+ u64 root_offset;
+ int ret;
+
+ ret = ioctl(fd, BTRFS_IOC_GET_SUBVOL_INFO, &subvol_info);
+ if (ret < 0)
+ return -errno;
+
+ if (!uuid_is_null(subvol_info.parent_uuid))
+ root_offset = subvol_info.otransid;
+ else
+ root_offset = 0;
+
+ add_root(root_lookup, subvol_info.id, 0,
+root_offset, subvol_info.flags, subvol_info.dirid,
+NULL, 0,
+subvol_info.otransid, subvol_info.generation,
+subvol_info.otime.sec, subvol_info.uuid,
+subvol_info.parent_uuid, subvol_info.received_uuid);
+
+ return 0;
+}
+
+static int fill_subvol_info_top(struct root_lookup *root_lookup, int fd)
+{
+ struct btrfs_
Re: [RFC PATCH v2 6/8] btrfs-progs: sub list: Allow normal user to call "subvolume list/show"
On 03/15/2018 09:15 AM, Misono, Tomohiro wrote:
> Allow normal user to call "subvolume list/show" by using 3 new
> unprivileged ioctls (BTRFS_IOC_GET_SUBVOL_INFO,
> BTRFS_IOC_GET_SUBVOL_ROOTREF and BTRFS_IOC_INO_LOOKUP_USER).
>
> Note that for root, "subvolume list" returns all the subvolume in the
> filesystem by default, but for normal user, it returns subvolumes
> which exist under the specified path (including the path itself).
I found the original "btrfs sub list" behavior quite confusing. I think that
the problem is that the output is too technical. And the '-a' switch increase
this confusion. May be that I am no smart enough :(
The "normal user behavior" seems to me more coherent. However I am not sure
that this differences should be acceptable. In any case it should be tracked in
the man page.
Time to add another command (something like "btrfs sub ls") with a more "human
friendly" output ?
> The specified path itself is not needed to be a subvolume.
> If the subvolume cannot be opened but the parent directory can be,
> the information other than name or id would be zeroed out.
>
> Also, for normal user, snapshot filed of "subvolume show" just lists
> the snapshots under the specified subvolume.
>
> Signed-off-by: Tomohiro Misono
> ---
> btrfs-list.c | 326
> +--
> cmds-subvolume.c | 13 +++
> 2 files changed, 332 insertions(+), 7 deletions(-)
>
[]
> static void print_subvolume_column(struct root_info *subv,
> enum btrfs_list_column_enum column)
> {
> @@ -1527,17 +1826,28 @@ static int btrfs_list_subvols(int fd, struct
> root_lookup *root_lookup,
> {
> int ret;
>
> - ret = list_subvol_search(fd, root_lookup);
> - if (ret) {
> - error("can't perform the search: %m");
> - return ret;
> + ret = check_perm_for_tree_search(fd);
> + if (ret < 0) {
> + error("can't check the permission for tree search: %s",
> + strerror(-ret));
> + return -1;
> }
>
> /*
>* now we have an rbtree full of root_info objects, but we need to fill
>* in their path names within the subvol that is referencing each one.
>*/
> - ret = list_subvol_fill_paths(fd, root_lookup);
> + if (ret) {
> + ret = list_subvol_search(fd, root_lookup);
> + if (ret) {
> + error("can't perform the search: %s", strerror(-ret));
> + return ret;
> + }
> + ret = list_subvol_fill_paths(fd, root_lookup);
> + } else {
> + ret = list_subvol_user(fd, root_lookup, path);
> + }
> +
> return ret;
I think that the check above should be refined: if I run "btrfs sub list"
patched in a kernel without patch I don't get any error and/or warning:
ghigo@venice:~/btrfs/btrfs-progs$ # kernel w/o patch; btrfs-progs w/patch
ghigo@venice:~/btrfs/btrfs-progs$ ./btrfs sub list /
ghigo@venice:~/btrfs/btrfs-progs$ # kernel w/o patch; btrfs-progs w/o patch
ghigo@venice:~/btrfs/btrfs-progs$ btrfs sub list /
ERROR: can't perform the search: Operation not permitted
I think that in both case an error should be raised
> }
>
> @@ -1631,12 +1941,14 @@ int btrfs_get_subvol(int fd, struct root_info
> *the_ri, const char *path)
> return ret;
> }
>
> + ret = -ENOENT;
> rbn = rb_first(&rl.root);
> while(rbn) {
> ri = rb_entry(rbn, struct root_info, rb_node);
> rr = resolve_root(&rl, ri, root_id);
> - if (rr == -ENOENT) {
> - ret = -ENOENT;
> + if (rr == -ENOENT ||
> + ri->root_id == BTRFS_FS_TREE_OBJECTID ||
> + uuid_is_null(ri->uuid)) {
> rbn = rb_next(rbn);
> continue;
> }
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index faa10c5a..7a7c6f3b 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -596,6 +596,19 @@ static int cmd_subvol_list(int argc, char **argv)
> goto out;
> }
>
> + ret = check_perm_for_tree_search(fd);
> + if (ret < 0) {
> + ret = -1;
> + error("can't check the permission for tree search: %s",
> + strerror(-ret));
> + goto out;
> + }
> + if (!ret && is_list_all) {
> + ret = -1;
> + error("only root can use -a option");
> + goto out;
> + }
> +
> if (flags)
> btrfs_list_setup_filter(&filter_set, BTRFS_LIST_FILTER_FLAGS,
> flags);
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordom
Re: [RFC PATCH v2 6/8] btrfs-progs: sub list: Allow normal user to call "subvolume list/show"
On 2018/03/17 22:23, Goffredo Baroncelli wrote:
> On 03/15/2018 09:15 AM, Misono, Tomohiro wrote:
>> Allow normal user to call "subvolume list/show" by using 3 new
>> unprivileged ioctls (BTRFS_IOC_GET_SUBVOL_INFO,
>> BTRFS_IOC_GET_SUBVOL_ROOTREF and BTRFS_IOC_INO_LOOKUP_USER).
>>
>> Note that for root, "subvolume list" returns all the subvolume in the
>> filesystem by default, but for normal user, it returns subvolumes
>> which exist under the specified path (including the path itself).
>
> I found the original "btrfs sub list" behavior quite confusing. I think that
> the problem is that the output is too technical. And the '-a' switch increase
> this confusion. May be that I am no smart enough :(
>
> The "normal user behavior" seems to me more coherent. However I am not sure
> that this differences should be acceptable. In any case it should be tracked
> in the man page.
>
> Time to add another command (something like "btrfs sub ls") with a more
> "human friendly" output ?
Yes, I'm also for to fix the output of "sub list" more user friendly (and the
behavior of -a or -o).
However, there are other ongoing works to btrfs-progs which changes code base
largely
(i.e libbtrfs and json output, and that is the part of reason this series is
RFC as the
changes conflict especially for libbtrfs). So, I'd like to see these changes
will be merged first.
>
>> The specified path itself is not needed to be a subvolume.
>> If the subvolume cannot be opened but the parent directory can be,
>> the information other than name or id would be zeroed out.
>>
>> Also, for normal user, snapshot filed of "subvolume show" just lists
>> the snapshots under the specified subvolume.
>>
>> Signed-off-by: Tomohiro Misono
>> ---
>> btrfs-list.c | 326
>> +--
>> cmds-subvolume.c | 13 +++
>> 2 files changed, 332 insertions(+), 7 deletions(-)
>>
>
> []
>
>> static void print_subvolume_column(struct root_info *subv,
>> enum btrfs_list_column_enum column)
>> {
>> @@ -1527,17 +1826,28 @@ static int btrfs_list_subvols(int fd, struct
>> root_lookup *root_lookup,
>> {
>> int ret;
>>
>> -ret = list_subvol_search(fd, root_lookup);
>> -if (ret) {
>> -error("can't perform the search: %m");
>> -return ret;
>> +ret = check_perm_for_tree_search(fd);
>> +if (ret < 0) {
>> +error("can't check the permission for tree search: %s",
>> +strerror(-ret));
>> +return -1;
>> }
>>
>> /*
>> * now we have an rbtree full of root_info objects, but we need to fill
>> * in their path names within the subvol that is referencing each one.
>> */
>> -ret = list_subvol_fill_paths(fd, root_lookup);
>> +if (ret) {
>> +ret = list_subvol_search(fd, root_lookup);
>> +if (ret) {
>> +error("can't perform the search: %s", strerror(-ret));
>> +return ret;
>> +}
>> +ret = list_subvol_fill_paths(fd, root_lookup);
>> +} else {
>> +ret = list_subvol_user(fd, root_lookup, path);
>> +}
>> +
>> return ret;
>
> I think that the check above should be refined: if I run "btrfs sub list"
> patched in a kernel without patch I don't get any error and/or warning:
>
> ghigo@venice:~/btrfs/btrfs-progs$ # kernel w/o patch; btrfs-progs w/patch
> ghigo@venice:~/btrfs/btrfs-progs$ ./btrfs sub list /
> ghigo@venice:~/btrfs/btrfs-progs$ # kernel w/o patch; btrfs-progs w/o patch
> ghigo@venice:~/btrfs/btrfs-progs$ btrfs sub list /
> ERROR: can't perform the search: Operation not permitted
>
> I think that in both case an error should be raised
I will fix this in next version.
Thanks,
Tomohiro Misono
>
>
>> }
>>
>> @@ -1631,12 +1941,14 @@ int btrfs_get_subvol(int fd, struct root_info
>> *the_ri, const char *path)
>> return ret;
>> }
>>
>> +ret = -ENOENT;
>> rbn = rb_first(&rl.root);
>> while(rbn) {
>> ri = rb_entry(rbn, struct root_info, rb_node);
>> rr = resolve_root(&rl, ri, root_id);
>> -if (rr == -ENOENT) {
>> -ret = -ENOENT;
>> +if (rr == -ENOENT ||
>> +ri->root_id == BTRFS_FS_TREE_OBJECTID ||
>> +uuid_is_null(ri->uuid)) {
>> rbn = rb_next(rbn);
>> continue;
>> }
>> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
>> index faa10c5a..7a7c6f3b 100644
>> --- a/cmds-subvolume.c
>> +++ b/cmds-subvolume.c
>> @@ -596,6 +596,19 @@ static int cmd_subvol_list(int argc, char **argv)
>> goto out;
>> }
>>
>> +ret = check_perm_for_tree_search(fd);
>> +if (ret < 0) {
>> +ret = -1;
>> +error("can't check the permission for tree search: %s",
>> +strerror(-ret));
>> +
