On 12/17/2012 02:30 PM, Jeff Liu wrote:
> On 12/17/2012 07:57 PM, Miao Xie wrote:
>> On mon, 17 Dec 2012 19:22:11 +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@xxxxxxxxxx>
>>> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
>>> Cc: Miao Xie <miaox@xxxxxxxxxxxxxx>
[...]
>>> +
>>> + if (strlen(label) > BTRFS_LABEL_SIZE - 1)
>>> + return -EINVAL;
>>
>> I think we should use strnlen()
> AFAICS, strnlen() is better only if the caller need to get the length of
> a length-limited string and make use of it proceeding, which means that
> the procedure would not return an error even if the length is beyond the
> limit. Or if the caller need to examine if a length-limited string is
> nul-terminated or not in a manner below,
> if (strnlen(buf, MAX_BUF_SIZE) == MAX_BUF_SIZE) {
> ....
> }
>
> I don't think it really needed here since the logic is clear with
> strlen(), or Am I miss anything?
I think that Miao fears strlen() searching a zero could go beyond the
page limit touching an un-mapped page and raising an segmentation fault....
I think that we should change the code as
+ label[BTRFS_LABEL_SIZE - 1] = 0;
+
+ if (strlen(label) > BTRFS_LABEL_SIZE - 1)
+ return -EINVAL;
My 2¢
Ciao
G.Baroncelli
>
>
> Thanks,
> -Jeff
>
>> Thanks
>> Miao
>>
>>> +
>>> + 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;
>>> + }
>>> +
>>> + label[BTRFS_LABEL_SIZE - 1] = '\0';
>>> + 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 +3850,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
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
