Btrfs: added new ioctl to set fs label
PATCH 1/1 Btrfs: added new ioctl to set fs label
Hello,
I'd like to introduce a new ioctl to set file system label.
With this feature, we can execute btrfs filesystem label [label] [path] through btrfs tools to set or change the label.
--- fs/btrfs/ctree.h | 6 ++++++ fs/btrfs/ioctl.c | 37 +++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 3 files changed, 45 insertions(+), 0 deletions(-)
fs/btrfs/ctree.h +6 -0
@@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { }; +struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE ]; +}; +/* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and
dir in
fs/btrfs/ioctl.c +35 -0
@@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)return put_user (inode->i_generation, arg); } +static int btrfs_ioctl_fs_setlabel (struct btrfs_root *root,void __user *arg) +{ +struct btrfs_super_block *super_block = &(root->fs_info->super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +if (btrfs_root_readonly (root)) +return -EROFS ; + + label_args =memdup_user (arg,sizeof (*label_args)); +if (IS_ERR (label_args)) +return PTR_ERR (label_args); + +if (label_args->len >=sizeof (label_args->label)) +return -EINVAL ; + +mutex_lock (&root->fs_info->volume_mutex); + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return PTR_ERR (trans); + +if (snprintf (super_block->label,BTRFS_LABEL_SIZE ,"%s" , + label_args->label) != label_args->len) +return -EFAULT ; + +btrfs_end_transaction (trans, root); +mutex_unlock (&root->fs_info->volume_mutex); + +kfree (label_args); +return 0 ; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void __user
*arg)
{
struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
@@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_fs_info(root, argp);
case BTRFS_IOC_DEV_INFO:
return btrfs_ioctl_dev_info(root, argp);
+ case BTRFS_IOC_FS_SETLABEL:
+ return btrfs_ioctl_fs_setlabel(root, argp);
case BTRFS_IOC_BALANCE:
return btrfs_balance(root->fs_info->dev_root);
case BTRFS_IOC_CLONE:fs/btrfs/ioctl.h +2 -0
@@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_dev_info_args)#define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args)#endif
Signature · 1 line
1.7.4.1
Quoted 19 lines · Jeff Liu
Hello,
I'd like to introduce a new ioctl to set file system label. With this feature, we can execute
btrfs filesystem label [label] [path]through btrfs tools to set or change the label.Signed-off-by Jie Liu <[email protected]>--- fs/btrfs/ctree.h | 6 ++++++ fs/btrfs/ioctl.c | 37 +++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 3 files changed, 45 insertions(+), 0 deletions(-)fs/btrfs/ctree.h +0
-0
fs/btrfs/ctree.h struct btrfs_ioctl_defrag_range_args { · line 1266
Quoted 8 lines
fs/btrfs/ctree.h +5
-0@@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { }; +struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE ]; +};
Why do we need to provide a length here? Simply force a zero byte
at the end of the string when you copy it into kernel space, and then use strcpy(). Then you have no need to test for length at all.
Quoted 22 lines
fs/btrfs/ctree.h +0
-0/* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in
fs/btrfs/ioctl.c +8
-0@@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)return put_user (inode->i_generation, arg); } +static int btrfs_ioctl_fs_setlabel (struct btrfs_root *root,void __user *arg) +{ +struct btrfs_super_block *super_block = &(root->fs_info->super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ;
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 290
Quoted 10 lines
fs/btrfs/ioctl.c +10
-0+ +if (btrfs_root_readonly (root)) +return -EROFS ; + + label_args =memdup_user (arg,sizeof (*label_args)); +if (IS_ERR (label_args)) +return PTR_ERR (label_args); + +if (label_args->len >=sizeof (label_args->label)) +return -EINVAL ;
Memory leak... you're not freeing label_args
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 293
Quoted 4 lines
fs/btrfs/ioctl.c +4
-0+mutex_lock (&root->fs_info->volume_mutex); + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return PTR_ERR (trans);
and here -- and you're leaving the mutex locked
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 297
Quoted 3 lines
fs/btrfs/ioctl.c +3
-0+if (snprintf (super_block->label,BTRFS_LABEL_SIZE ,"%s" , + label_args->label) != label_args->len) +return -EFAULT ;
and here -- plus the transaction is still running
Quoted 25 lines
fs/btrfs/ioctl.c +7
-0+btrfs_end_transaction (trans, root); +mutex_unlock (&root->fs_info->volume_mutex); + +kfree (label_args); +return 0 ; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void
__user*arg){ struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;fs/btrfs/ioctl.c +2
-0@@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned intreturn btrfs_ioctl_fs_info (root, argp);case BTRFS_IOC_DEV_INFO :return btrfs_ioctl_dev_info (root, argp); +case BTRFS_IOC_FS_SETLABEL : +return btrfs_ioctl_fs_setlabel (root, argp);case BTRFS_IOC_BALANCE :return btrfs_balance (root->fs_info->dev_root);case BTRFS_IOC_CLONE :fs/btrfs/ioctl.h +0
-0
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 252
Quoted 5 lines
fs/btrfs/ioctl.h +2
-0@@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_dev_info_args)#define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args)
Could you use an unassigned number from [1], and update the wiki
page, please? (The page only went up yesterday, but it's been needed for a while).
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 253
Quoted 1 line
fs/btrfs/ioctl.h +0
-0#endif
Will there be a userspace patch for this along shortly?
Hugo.
[1] https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read
Signature · 4 lines
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- You can't expect a boy to be depraved until he's gone to ---
a good school. Hi Hugo,
Quoted 23 lines · Hugo Mills
Quoted 22 lines · Jeff Liu
Hello,
I'd like to introduce a new ioctl to set file system label. With this feature, we can execute
btrfs filesystem label [label] [path]through btrfs tools to set or change the label.Signed-off-by Jie Liu<[email protected]>--- fs/btrfs/ctree.h | 6 ++++++ fs/btrfs/ioctl.c | 37 +++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 3 files changed, 45 insertions(+), 0 deletions(-)fs/btrfs/ctree.h +0
-0@@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { };
Quoted 8 lines
fs/btrfs/ctree.h struct btrfs_ioctl_defrag_range_args { · line 1266
Quoted 5 lines
fs/btrfs/ctree.h +5
-0+struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE ]; +};Why do we need to provide a length here? Simply force a zero byte
at the end of the string when you copy it into kernel space, and then usestrcpy(). Then you have no need to test for length at all.
At first, I am afraid if an evil user may input an unexpected label string with huge bytes to consume memory.
Quoted 37 lines
Quoted 22 lines
fs/btrfs/ctree.h +0
-0/* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in
fs/btrfs/ioctl.c +8
-0@@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)return put_user (inode->i_generation, arg); } +static int btrfs_ioctl_fs_setlabel (struct btrfs_root *root,void __user *arg) +{ +struct btrfs_super_block *super_block =&(root->fs_info->super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ;fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 290
Quoted 10 lines
fs/btrfs/ioctl.c +10
-0+ +if (btrfs_root_readonly (root)) +return -EROFS ; + + label_args =memdup_user (arg,sizeof (*label_args)); +if (IS_ERR (label_args)) +return PTR_ERR (label_args); + +if (label_args->len>=sizeof (label_args->label)) +return -EINVAL ;Memory leak... you're not freeing
label_argsfs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 293
Quoted 4 lines
fs/btrfs/ioctl.c +4
-0+mutex_lock (&root->fs_info->volume_mutex); + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return PTR_ERR (trans);
Quoted 6 lines
and here -- and you're leaving the mutex locked
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 297
Quoted 3 lines
fs/btrfs/ioctl.c +3
-0+if (snprintf (super_block->label,BTRFS_LABEL_SIZE ,"%s" , + label_args->label) != label_args->len) +return -EFAULT ;and here -- plus the transaction is still running
Sorry for my stupid mistake and thanks for pointing those out!
Quoted 25 lines
Quoted 25 lines
fs/btrfs/ioctl.c +7
-0+btrfs_end_transaction (trans, root); +mutex_unlock (&root->fs_info->volume_mutex); + +kfree (label_args); +return 0 ; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void
__user*arg){ struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;fs/btrfs/ioctl.c +2
-0@@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned intreturn btrfs_ioctl_fs_info (root, argp);case BTRFS_IOC_DEV_INFO :return btrfs_ioctl_dev_info (root, argp); +case BTRFS_IOC_FS_SETLABEL : +return btrfs_ioctl_fs_setlabel (root, argp);case BTRFS_IOC_BALANCE :return btrfs_balance (root->fs_info->dev_root);case BTRFS_IOC_CLONE :fs/btrfs/ioctl.h +0
-0@@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args {
Quoted 8 lines
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 252
Quoted 5 lines
fs/btrfs/ioctl.h +2
-0struct btrfs_ioctl_dev_info_args)#define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args)Could you use an unassigned number from [1], and update the wiki
page, please? (The page only went up yesterday, but it's been needed for a while).
Ok, looks number 5 is free. :)
I'll update the wiki later.
Regards,
-Jeff
Quoted 7 lines
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 253
Quoted 1 line
fs/btrfs/ioctl.h +0
-0#endif Will there be a userspace patch for this along shortly?
Hugo.
[1] https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read
Quoted 34 lines · Jeff Liu
Hi Hugo,
Quoted 23 lines · Hugo Mills
Quoted 22 lines · Jeff Liu
Hello,
I'd like to introduce a new ioctl to set file system label. With this feature, we can execute
btrfs filesystem label [label] [path]through btrfs tools to set or change the label.Signed-off-by Jie Liu<[email protected]>--- fs/btrfs/ctree.h | 6 ++++++ fs/btrfs/ioctl.c | 37 +++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 3 files changed, 45 insertions(+), 0 deletions(-)fs/btrfs/ctree.h +0
-0@@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { };Quoted 8 lines
fs/btrfs/ctree.h struct btrfs_ioctl_defrag_range_args { · line 1266
Quoted 5 lines
fs/btrfs/ctree.h +5
-0+struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE ]; +};Why do we need to provide a length here? Simply force a zero byte
at the end of the string when you copy it into kernel space, and then usestrcpy(). Then you have no need to test for length at all.
Quoted 2 lines
At first, I am afraid if an evil user may input an unexpected label string with huge bytes to consume memory.
That's why you force a known 0 byte at the end of the string when
you do the copy. (See below) Note also that the evil user can't consume more than sizeof(btrfs_ioctl_fs_label_args) anyway, because that's how much you're memdup()ing. The only issue is dealing with an unterminated string... which you can fix by explicitly terminating it.
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
fs/btrfs/ctree.h +0
-0/* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in
fs/btrfs/ioctl.c +0
-0@@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)return put_user (inode->i_generation, arg); }
Quoted 16 lines
Quoted 8 lines
Quoted 8 lines
fs/btrfs/ioctl.c +7
-0+static int btrfs_ioctl_fs_setlabel (struct btrfs_root *root,void __user *arg) +{ +struct btrfs_super_block *super_block =&(root->fs_info->super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable (CAP_SYS_ADMIN ))Quoted 8 lines
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 285
Quoted 8 lines
fs/btrfs/ioctl.c +8
-0+return -EPERM ; + +if (btrfs_root_readonly (root)) +return -EROFS ; + + label_args =memdup_user (arg,sizeof (*label_args)); +if (IS_ERR (label_args)) +return PTR_ERR (label_args);
label_args->label[BTRFS_LABEL_SIZE-1] = 0;
This guarantees that the string is no longer than
BTRFS_LABEL_SIZE-1 bytes long.
Quoted 2 lines
Quoted 2 lines
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 289
Quoted 2 lines
fs/btrfs/ioctl.c +2
-0+if (label_args->len>=sizeof (label_args->label)) +return -EINVAL ;
Having ensured that the string is no longer than our buffers, we
don't need this.
Quoted 9 lines
Quoted 9 lines
Memory leak... you're not freeing
label_argsfs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 293
Quoted 4 lines
fs/btrfs/ioctl.c +4
-0+mutex_lock (&root->fs_info->volume_mutex); + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return PTR_ERR (trans);and here -- and you're leaving the mutex locked
Quoted 2 lines
fs/btrfs/ioctl.c +2
-0+if (snprintf (super_block->label,BTRFS_LABEL_SIZE ,"%s" , + label_args->label) != label_args->len)
It's now safe to use strcpy() here, since we know that the string
*must* be zero terminated at or before BTRFS_LABEL_SIZE.
Quoted 50 lines
Quoted 2 lines
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 297
Quoted 1 line
fs/btrfs/ioctl.c +1
-0+return -EFAULT ;and here -- plus the transaction is still running
Sorry for my stupid mistake and thanks for pointing those out!
Quoted 25 lines
Quoted 25 lines
fs/btrfs/ioctl.c +7
-0+btrfs_end_transaction (trans, root); +mutex_unlock (&root->fs_info->volume_mutex); + +kfree (label_args); +return 0 ; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void
__user*arg){ struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;fs/btrfs/ioctl.c +2
-0@@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned intreturn btrfs_ioctl_fs_info (root, argp);case BTRFS_IOC_DEV_INFO :return btrfs_ioctl_dev_info (root, argp); +case BTRFS_IOC_FS_SETLABEL : +return btrfs_ioctl_fs_setlabel (root, argp);case BTRFS_IOC_BALANCE :return btrfs_balance (root->fs_info->dev_root);case BTRFS_IOC_CLONE :fs/btrfs/ioctl.h +0
-0@@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args {Quoted 8 lines
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 252
Quoted 5 lines
fs/btrfs/ioctl.h +2
-0struct btrfs_ioctl_dev_info_args)#define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args)Could you use an unassigned number from [1], and update the wiki
page, please? (The page only went up yesterday, but it's been needed for a while).Ok, looks number 5 is free. :)
I'll update the wiki later.Regards,
-JeffQuoted 7 lines
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 253
Quoted 1 line
fs/btrfs/ioctl.h +0
-0#endif Will there be a userspace patch for this along shortly?
Hugo.
[1] https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read
Signature · 3 lines
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- He's playing Schubert. I think Schubert is losing. --- PATCH 1/1 Btrfs: added new ioctl to set fs label V2
Thanks again for your nice comments!
The wiki update is in progress.
Btw, is it make sense to improve the "struct btrfs_ioctl_fs_info_args" to retrieve the label info through BTRFS_IOC_FS_INFO?
Would you please review the revised version?
--- fs/btrfs/ctree.h | 4 ++++ fs/btrfs/ioctl.c | 36 ++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 3 files changed, 42 insertions(+), 0 deletions(-)
fs/btrfs/ctree.h +4 -0
@@ -1259,6 +1259,10 @@ struct btrfs_ioctl_defrag_range_args { }; +struct btrfs_ioctl_fs_label_args { +char label[BTRFS_LABEL_SIZE ]; +}; +/* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and
dir in
fs/btrfs/ioctl.c +34 -0
@@ -268,6 +268,40 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)return put_user (inode->i_generation, arg); } +static int btrfs_ioctl_fs_setlabel (struct btrfs_root *root,void __user *arg) +{ +struct btrfs_super_block *super_block = &(root->fs_info->super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; +int ret; + +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +if (btrfs_root_readonly (root)) +return -EROFS ; + + label_args =memdup_user (arg,sizeof (*label_args)); +if (IS_ERR (label_args)) +return PTR_ERR (label_args); + + label_args->label[BTRFS_LABEL_SIZE -1 ] ='\0' ; + +mutex_lock (&root->fs_info->volume_mutex); + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) { + ret =PTR_ERR (trans); + goto out_unlock; + } +strcpy (super_block->label, label_args->label); +btrfs_end_transaction (trans, root); + +out_unlock: +mutex_unlock (&root->fs_info->volume_mutex); +kfree (label_args); +return 0 ; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void __user
*arg)
{
struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
@@ -2876,6 +2910,8 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_fs_info(root, argp);
case BTRFS_IOC_DEV_INFO:
return btrfs_ioctl_dev_info(root, argp);
+ case BTRFS_IOC_FS_SETLABEL:
+ return btrfs_ioctl_fs_setlabel(root, argp);
case BTRFS_IOC_BALANCE:
return btrfs_balance(root->fs_info->dev_root);
case BTRFS_IOC_CLONE:fs/btrfs/ioctl.h +2 -0
@@ -201,6 +201,8 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_vol_args)#define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \ struct btrfs_ioctl_vol_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 5, \ + struct btrfs_ioctl_fs_label_args)/* trans start and trans end are dangerous, and only for * use by applications that know how to avoid the * resulting deadlocks
Signature · 1 line
1.7.4.1
Quoted 143 lines · Hugo Mills
Quoted 34 lines · Jeff Liu
Hi Hugo,
Quoted 23 lines · Hugo Mills
Quoted 22 lines · Jeff Liu
Hello,
I'd like to introduce a new ioctl to set file system label. With this feature, we can execute
btrfs filesystem label [label] [path]through btrfs tools to set or change the label.Signed-off-by Jie Liu<[email protected]>--- fs/btrfs/ctree.h | 6 ++++++ fs/btrfs/ioctl.c | 37 +++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 3 files changed, 45 insertions(+), 0 deletions(-)fs/btrfs/ctree.h +0
-0@@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { };Quoted 8 lines
fs/btrfs/ctree.h struct btrfs_ioctl_defrag_range_args { · line 1266
Quoted 5 lines
fs/btrfs/ctree.h +5
-0+struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE ]; +};Why do we need to provide a length here? Simply force a zero byte
at the end of the string when you copy it into kernel space, and then usestrcpy(). Then you have no need to test for length at all.Quoted 2 lines
At first, I am afraid if an evil user may input an unexpected label string with huge bytes to consume memory.
That's why you force a known 0 byte at the end of the string when
you do the copy. (See below) Note also that the evil user can't consume more thansizeof(btrfs_ioctl_fs_label_args)anyway, because that's how much you're memdup()ing. The only issue is dealing with an unterminated string... which you can fix by explicitly terminating it.Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
fs/btrfs/ctree.h +0
-0/* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in
fs/btrfs/ioctl.c +0
-0@@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)return put_user (inode->i_generation, arg); }Quoted 16 lines
Quoted 8 lines
Quoted 8 lines
fs/btrfs/ioctl.c +7
-0+static int btrfs_ioctl_fs_setlabel (struct btrfs_root *root,void __user *arg) +{ +struct btrfs_super_block *super_block =&(root->fs_info->super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable (CAP_SYS_ADMIN ))Quoted 8 lines
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 287
Quoted 8 lines
fs/btrfs/ioctl.c +8
-0+return -EPERM ; + +if (btrfs_root_readonly (root)) +return -EROFS ; + + label_args =memdup_user (arg,sizeof (*label_args)); +if (IS_ERR (label_args)) +return PTR_ERR (label_args);label_args->label[BTRFS_LABEL_SIZE-1] = 0;
This guarantees that the string is no longer than
BTRFS_LABEL_SIZE-1 bytes long.Quoted 2 lines
Quoted 2 lines
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 289
Quoted 2 lines
fs/btrfs/ioctl.c +2
-0+if (label_args->len>=sizeof (label_args->label)) +return -EINVAL ;Having ensured that the string is no longer than our buffers, we
don't need this.Quoted 9 lines
Quoted 9 lines
Memory leak... you're not freeing
label_argsfs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 293
Quoted 4 lines
fs/btrfs/ioctl.c +4
-0+mutex_lock (&root->fs_info->volume_mutex); + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return PTR_ERR (trans);and here -- and you're leaving the mutex locked
Quoted 2 lines
fs/btrfs/ioctl.c +2
-0+if (snprintf (super_block->label,BTRFS_LABEL_SIZE ,"%s" , + label_args->label) != label_args->len)It's now safe to use
strcpy()here, since we know that the string
*must* be zero terminated at or beforeBTRFS_LABEL_SIZE.Quoted 49 lines
Quoted 2 lines
fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file · line 296
Quoted 1 line
fs/btrfs/ioctl.c +1
-0+return -EFAULT ;and here -- plus the transaction is still running
Sorry for my stupid mistake and thanks for pointing those out!
Quoted 25 lines
Quoted 25 lines
fs/btrfs/ioctl.c +7
-0+btrfs_end_transaction (trans, root); +mutex_unlock (&root->fs_info->volume_mutex); + +kfree (label_args); +return 0 ; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void
__user*arg){ struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;fs/btrfs/ioctl.c +2
-0@@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned intreturn btrfs_ioctl_fs_info (root, argp);case BTRFS_IOC_DEV_INFO :return btrfs_ioctl_dev_info (root, argp); +case BTRFS_IOC_FS_SETLABEL : +return btrfs_ioctl_fs_setlabel (root, argp);case BTRFS_IOC_BALANCE :return btrfs_balance (root->fs_info->dev_root);case BTRFS_IOC_CLONE :fs/btrfs/ioctl.h +0
-0@@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args {Quoted 8 lines
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 252
Quoted 5 lines
fs/btrfs/ioctl.h +2
-0struct btrfs_ioctl_dev_info_args)#define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args)Could you use an unassigned number from [1], and update the wiki
page, please? (The page only went up yesterday, but it's been needed for a while).Ok, looks number 5 is free. :)
I'll update the wiki later.Regards,
-JeffQuoted 7 lines
fs/btrfs/ioctl.h struct btrfs_ioctl_space_args { · line 253
Quoted 1 line
fs/btrfs/ioctl.h +0
-0#endif Will there be a userspace patch for this along shortly?
Hugo.
[1] https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read