Messages in this thread Patch in this message | | | Date | Fri, 10 Apr 2015 08:39:53 +0800 | From | Qu Wenruo <> | Subject | Re: [PATCH v2 4/6] Btrfs: fail on mismatched subvol and subvolid mount options |
| |
-------- Original Message -------- Subject: [PATCH v2 4/6] Btrfs: fail on mismatched subvol and subvolid mount options From: Omar Sandoval <osandov@osandov.com> To: Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>, David Sterba <dsterba@suse.cz>, <linux-btrfs@vger.kernel.org> Date: 2015年04月10日 05:34
> There's nothing to stop a user from passing both subvol= and subvolid= > to mount, but if they don't refer to the same subvolume, someone is > going to be surprised at some point. Error out on this case, but allow > users to pass in both if they do match (which they could, for example, > get out of /proc/mounts). Not sure should we do this extra check, as later mount options override previous mount option.
I previous tried to do such thing for mount option like inode/noinode, but was rejected for that reason.
So not sure such error-out behavior is OK or not. Maybe only taking the latest subvol/subvolid is a better choice?
Thanks, Qu > > Signed-off-by: Omar Sandoval <osandov@osandov.com> > --- > fs/btrfs/super.c | 32 ++++++++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index ab100e5..20b470d 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -1163,8 +1163,9 @@ static char *setup_root_args(char *args) > return buf; > } > > -static struct dentry *mount_subvol(const char *subvol_name, int flags, > - const char *device_name, char *data) > +static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid, > + int flags, const char *device_name, > + char *data) > { > struct dentry *root; > struct vfsmount *mnt = NULL; > @@ -1210,12 +1211,26 @@ static struct dentry *mount_subvol(const char *subvol_name, int flags, > /* mount_subtree() drops our reference on the vfsmount. */ > mnt = NULL; > > - if (!IS_ERR(root) && !is_subvolume_inode(root->d_inode)) { > + if (!IS_ERR(root)) { > struct super_block *s = root->d_sb; > - dput(root); > - root = ERR_PTR(-EINVAL); > - deactivate_locked_super(s); > - pr_err("BTRFS: '%s' is not a valid subvolume\n", subvol_name); > + u64 root_objectid = BTRFS_I(root->d_inode)->root->root_key.objectid; > + > + ret = 0; > + if (!is_subvolume_inode(root->d_inode)) { > + pr_err("BTRFS: '%s' is not a valid subvolume\n", > + subvol_name); > + ret = -EINVAL; > + } > + if (subvol_objectid && root_objectid != subvol_objectid) { > + pr_err("BTRFS: subvol '%s' does not match subvolid %llu\n", > + subvol_name, subvol_objectid); > + ret = -EINVAL; > + } > + if (ret) { > + dput(root); > + root = ERR_PTR(ret); > + deactivate_locked_super(s); > + } > } > > out: > @@ -1308,7 +1323,8 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, > > if (subvol_name) { > /* mount_subvol() will free subvol_name. */ > - return mount_subvol(subvol_name, flags, device_name, data); > + return mount_subvol(subvol_name, subvol_objectid, flags, > + device_name, data); > } > > security_init_mnt_opts(&new_sec_opts); >
| |