From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp206.alice.it ([82.57.200.102]:59779 "EHLO smtp206.alice.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796Ab2LTUQr (ORCPT ); Thu, 20 Dec 2012 15:16:47 -0500 Message-ID: <50D372A5.4090906@inwind.it> Date: Thu, 20 Dec 2012 21:18:45 +0100 From: Goffredo Baroncelli Reply-To: kreijack@inwind.it MIME-Version: 1.0 To: Jeff Liu CC: linux-btrfs@vger.kernel.org, anand.jain@oracle.com, miaox@cn.fujitsu.com, dsterba@suse.cz Subject: Re: [RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system In-Reply-To: <50D2CF98.6010806@oracle.com> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-btrfs-owner@vger.kernel.org List-ID: References: <50D2CF98.6010806@oracle.com> HI Jeff, On 12/20/2012 09:43 AM, Jeff Liu wrote: > With the new ioctl(2) BTRFS_IOC_GET_FSLABEL we can fetch the label of a mounted file system. > > Signed-off-by: Jie Liu > Signed-off-by: Anand Jain > Cc: Miao Xie > Cc: Goffredo Baroncelli > Cc: David Sterba [...] > +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)); Sorry for pointing out my doubt too late, but should we trust super_copy->label ? An user could insert a usb-key with a btrfs filesystem with a label without zero. In this case strlen() could access outside super_copy->label[]. I think that it should be quite easy to alter artificially a filesystem to crash the kernel. So I not consider this as big problem. However *in case* of a further cycle of this patch I suggest to replace strlen() with strnlen(). > + 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 -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5