On 12/17/2012 02:30 PM, Jeff Liu wrote:

On 12/17/2012 07:57 PM, Miao Xie wrote:

quoted

On 	mon, 17 Dec 2012 19:22:11 +0800, Jeff Liu wrote:

quoted

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

Signed-off-by: Jie Liu <redacted>
Signed-off-by: Anand Jain <redacted>
Cc: Miao Xie <redacted>

[...]

quoted

quoted

+
+	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

quoted

Thanks
Miao

quoted

+
+	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@vger.kernel.org
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@vger.kernel.org
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