* [RFC PATCH V6 0/2] Btrfs: get/set label of a mounted file system @ 2012-12-18 3:05 Jeff Liu 2012-12-18 3:05 ` [RFC PATCH V6 2/1] Btrfs: Add a new ioctl to get the " Jeff Liu 2012-12-18 3:06 ` [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change " Jeff Liu 0 siblings, 2 replies; 8+ messages in thread From: Jeff Liu @ 2012-12-18 3:05 UTC (permalink / raw) To: linux-btrfs; +Cc: anand.jain, miaox, kreijack 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH V6 2/1] Btrfs: Add a new ioctl to get the label of a mounted file system 2012-12-18 3:05 [RFC PATCH V6 0/2] Btrfs: get/set label of a mounted file system Jeff Liu @ 2012-12-18 3:05 ` Jeff Liu 2012-12-18 3:06 ` [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change " Jeff Liu 1 sibling, 0 replies; 8+ messages in thread From: Jeff Liu @ 2012-12-18 3:05 UTC (permalink / raw) To: linux-btrfs; +Cc: anand.jain, miaox, Goffredo Baroncelli Introduce a new ioctl BTRFS_IOC_GET_FSLABEL to fetch the label of a mounted file system. Signed-off-by: Jie Liu <jeff.liu@oracle.com> Signed-off-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/ioctl.c | 15 +++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..6a2488a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -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; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -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 -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system 2012-12-18 3:05 [RFC PATCH V6 0/2] Btrfs: get/set label of a mounted file system Jeff Liu 2012-12-18 3:05 ` [RFC PATCH V6 2/1] Btrfs: Add a new ioctl to get the " Jeff Liu @ 2012-12-18 3:06 ` Jeff Liu 2012-12-18 3:31 ` Miao Xie 2012-12-19 14:21 ` David Sterba 1 sibling, 2 replies; 8+ messages in thread From: Jeff Liu @ 2012-12-18 3:06 UTC (permalink / raw) To: linux-btrfs; +Cc: anand.jain, miaox, Goffredo Baroncelli Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu <jeff.liu@oracle.com> Signed-off-by: Anand Jain <anand.jain@oracle.com> Cc: Miao Xie <miaox@cn.fujitsu.com> Cc: Goffredo Baroncelli <kreijack@inwind.it> --- fs/btrfs/ioctl.c | 39 +++++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h | 2 ++ 2 files changed, 41 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6a2488a..2bb037e 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -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; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -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 -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system 2012-12-18 3:06 ` [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change " Jeff Liu @ 2012-12-18 3:31 ` Miao Xie 2012-12-18 18:00 ` Goffredo Baroncelli 2012-12-19 14:21 ` David Sterba 1 sibling, 1 reply; 8+ messages in thread From: Miao Xie @ 2012-12-18 3:31 UTC (permalink / raw) To: Jeff Liu; +Cc: linux-btrfs, anand.jain, Goffredo Baroncelli On Tue, 18 Dec 2012 11:06:07 +0800, Jeff Liu wrote: > Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. > > Signed-off-by: Jie Liu <jeff.liu@oracle.com> > Signed-off-by: Anand Jain <anand.jain@oracle.com> > Cc: Miao Xie <miaox@cn.fujitsu.com> > Cc: Goffredo Baroncelli <kreijack@inwind.it> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com> > > --- > fs/btrfs/ioctl.c | 39 +++++++++++++++++++++++++++++++++++++++ > fs/btrfs/ioctl.h | 2 ++ > 2 files changed, 41 insertions(+) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 6a2488a..2bb037e 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -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; > diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h > index 5b2cbef..2abe239 100644 > --- a/fs/btrfs/ioctl.h > +++ b/fs/btrfs/ioctl.h > @@ -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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system 2012-12-18 3:31 ` Miao Xie @ 2012-12-18 18:00 ` Goffredo Baroncelli 2012-12-19 3:42 ` Jeff Liu 0 siblings, 1 reply; 8+ messages in thread From: Goffredo Baroncelli @ 2012-12-18 18:00 UTC (permalink / raw) Cc: miaox, Jeff Liu, linux-btrfs, anand.jain Hi Jeff, On 12/18/2012 04:31 AM, Miao Xie wrote: [...] >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c [...] >> +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); 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 -- 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] 8+ messages in thread
* Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system 2012-12-18 18:00 ` Goffredo Baroncelli @ 2012-12-19 3:42 ` Jeff Liu 0 siblings, 0 replies; 8+ messages in thread From: Jeff Liu @ 2012-12-19 3:42 UTC (permalink / raw) To: kreijack; +Cc: Goffredo Baroncelli, miaox, linux-btrfs, anand.jain Hi Goffredo, Thanks for your review. On 12/19/2012 02:00 AM, Goffredo Baroncelli wrote: > Hi Jeff, > > On 12/18/2012 04:31 AM, Miao Xie wrote: > [...] >>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > [...] > >>> +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); > > I think that you removed for mistake the following line > > + 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(). > > 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 > > BR > G.Baroncelli > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system 2012-12-18 3:06 ` [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change " Jeff Liu 2012-12-18 3:31 ` Miao Xie @ 2012-12-19 14:21 ` David Sterba 2012-12-20 8:18 ` Jeff Liu 1 sibling, 1 reply; 8+ messages in thread From: David Sterba @ 2012-12-19 14:21 UTC (permalink / raw) To: Jeff Liu; +Cc: linux-btrfs, anand.jain, miaox, Goffredo Baroncelli On Tue, Dec 18, 2012 at 11:06:07AM +0800, Jeff Liu wrote: > +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); If this fails, eg. with EIO, it will not be reported back to the user ret = btrfs_end_transaction(trans, root); should fix it. > + > +out_unlock: > + mutex_unlock(&root->fs_info->volume_mutex); > + mnt_drop_write_file(file); > + return ret; > +} ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system 2012-12-19 14:21 ` David Sterba @ 2012-12-20 8:18 ` Jeff Liu 0 siblings, 0 replies; 8+ messages in thread From: Jeff Liu @ 2012-12-20 8:18 UTC (permalink / raw) To: dsterba; +Cc: linux-btrfs, anand.jain, miaox, Goffredo Baroncelli On 12/19/2012 10:21 PM, David Sterba wrote: > On Tue, Dec 18, 2012 at 11:06:07AM +0800, Jeff Liu wrote: >> +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); > > If this fails, eg. with EIO, it will not be reported back to the user > > ret = btrfs_end_transaction(trans, root); > > should fix it. Thank you, I'll send out a revised version after a little while. -Jeff > >> + >> +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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-12-20 8:19 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-18 3:05 [RFC PATCH V6 0/2] Btrfs: get/set label of a mounted file system Jeff Liu 2012-12-18 3:05 ` [RFC PATCH V6 2/1] Btrfs: Add a new ioctl to get the " Jeff Liu 2012-12-18 3:06 ` [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change " Jeff Liu 2012-12-18 3:31 ` Miao Xie 2012-12-18 18:00 ` Goffredo Baroncelli 2012-12-19 3:42 ` Jeff Liu 2012-12-19 14:21 ` David Sterba 2012-12-20 8:18 ` Jeff Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).