* [PATCH] btrfs: make root id query unprivileged
@ 2015-05-12 17:14 David Sterba
2015-05-13 16:41 ` Goffredo Baroncelli
0 siblings, 1 reply; 3+ messages in thread
From: David Sterba @ 2015-05-12 17:14 UTC (permalink / raw
To: linux-btrfs; +Cc: David Sterba
The INO_LOOKUP ioctl can lookup path for a given inode number and is
thus restricted. As a sideefect it can find the root id of the
containing subvolume and we're using this int the 'btrfs inspect rootid'
command.
The restriction is unnecessary in case we set the ioctl args
args::treeid = 0
args::objectid = 256 (BTRFS_FIRST_FREE_OBJECTID)
Then the path will be empty and the treeid is filled with the root id of
the inode on which the ioctl is called. This behaviour is unchanged,
after the root restriction is removed.
Signed-off-by: David Sterba <dsterba@suse.cz>
---
fs/btrfs/ioctl.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 1c22c6518504..578ff63a9b74 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2271,10 +2271,7 @@ static noinline int btrfs_ioctl_ino_lookup(struct file *file,
{
struct btrfs_ioctl_ino_lookup_args *args;
struct inode *inode;
- int ret;
-
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
+ int ret = 0;
args = memdup_user(argp, sizeof(*args));
if (IS_ERR(args))
@@ -2282,13 +2279,28 @@ static noinline int btrfs_ioctl_ino_lookup(struct file *file,
inode = file_inode(file);
+ /*
+ * Unprivileged query to obtain the containing subvolume root id. The
+ * path is reset so it's consistent with btrfs_search_path_in_tree.
+ */
if (args->treeid == 0)
args->treeid = BTRFS_I(inode)->root->root_key.objectid;
+ if (args->objectid == BTRFS_FIRST_FREE_OBJECTID) {
+ args->name[0] = 0;
+ goto out;
+ }
+
+ if (!capable(CAP_SYS_ADMIN)) {
+ ret = -EPERM;
+ goto out;
+ }
+
ret = btrfs_search_path_in_tree(BTRFS_I(inode)->root->fs_info,
args->treeid, args->objectid,
args->name);
+out:
if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
ret = -EFAULT;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: make root id query unprivileged
2015-05-12 17:14 [PATCH] btrfs: make root id query unprivileged David Sterba
@ 2015-05-13 16:41 ` Goffredo Baroncelli
2015-05-13 17:02 ` David Sterba
0 siblings, 1 reply; 3+ messages in thread
From: Goffredo Baroncelli @ 2015-05-13 16:41 UTC (permalink / raw
To: David Sterba, linux-btrfs
Hi David,
On 2015-05-12 19:14, David Sterba wrote:
> The INO_LOOKUP ioctl can lookup path for a given inode number and is
> thus restricted. As a sideefect it can find the root id of the
> containing subvolume and we're using this int the 'btrfs inspect rootid'
> command.
>
> The restriction is unnecessary in case we set the ioctl args
> args::treeid = 0
> args::objectid = 256 (BTRFS_FIRST_FREE_OBJECTID)
>
> Then the path will be empty and the treeid is filled with the root id of
> the inode on which the ioctl is called. This behaviour is unchanged,
> after the root restriction is removed.
I think that make sense to add another ioctl instead of relaxing the privilege check
on the basis of the parameters.
If I read correctly the code, in this case the function
btrfs_search_path_in_tree() is not even called: it is requested to return only
BTRFS_I(inode)->root->root_key.objectid;
Goffredo
>
> Signed-off-by: David Sterba <dsterba@suse.cz>
> ---
>
> fs/btrfs/ioctl.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 1c22c6518504..578ff63a9b74 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2271,10 +2271,7 @@ static noinline int btrfs_ioctl_ino_lookup(struct file *file,
> {
> struct btrfs_ioctl_ino_lookup_args *args;
> struct inode *inode;
> - int ret;
> -
> - if (!capable(CAP_SYS_ADMIN))
> - return -EPERM;
> + int ret = 0;
>
> args = memdup_user(argp, sizeof(*args));
> if (IS_ERR(args))
> @@ -2282,13 +2279,28 @@ static noinline int btrfs_ioctl_ino_lookup(struct file *file,
>
> inode = file_inode(file);
>
> + /*
> + * Unprivileged query to obtain the containing subvolume root id. The
> + * path is reset so it's consistent with btrfs_search_path_in_tree.
> + */
> if (args->treeid == 0)
> args->treeid = BTRFS_I(inode)->root->root_key.objectid;
>
> + if (args->objectid == BTRFS_FIRST_FREE_OBJECTID) {
> + args->name[0] = 0;
> + goto out;
> + }
> +
> + if (!capable(CAP_SYS_ADMIN)) {
> + ret = -EPERM;
> + goto out;
> + }
> +
> ret = btrfs_search_path_in_tree(BTRFS_I(inode)->root->fs_info,
> args->treeid, args->objectid,
> args->name);
>
> +out:
> if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
> ret = -EFAULT;
>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: make root id query unprivileged
2015-05-13 16:41 ` Goffredo Baroncelli
@ 2015-05-13 17:02 ` David Sterba
0 siblings, 0 replies; 3+ messages in thread
From: David Sterba @ 2015-05-13 17:02 UTC (permalink / raw
To: Goffredo Baroncelli; +Cc: David Sterba, linux-btrfs
On Wed, May 13, 2015 at 06:41:54PM +0200, Goffredo Baroncelli wrote:
> I think that make sense to add another ioctl instead of relaxing the privilege check
> on the basis of the parameters.
Yeah, that was another way how to do it. In general I'm more inclined to
use the existing ioctls and related structures and avoid single purpose
ioctl.
Furthermore, we can teach btrfs_ioctl_ino_lookup to verify access rights
to the file it finds. The root restriction is not unnecessary in all
cases.
We've relaxed the ioctl permissions in the past, see FS_INFO for
example. I hope we can afford that if that's for sake of better
usability and user convenience.
> If I read correctly the code, in this case the function
> btrfs_search_path_in_tree() is not even called: it is requested to return only
>
> BTRFS_I(inode)->root->root_key.objectid;
Right.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-05-13 17:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-12 17:14 [PATCH] btrfs: make root id query unprivileged David Sterba
2015-05-13 16:41 ` Goffredo Baroncelli
2015-05-13 17:02 ` 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.