Skip to contentSkip to conversation

Btrfs: get/set label of a mounted file system

RFC PATCH Superseded Jeff Liu 4 participants 8 messages Dec 18, 2012 – Dec 20, 2012 Btrfs file system

v6 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Jeff Liu

author
#0 lore raw

v6 cover Btrfs: get/set label of a mounted file system

Hello,

Here is quick fix for btrfs set lable ioctl(2) based on v5, it does label length check up with strnlen() to avoid searching a zero could go beyond the page limit touching an un-mapped page and raising an segmentation fault with strlen().

V6->v5:
- Improve the input label string validation with strnlen().

The old versions can be found at:
v5:
http://www.spinics.net/lists/linux-btrfs/msg20888.html

v4:
http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21618

v3:
https://patchwork.kernel.org/patch/1124642/

v2:
http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877

v1:
http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872

Thanks,
-Jeff

author
#1 lore raw

[RFC PATCH V6 2/1] Btrfs: Add a new ioctl to get the label of a mounted file system

Introduce a new ioctl BTRFS_IOC_GET_FSLABEL to fetch the label of a mounted file system.

Signed-off-by Jie Liu <[email protected]>
Signed-off-by Anand Jain <[email protected]>
Reviewed-by Miao Xie <[email protected]>
---
 fs/btrfs/ioctl.c |   15 +++++++++++++++
 fs/btrfs/ioctl.h |    2 ++
 2 files changed, 17 insertions(+)
fs/btrfs/ioctl.c +15 -0
index 8fcf9a5..6a2488a 100644
@@ -3699,6 +3699,19 @@ out:
 	return ret;
 }
 
+static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	const char *label = root->fs_info->super_copy->label;
+	int ret;
+
+	mutex_lock(&root->fs_info->volume_mutex);
+	ret = copy_to_user(arg, label, strlen(label));
+	mutex_unlock(&root->fs_info->volume_mutex);
+
+	return ret ? -EFAULT : 0;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {

@@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_qgroup_create(root, argp);
 	case BTRFS_IOC_QGROUP_LIMIT:
 		return btrfs_ioctl_qgroup_limit(root, argp);
+	case BTRFS_IOC_GET_FSLABEL:
+		return btrfs_ioctl_get_fslabel(file, argp);
 	}
 
 	return -ENOTTY;
fs/btrfs/ioctl.h +2 -0
index 731e287..5b2cbef 100644
@@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args {
 			       struct btrfs_ioctl_qgroup_create_args)
 #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \
 			       struct btrfs_ioctl_qgroup_limit_args)
+#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
+				   char[BTRFS_LABEL_SIZE])
 #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
 				      struct btrfs_ioctl_get_dev_stats)
 #endif
Signature · 1 line
1.7.9.5
author
#2 lore raw

RFC PATCH v6 2/2 Btrfs: Add a new ioctl to change the label of a mounted file system

In reply to Jeff Liu

Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system.

Signed-off-by Jie Liu <[email protected]>
Signed-off-by Anand Jain <[email protected]>
Cc Miao Xie <[email protected]>
Cc Goffredo Baroncelli <[email protected]>
---
 fs/btrfs/ioctl.c |   39 +++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h |    2 ++
 2 files changed, 41 insertions(+)
fs/btrfs/ioctl.c +39 -0
index 6a2488a..2bb037e 100644
@@ -3712,6 +3712,43 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg)
 	return ret ? -EFAULT : 0;
 }
 
+static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct btrfs_super_block *super_block = root->fs_info->super_copy;
+	struct btrfs_trans_handle *trans;
+	char label[BTRFS_LABEL_SIZE];
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, arg, sizeof(label)))
+		return -EFAULT;
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
+	mutex_lock(&root->fs_info->volume_mutex);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_unlock;
+	}
+
+	strcpy(super_block->label, label);
+	btrfs_end_transaction(trans, root);
+
+out_unlock:
+	mutex_unlock(&root->fs_info->volume_mutex);
+	mnt_drop_write_file(file);
+	return ret;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {

@@ -3812,6 +3849,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_qgroup_limit(root, argp);
 	case BTRFS_IOC_GET_FSLABEL:
 		return btrfs_ioctl_get_fslabel(file, argp);
+	case BTRFS_IOC_SET_FSLABEL:
+		return btrfs_ioctl_set_fslabel(file, argp);
 	}
 
 	return -ENOTTY;
fs/btrfs/ioctl.h +2 -0
index 5b2cbef..2abe239 100644
@@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args {
 			       struct btrfs_ioctl_qgroup_limit_args)
 #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
 				   char[BTRFS_LABEL_SIZE])
+#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
+				   char[BTRFS_LABEL_SIZE])
 #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
 				      struct btrfs_ioctl_get_dev_stats)
 #endif
Signature · 1 line
1.7.9.5
Reviewed-by on v6 2/2
#3 lore raw

Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system

Quoted 6 lines · Jeff Liu

Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system.

Signed-off-by Jie Liu <[email protected]>
Signed-off-by Anand Jain <[email protected]>
Cc Miao Xie <[email protected]>
Cc Goffredo Baroncelli <[email protected]>
Reviewed-by Miao Xie <[email protected]>
Quoted 77 lines
---
 fs/btrfs/ioctl.c |   39 +++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h |    2 ++
 2 files changed, 41 insertions(+)
fs/btrfs/ioctl.c +39 -0
index 6a2488a..2bb037e 100644
@@ -3712,6 +3712,43 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg)
 	return ret ? -EFAULT : 0;
 }
 
+static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct btrfs_super_block *super_block = root->fs_info->super_copy;
+	struct btrfs_trans_handle *trans;
+	char label[BTRFS_LABEL_SIZE];
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, arg, sizeof(label)))
+		return -EFAULT;
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
+	mutex_lock(&root->fs_info->volume_mutex);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_unlock;
+	}
+
+	strcpy(super_block->label, label);
+	btrfs_end_transaction(trans, root);
+
+out_unlock:
+	mutex_unlock(&root->fs_info->volume_mutex);
+	mnt_drop_write_file(file);
+	return ret;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {

@@ -3812,6 +3849,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_qgroup_limit(root, argp);
 	case BTRFS_IOC_GET_FSLABEL:
 		return btrfs_ioctl_get_fslabel(file, argp);
+	case BTRFS_IOC_SET_FSLABEL:
+		return btrfs_ioctl_set_fslabel(file, argp);
 	}
 
 	return -ENOTTY;
fs/btrfs/ioctl.h +2 -0
index 5b2cbef..2abe239 100644
@@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args {
 			       struct btrfs_ioctl_qgroup_limit_args)
 #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
 				   char[BTRFS_LABEL_SIZE])
+#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
+				   char[BTRFS_LABEL_SIZE])
 #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
 				      struct btrfs_ioctl_get_dev_stats)
 #endif
#4 lore raw

Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system

Hi Jeff,

On 12/18/2012 04:31 AM, Miao Xie wrote:
[...]

Quoted 1 line
Quoted 1 line
fs/btrfs/ioctl.c +0 -0

[...]

Quoted 21 lines
Quoted 21 lines
fs/btrfs/ioctl.c +21 -0
+static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct btrfs_super_block *super_block = root->fs_info->super_copy;
+	struct btrfs_trans_handle *trans;
+	char label[BTRFS_LABEL_SIZE];
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, arg, sizeof(label)))
+		return -EFAULT;
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
Quoted 8 lines

fs/btrfs/ioctl.c static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) · line 3743

Quoted 8 lines
fs/btrfs/ioctl.c +8 -0
+	mutex_lock(&root->fs_info->volume_mutex);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_unlock;
+	}
+
+	strcpy(super_block->label, label);

I think that you removed for mistake the following line

+ label[BTRFS_LABEL_SIZE - 1] = '\0';

In the V5 patch it was present.

May be we could replace strcpy() with strlcpy(super_block->label, label, BTRFS_LABEL_SIZE-1) ?

BR
G.Baroncelli

Signature · 2 lines
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
author
#5 lore raw

Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system

Hi Goffredo,

Thanks for your review.

Quoted 39 lines · Goffredo Baroncelli

Hi Jeff,

On 12/18/2012 04:31 AM, Miao Xie wrote:
[...]

Quoted 1 line
Quoted 1 line
fs/btrfs/ioctl.c +0 -0

[...]

Quoted 21 lines
Quoted 21 lines
fs/btrfs/ioctl.c +21 -0
+static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct btrfs_super_block *super_block = root->fs_info->super_copy;
+	struct btrfs_trans_handle *trans;
+	char label[BTRFS_LABEL_SIZE];
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, arg, sizeof(label)))
+		return -EFAULT;
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
Quoted 8 lines

fs/btrfs/ioctl.c static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) · line 3743

Quoted 8 lines
fs/btrfs/ioctl.c +8 -0
+	mutex_lock(&root->fs_info->volume_mutex);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_unlock;
+	}
+
+	strcpy(super_block->label, label);

I think that you removed for mistake the following line

Review

Quoted 1 line
(quoted diff) +1 -0
+	label[BTRFS_LABEL_SIZE - 1] = '\0';

I removed it since it was used to cut the label string off the max array size but now we have the previous strnlen().

Quoted 5 lines

In the V5 patch it was present.

May be we could replace strcpy() with strlcpy(super_block->label, label, BTRFS_LABEL_SIZE-1) ?

That is ok to me. However, it should be strlcpy(super_block->label, label, BTRFS_LABEL_SIZE) ranther than 'BTRFS_LABREL_SIZE -1' because strlcpy() does "size - 1" internally. i.e.

strlcpy(char *d, const char *s, size_t size)

{	
	size_t ret = strlen(s);

	.....
	size_t len = (ret >= size) ? size - 1 : ret;
	....
}

But does the current implementation make anything wrong? :)

Thanks,
-Jeff

Quoted 4 lines

BR
G.Baroncelli

#6 lore raw

Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system

In reply to Jeff Liu
Quoted 20 lines · Jeff Liu
fs/btrfs/ioctl.c +20 -0
+static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct btrfs_super_block *super_block = root->fs_info->super_copy;
+	struct btrfs_trans_handle *trans;
+	char label[BTRFS_LABEL_SIZE];
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, arg, sizeof(label)))
+		return -EFAULT;
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;

fs/btrfs/ioctl.c static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) · line 3744

Quoted 10 lines
fs/btrfs/ioctl.c +10 -0
+
+	mutex_lock(&root->fs_info->volume_mutex);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_unlock;
+	}
+
+	strcpy(super_block->label, label);
+	btrfs_end_transaction(trans, root);

If this fails, eg. with EIO, it will not be reported back to the user

        ret = btrfs_end_transaction(trans, root);

should fix it.

Quoted 6 lines
fs/btrfs/ioctl.c +6 -0
+
+out_unlock:
+	mutex_unlock(&root->fs_info->volume_mutex);
+	mnt_drop_write_file(file);
+	return ret;
+}
author
#7 lore raw

Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system

Quoted 34 lines · David Sterba
Quoted 20 lines · Jeff Liu
fs/btrfs/ioctl.c +20 -0
+static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct btrfs_super_block *super_block = root->fs_info->super_copy;
+	struct btrfs_trans_handle *trans;
+	char label[BTRFS_LABEL_SIZE];
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, arg, sizeof(label)))
+		return -EFAULT;
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;

fs/btrfs/ioctl.c static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) · line 3744

Quoted 10 lines
fs/btrfs/ioctl.c +10 -0
+
+	mutex_lock(&root->fs_info->volume_mutex);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_unlock;
+	}
+
+	strcpy(super_block->label, label);
+	btrfs_end_transaction(trans, root);

If this fails, eg. with EIO, it will not be reported back to the user

Quoted 3 lines
        ret = btrfs_end_transaction(trans, root);

should fix it.

Thank you, I'll send out a revised version after a little while.

-Jeff

Quoted 12 lines

fs/btrfs/ioctl.c static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) · line 3750

Quoted 7 lines
fs/btrfs/ioctl.c +6 -0
+
+out_unlock:
+	mutex_unlock(&root->fs_info->volume_mutex);
+	mnt_drop_write_file(file);
+	return ret;
+}

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html