All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC V2] new ioctl BTRFS_IOC_GET_CHUNK_INFO
@ 2020-03-19 18:05 Goffredo Baroncelli
  2020-03-19 18:05 ` [PATCH RFC v2] New " Goffredo Baroncelli
  0 siblings, 1 reply; 6+ messages in thread
From: Goffredo Baroncelli @ 2020-03-19 18:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Josef Bacik


This is a repost of an old patch (~2017). At the time it din't received
any feedback. I repost it hoping that it still be interesting.

This patch is the kernel related one; another one related to the
btrfs-progs is send separately.

v2: 
  - Remove the goto from copy_chunk_info()
V1: 
  - First issue

----

This patch set creates a new ioctl BTRFS_IOC_GET_CHUNK_INFO.
The aim is to replace the BTRFS_IOC_TREE_SEARCH ioctl
used by "btrfs fi usage" to obtain information about the 
chunks/block groups. 

The problems in using the BTRFS_IOC_TREE_SEARCH is that it access
the very low data structure of BTRFS. This means: 
1) this would be complicated a possible change of the disk format
2) it requires the root privileges

The BTRFS_IOC_GET_CHUNK_INFO ioctl can be called even from a not root
user: I think that the data exposed are not sensibile data.

These patches allow to use "btrfs fi usage" without root privileges.

before:
-------------------------------------------

$ btrfs fi us /
WARNING: cannot read detailed chunk info, per-device usage will not be shown, run as root
Overall:
    Device size:                 100.00GiB
    Device allocated:             26.03GiB
    Device unallocated:           73.97GiB
    Device missing:                  0.00B
    Used:                         17.12GiB
    Free (estimated):             80.42GiB      (min: 80.42GiB)
    Data ratio:                       1.00
    Metadata ratio:                   1.00
    Global reserve:               53.12MiB      (used: 0.00B)

Data,single: Size:23.00GiB, Used:16.54GiB (71.93%)

Metadata,single: Size:3.00GiB, Used:588.94MiB (19.17%)

System,single: Size:32.00MiB, Used:16.00KiB (0.05%)

after:
-----------------------------------------------
$ ./btrfs fi us /
Overall:
    Device size:                 100.00GiB
    Device allocated:             26.03GiB
    Device unallocated:           73.97GiB
    Device missing:                  0.00B
    Used:                         17.12GiB
    Free (estimated):             80.42GiB      (min: 80.42GiB)
    Data ratio:                       1.00
    Metadata ratio:                   1.00
    Global reserve:               53.12MiB      (used: 0.00B)

Data,single: Size:23.00GiB, Used:16.54GiB (71.93%)
   /dev/sdd3      23.00GiB

Metadata,single: Size:3.00GiB, Used:588.94MiB (19.17%)
   /dev/sdd3       3.00GiB

System,single: Size:32.00MiB, Used:16.00KiB (0.05%)
   /dev/sdd3      32.00MiB

Unallocated:
   /dev/sdd3      73.97GiB

Comments are welcome
BR
G.Baroncelli


-- 
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] 6+ messages in thread

* [PATCH RFC v2] New ioctl BTRFS_IOC_GET_CHUNK_INFO.
  2020-03-19 18:05 [PATCH RFC V2] new ioctl BTRFS_IOC_GET_CHUNK_INFO Goffredo Baroncelli
@ 2020-03-19 18:05 ` Goffredo Baroncelli
  2020-03-19 18:15   ` Josef Bacik
  0 siblings, 1 reply; 6+ messages in thread
From: Goffredo Baroncelli @ 2020-03-19 18:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Josef Bacik, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>

Add a new ioctl to get info about chunk without requiring the root privileges.
This allow to a non root user to know how the space of the filesystem is
allocated.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 fs/btrfs/ioctl.c           | 211 +++++++++++++++++++++++++++++++++++++
 include/uapi/linux/btrfs.h |  38 +++++++
 2 files changed, 249 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 40b729dce91c..e9231d597422 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2234,6 +2234,215 @@ static noinline int btrfs_ioctl_tree_search_v2(struct file *file,
 	return ret;
 }
 
+/*
+ * Return:
+ *	0		-> copied all data, possible further data
+ *	1		-> copied all data, no further data
+ *	-EAGAIN		-> not enough space, restart it
+ *	-EFAULT		-> the user passed an invalid address/size pair
+ */
+static noinline int copy_chunk_info(struct btrfs_path *path,
+			       char __user *ubuf,
+			       size_t buf_size,
+			       u64 *used_buf,
+			       int *num_found,
+			       u64 *offset)
+{
+	struct extent_buffer *leaf;
+	unsigned long item_off;
+	unsigned long item_len;
+	int nritems;
+	int i;
+	int slot;
+	struct btrfs_key key;
+
+	leaf = path->nodes[0];
+	slot = path->slots[0];
+	nritems = btrfs_header_nritems(leaf);
+
+	for (i = slot; i < nritems; i++) {
+		u64 destsize;
+		struct btrfs_chunk_info ci;
+		struct btrfs_chunk chunk;
+		int j, chunk_size;
+
+		item_off = btrfs_item_ptr_offset(leaf, i);
+		item_len = btrfs_item_size_nr(leaf, i);
+
+		btrfs_item_key_to_cpu(leaf, &key, i);
+		/*
+		 * we are not interested in other items type
+		 */
+		if (key.type != BTRFS_CHUNK_ITEM_KEY)
+			return 1;
+
+		/*
+		 * In any case, the next search must start from here
+		 */
+		*offset = key.offset;
+		read_extent_buffer(leaf, &chunk, item_off, sizeof(chunk));
+
+		/*
+		 * chunk.num_stripes-1 is correct, because btrfs_chunk includes
+		 * already a stripe
+		 */
+		destsize = sizeof(struct btrfs_chunk_info) +
+			(chunk.num_stripes - 1) * sizeof(struct btrfs_stripe);
+
+		if (destsize > item_len) {
+			ASSERT(0);
+			return -EINVAL;
+		}
+
+		if (buf_size < destsize + *used_buf) {
+			if (*num_found)
+				/* try onother time */
+				return -EAGAIN;
+			else
+				/* in any case the buffer is too small */
+				return -EOVERFLOW;
+		}
+
+		/* copy chunk */
+		chunk_size = offsetof(struct btrfs_chunk_info, stripes);
+		memset(&ci, 0, chunk_size);
+		ci.length = btrfs_stack_chunk_length(&chunk);
+		ci.stripe_len = btrfs_stack_chunk_stripe_len(&chunk);
+		ci.type = btrfs_stack_chunk_type(&chunk);
+		ci.num_stripes = btrfs_stack_chunk_num_stripes(&chunk);
+		ci.sub_stripes = btrfs_stack_chunk_sub_stripes(&chunk);
+		ci.offset = key.offset;
+
+		if (copy_to_user(ubuf + *used_buf, &ci, chunk_size))
+			return -EFAULT;
+
+		*used_buf += chunk_size;
+
+		/* copy stripes */
+		for (j = 0 ; j < chunk.num_stripes ; j++) {
+			struct btrfs_stripe chunk_stripe;
+			struct btrfs_chunk_info_stripe csi;
+
+			/*
+			 * j-1 is correct, because btrfs_chunk includes already
+			 * a stripe
+			 */
+			read_extent_buffer(leaf, &chunk_stripe,
+					item_off + sizeof(struct btrfs_chunk) +
+						sizeof(struct btrfs_stripe) *
+						(j - 1), sizeof(chunk_stripe));
+
+			memset(&csi, 0, sizeof(csi));
+
+			csi.devid = btrfs_stack_stripe_devid(&chunk_stripe);
+			csi.offset = btrfs_stack_stripe_offset(&chunk_stripe);
+			memcpy(csi.dev_uuid, chunk_stripe.dev_uuid,
+				sizeof(chunk_stripe.dev_uuid));
+			if (copy_to_user(ubuf + *used_buf, &csi, sizeof(csi)))
+				return -EFAULT;
+
+			*used_buf += sizeof(csi);
+		}
+
+		++(*num_found);
+	}
+
+	if (*offset < (u64)-1)
+		++(*offset);
+
+	return 0;
+}
+
+static noinline int search_chunk_info(struct inode *inode, u64 *offset,
+				      int *items_count,
+				      char __user *ubuf, u64 buf_size)
+{
+	struct btrfs_fs_info *info = btrfs_sb(inode->i_sb);
+	struct btrfs_root *root;
+	struct btrfs_key key;
+	struct btrfs_path *path;
+	int ret;
+	u64 used_buf = 0;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	/* search for BTRFS_CHUNK_TREE_OBJECTID tree */
+	key.objectid = BTRFS_CHUNK_TREE_OBJECTID;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = (u64)-1;
+	root = btrfs_read_fs_root_no_name(info, &key);
+	if (IS_ERR(root)) {
+		btrfs_err(info, "could not find root\n");
+		btrfs_free_path(path);
+		return -ENOENT;
+	}
+
+
+	while (used_buf < buf_size) {
+		key.objectid = 0x0100;
+		key.type = BTRFS_CHUNK_ITEM_KEY;
+		key.offset = *offset;
+
+		ret = btrfs_search_forward(root, &key, path, 0);
+		if (ret != 0) {
+			if (ret > 0)
+				ret = 0;
+			goto ret;
+		}
+
+		ret = copy_chunk_info(path, ubuf, buf_size,
+				      &used_buf, items_count, offset);
+
+		btrfs_release_path(path);
+		if (ret)
+			break;
+
+	}
+
+ret:
+	btrfs_free_path(path);
+	return ret;
+}
+
+static noinline int btrfs_ioctl_get_chunk_info(struct file *file,
+					       void __user *argp)
+{
+	struct btrfs_ioctl_chunk_info arg;
+	struct inode *inode;
+	int ret;
+	size_t buf_size;
+	u64 data_offset;
+	const size_t buf_limit = SZ_16M;
+
+
+	data_offset = sizeof(struct btrfs_ioctl_chunk_info);
+	inode = file_inode(file);
+
+	if (copy_from_user(&arg, argp, sizeof(arg)))
+		return -EFAULT;
+
+	buf_size = arg.buf_size;
+	arg.items_count = 0;
+
+	if (buf_size < sizeof(struct btrfs_ioctl_chunk_info) +
+			sizeof(struct btrfs_chunk_info))
+		return -EOVERFLOW;
+
+	/* limit result size to 16MB */
+	if (buf_size > buf_limit)
+		buf_size = buf_limit;
+
+	ret = search_chunk_info(inode, &arg.offset, &arg.items_count,
+			argp + data_offset, buf_size - data_offset);
+
+	if (copy_to_user(argp, &arg, data_offset))
+		return -EFAULT;
+
+	return ret;
+}
+
 /*
  * Search INODE_REFs to identify path name of 'dirid' directory
  * in a 'tree_id' tree. and sets path name to 'name'.
@@ -4907,6 +5116,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_subvol_rootref(file, argp);
 	case BTRFS_IOC_INO_LOOKUP_USER:
 		return btrfs_ioctl_ino_lookup_user(file, argp);
+	case BTRFS_IOC_GET_CHUNK_INFO:
+		return btrfs_ioctl_get_chunk_info(file, argp);
 	}
 
 	return -ENOTTY;
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 8134924cfc17..b28f7886dcbd 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -734,6 +734,42 @@ struct btrfs_ioctl_received_subvol_args {
 	__u64	reserved[16];		/* in */
 };
 
+struct btrfs_chunk_info_stripe {
+	__u64 devid;
+	__u64 offset;
+	__u8 dev_uuid[BTRFS_UUID_SIZE];
+};
+
+struct btrfs_chunk_info {
+	/* logical start of this chunk */
+	__u64 offset;
+	/* size of this chunk in bytes */
+	__u64 length;
+
+	__u64 stripe_len;
+	__u64 type;
+
+	/* 2^16 stripes is quite a lot, a second limit is the size of a single
+	 * item in the btree
+	 */
+	__u16 num_stripes;
+
+	/* sub stripes only matter for raid10 */
+	__u16 sub_stripes;
+
+	struct btrfs_chunk_info_stripe stripes[1];
+	/* additional stripes go here */
+};
+
+
+struct btrfs_ioctl_chunk_info {
+	u64			offset;		/* offset to start the search */
+	u32			buf_size;	/* size of the buffer, including
+						 * btrfs_ioctl_chunk_info
+						 */
+	u32			items_count;	/* number of items returned */
+};
+
 /*
  * Caller doesn't want file data in the send stream, even if the
  * search of clone sources doesn't find an extent. UPDATE_EXTENT
@@ -972,5 +1008,7 @@ enum btrfs_err_code {
 				struct btrfs_ioctl_ino_lookup_user_args)
 #define BTRFS_IOC_SNAP_DESTROY_V2 _IOW(BTRFS_IOCTL_MAGIC, 63, \
 				struct btrfs_ioctl_vol_args_v2)
+#define BTRFS_IOC_GET_CHUNK_INFO _IOR(BTRFS_IOCTL_MAGIC, 64, \
+				struct btrfs_ioctl_chunk_info)
 
 #endif /* _UAPI_LINUX_BTRFS_H */
-- 
2.26.0.rc2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC v2] New ioctl BTRFS_IOC_GET_CHUNK_INFO.
  2020-03-19 18:05 ` [PATCH RFC v2] New " Goffredo Baroncelli
@ 2020-03-19 18:15   ` Josef Bacik
  2020-03-19 18:44     ` Goffredo Baroncelli
  0 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-03-19 18:15 UTC (permalink / raw)
  To: Goffredo Baroncelli, linux-btrfs; +Cc: Goffredo Baroncelli

On 3/19/20 2:05 PM, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli <kreijack@inwind.it>
> 
> Add a new ioctl to get info about chunk without requiring the root privileges.
> This allow to a non root user to know how the space of the filesystem is
> allocated.
> 
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
>   fs/btrfs/ioctl.c           | 211 +++++++++++++++++++++++++++++++++++++
>   include/uapi/linux/btrfs.h |  38 +++++++
>   2 files changed, 249 insertions(+)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 40b729dce91c..e9231d597422 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2234,6 +2234,215 @@ static noinline int btrfs_ioctl_tree_search_v2(struct file *file,
>   	return ret;
>   }
>   
> +/*
> + * Return:
> + *	0		-> copied all data, possible further data
> + *	1		-> copied all data, no further data
> + *	-EAGAIN		-> not enough space, restart it
> + *	-EFAULT		-> the user passed an invalid address/size pair
> + */
> +static noinline int copy_chunk_info(struct btrfs_path *path,
> +			       char __user *ubuf,
> +			       size_t buf_size,
> +			       u64 *used_buf,
> +			       int *num_found,
> +			       u64 *offset)
> +{
> +	struct extent_buffer *leaf;
> +	unsigned long item_off;
> +	unsigned long item_len;
> +	int nritems;
> +	int i;
> +	int slot;
> +	struct btrfs_key key;
> +
> +	leaf = path->nodes[0];
> +	slot = path->slots[0];
> +	nritems = btrfs_header_nritems(leaf);
> +
> +	for (i = slot; i < nritems; i++) {
> +		u64 destsize;
> +		struct btrfs_chunk_info ci;
> +		struct btrfs_chunk chunk;
> +		int j, chunk_size;
> +
> +		item_off = btrfs_item_ptr_offset(leaf, i);
> +		item_len = btrfs_item_size_nr(leaf, i);
> +
> +		btrfs_item_key_to_cpu(leaf, &key, i);
> +		/*
> +		 * we are not interested in other items type
> +		 */
> +		if (key.type != BTRFS_CHUNK_ITEM_KEY)
> +			return 1;
> +

We'll leak this to user space, this should probably be handled differently 
right?  Thanks,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC v2] New ioctl BTRFS_IOC_GET_CHUNK_INFO.
  2020-03-19 18:15   ` Josef Bacik
@ 2020-03-19 18:44     ` Goffredo Baroncelli
  2020-03-19 18:50       ` Josef Bacik
  0 siblings, 1 reply; 6+ messages in thread
From: Goffredo Baroncelli @ 2020-03-19 18:44 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs, Goffredo Baroncelli

Hi Josef,

On 3/19/20 7:15 PM, Josef Bacik wrote:
> On 3/19/20 2:05 PM, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>
>> Add a new ioctl to get info about chunk without requiring the root privileges.
>> This allow to a non root user to know how the space of the filesystem is
>> allocated.
>>
>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>> ---
>>   fs/btrfs/ioctl.c           | 211 +++++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/btrfs.h |  38 +++++++
>>   2 files changed, 249 insertions(+)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 40b729dce91c..e9231d597422 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -2234,6 +2234,215 @@ static noinline int btrfs_ioctl_tree_search_v2(struct file *file,
>>       return ret;
>>   }
>> +/*
>> + * Return:
>> + *    0        -> copied all data, possible further data
>> + *    1        -> copied all data, no further data
>> + *    -EAGAIN        -> not enough space, restart it
>> + *    -EFAULT        -> the user passed an invalid address/size pair
>> + */
>> +static noinline int copy_chunk_info(struct btrfs_path *path,
>> +                   char __user *ubuf,
>> +                   size_t buf_size,
>> +                   u64 *used_buf,
>> +                   int *num_found,
>> +                   u64 *offset)
>> +{
>> +    struct extent_buffer *leaf;
>> +    unsigned long item_off;
>> +    unsigned long item_len;
>> +    int nritems;
>> +    int i;
>> +    int slot;
>> +    struct btrfs_key key;
>> +
>> +    leaf = path->nodes[0];
>> +    slot = path->slots[0];
>> +    nritems = btrfs_header_nritems(leaf);
>> +
>> +    for (i = slot; i < nritems; i++) {
>> +        u64 destsize;
>> +        struct btrfs_chunk_info ci;
>> +        struct btrfs_chunk chunk;
>> +        int j, chunk_size;
>> +
>> +        item_off = btrfs_item_ptr_offset(leaf, i);
>> +        item_len = btrfs_item_size_nr(leaf, i);
>> +
>> +        btrfs_item_key_to_cpu(leaf, &key, i);
>> +        /*
>> +         * we are not interested in other items type
>> +         */
>> +        if (key.type != BTRFS_CHUNK_ITEM_KEY)
>> +            return 1;
>> +
> 
> We'll leak this to user space, this should probably be handled differently right?  Thanks,

Likely I am missing something obvious, but I can't understand what can be leaked and why. Could you be so kindly to elaborate your answer ?
Many thanks

> 
> Josef

BR
G.Baroncelli


-- 
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] 6+ messages in thread

* Re: [PATCH RFC v2] New ioctl BTRFS_IOC_GET_CHUNK_INFO.
  2020-03-19 18:44     ` Goffredo Baroncelli
@ 2020-03-19 18:50       ` Josef Bacik
  2020-03-19 19:54         ` Goffredo Baroncelli
  0 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-03-19 18:50 UTC (permalink / raw)
  To: kreijack; +Cc: linux-btrfs

On 3/19/20 2:44 PM, Goffredo Baroncelli wrote:
> Hi Josef,
> 
> On 3/19/20 7:15 PM, Josef Bacik wrote:
>> On 3/19/20 2:05 PM, Goffredo Baroncelli wrote:
>>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>>
>>> Add a new ioctl to get info about chunk without requiring the root privileges.
>>> This allow to a non root user to know how the space of the filesystem is
>>> allocated.
>>>
>>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>>> ---
>>>   fs/btrfs/ioctl.c           | 211 +++++++++++++++++++++++++++++++++++++
>>>   include/uapi/linux/btrfs.h |  38 +++++++
>>>   2 files changed, 249 insertions(+)
>>>
>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>> index 40b729dce91c..e9231d597422 100644
>>> --- a/fs/btrfs/ioctl.c
>>> +++ b/fs/btrfs/ioctl.c
>>> @@ -2234,6 +2234,215 @@ static noinline int btrfs_ioctl_tree_search_v2(struct 
>>> file *file,
>>>       return ret;
>>>   }
>>> +/*
>>> + * Return:
>>> + *    0        -> copied all data, possible further data
>>> + *    1        -> copied all data, no further data
>>> + *    -EAGAIN        -> not enough space, restart it
>>> + *    -EFAULT        -> the user passed an invalid address/size pair
>>> + */
>>> +static noinline int copy_chunk_info(struct btrfs_path *path,
>>> +                   char __user *ubuf,
>>> +                   size_t buf_size,
>>> +                   u64 *used_buf,
>>> +                   int *num_found,
>>> +                   u64 *offset)
>>> +{
>>> +    struct extent_buffer *leaf;
>>> +    unsigned long item_off;
>>> +    unsigned long item_len;
>>> +    int nritems;
>>> +    int i;
>>> +    int slot;
>>> +    struct btrfs_key key;
>>> +
>>> +    leaf = path->nodes[0];
>>> +    slot = path->slots[0];
>>> +    nritems = btrfs_header_nritems(leaf);
>>> +
>>> +    for (i = slot; i < nritems; i++) {
>>> +        u64 destsize;
>>> +        struct btrfs_chunk_info ci;
>>> +        struct btrfs_chunk chunk;
>>> +        int j, chunk_size;
>>> +
>>> +        item_off = btrfs_item_ptr_offset(leaf, i);
>>> +        item_len = btrfs_item_size_nr(leaf, i);
>>> +
>>> +        btrfs_item_key_to_cpu(leaf, &key, i);
>>> +        /*
>>> +         * we are not interested in other items type
>>> +         */
>>> +        if (key.type != BTRFS_CHUNK_ITEM_KEY)
>>> +            return 1;
>>> +
>>
>> We'll leak this to user space, this should probably be handled differently 
>> right?  Thanks,
> 
> Likely I am missing something obvious, but I can't understand what can be leaked 
> and why. Could you be so kindly to elaborate your answer ?
> Many thanks
> 

we return 1 here

+               ret = copy_chunk_info(path, ubuf, buf_size,
+                                     &used_buf, items_count, offset);
+
+               btrfs_release_path(path);
+               if (ret)
+                       break;

this bit breaks and we return ret

+       ret = search_chunk_info(inode, &arg.offset, &arg.items_count,
+                       argp + data_offset, buf_size - data_offset);
+
+       if (copy_to_user(argp, &arg, data_offset))
+               return -EFAULT;
+
+       return ret;

and we return ret here.  So we'd return 1 to userspace.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC v2] New ioctl BTRFS_IOC_GET_CHUNK_INFO.
  2020-03-19 18:50       ` Josef Bacik
@ 2020-03-19 19:54         ` Goffredo Baroncelli
  0 siblings, 0 replies; 6+ messages in thread
From: Goffredo Baroncelli @ 2020-03-19 19:54 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs

On 3/19/20 7:50 PM, Josef Bacik wrote:
> On 3/19/20 2:44 PM, Goffredo Baroncelli wrote:
>> Hi Josef,
>>
>> On 3/19/20 7:15 PM, Josef Bacik wrote:
>>> On 3/19/20 2:05 PM, Goffredo Baroncelli wrote:
>>>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>>>
>>>> Add a new ioctl to get info about chunk without requiring the root privileges.
>>>> This allow to a non root user to know how the space of the filesystem is
>>>> allocated.
>>>>
>>>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>>>> ---
>>>>   fs/btrfs/ioctl.c           | 211 +++++++++++++++++++++++++++++++++++++
>>>>   include/uapi/linux/btrfs.h |  38 +++++++
>>>>   2 files changed, 249 insertions(+)
>>>>
>>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>>> index 40b729dce91c..e9231d597422 100644
>>>> --- a/fs/btrfs/ioctl.c
>>>> +++ b/fs/btrfs/ioctl.c
>>>> @@ -2234,6 +2234,215 @@ static noinline int btrfs_ioctl_tree_search_v2(struct file *file,
>>>>       return ret;
>>>>   }
>>>> +/*
>>>> + * Return:
>>>> + *    0        -> copied all data, possible further data
>>>> + *    1        -> copied all data, no further data
>>>> + *    -EAGAIN        -> not enough space, restart it
>>>> + *    -EFAULT        -> the user passed an invalid address/size pair
>>>> + */
>>>> +static noinline int copy_chunk_info(struct btrfs_path *path,
>>>> +                   char __user *ubuf,
>>>> +                   size_t buf_size,
>>>> +                   u64 *used_buf,
>>>> +                   int *num_found,
>>>> +                   u64 *offset)
>>>> +{
>>>> +    struct extent_buffer *leaf;
>>>> +    unsigned long item_off;
>>>> +    unsigned long item_len;
>>>> +    int nritems;
>>>> +    int i;
>>>> +    int slot;
>>>> +    struct btrfs_key key;
>>>> +
>>>> +    leaf = path->nodes[0];
>>>> +    slot = path->slots[0];
>>>> +    nritems = btrfs_header_nritems(leaf);
>>>> +
>>>> +    for (i = slot; i < nritems; i++) {
>>>> +        u64 destsize;
>>>> +        struct btrfs_chunk_info ci;
>>>> +        struct btrfs_chunk chunk;
>>>> +        int j, chunk_size;
>>>> +
>>>> +        item_off = btrfs_item_ptr_offset(leaf, i);
>>>> +        item_len = btrfs_item_size_nr(leaf, i);
>>>> +
>>>> +        btrfs_item_key_to_cpu(leaf, &key, i);
>>>> +        /*
>>>> +         * we are not interested in other items type
>>>> +         */
>>>> +        if (key.type != BTRFS_CHUNK_ITEM_KEY)
>>>> +            return 1;
>>>> +
>>>
>>> We'll leak this to user space, this should probably be handled differently right?  Thanks,
>>
>> Likely I am missing something obvious, but I can't understand what can be leaked and why. Could you be so kindly to elaborate your answer ?
>> Many thanks
>>
> 
> we return 1 here
> 
> +               ret = copy_chunk_info(path, ubuf, buf_size,
> +                                     &used_buf, items_count, offset);
> +
> +               btrfs_release_path(path);
> +               if (ret)
> +                       break;
> 
> this bit breaks and we return ret
> 
> +       ret = search_chunk_info(inode, &arg.offset, &arg.items_count,
> +                       argp + data_offset, buf_size - data_offset);
> +
> +       if (copy_to_user(argp, &arg, data_offset))
> +               return -EFAULT;
> +
> +       return ret;
> 
> and we return ret here.  So we'd return 1 to userspace.  Thanks,
> 
> Josef

Good catch !
Thanks for the review. I updated the patch and now I am testing it.

BR
G.Baroncelli


-- 
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] 6+ messages in thread

end of thread, other threads:[~2020-03-19 20:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 18:05 [PATCH RFC V2] new ioctl BTRFS_IOC_GET_CHUNK_INFO Goffredo Baroncelli
2020-03-19 18:05 ` [PATCH RFC v2] New " Goffredo Baroncelli
2020-03-19 18:15   ` Josef Bacik
2020-03-19 18:44     ` Goffredo Baroncelli
2020-03-19 18:50       ` Josef Bacik
2020-03-19 19:54         ` Goffredo Baroncelli

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.