lkml.org 
[lkml]   [2015]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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);
>


\
 
 \ /
  Last update: 2015-04-10 03:21    [W:0.155 / U:0.496 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site