Re: [PATCH] ecryptfs: disallow ecryptfs as underlying filesystem

[Pekka Enberg]

On Sat, Apr 24, 2010 at 5:41 AM, Eric Sandeen <sandeen@xxxxxxxxxx> wrote:
> mounting stacked ecryptfs on ecryptfs has been shown to lead to bugs
> in testing.  For crypto info in xattr, there is no mechanism for handling
> this at all, and for normal file headers, we run into other trouble:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> IP: [<ffffffffa015b0b3>] ecryptfs_d_revalidate+0x43/0xa0 [ecryptfs]
> ...
>
> There doesn't seem to be any good usecase for this, so I'd suggest just
> disallowing the configuration.

Maybe there's no good use case for it but it sure sounds like a good
test case for shaking out bugs in filesystem stacking code.

> Based on a patch originally, I believe, from Mike Halcrow.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index af1a8f0..7ada044 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -594,28 +594,46 @@ static int ecryptfs_get_sb(struct file_system_type *fs_type, int flags,
>                        struct vfsmount *mnt)
>  {
>        int rc;
> -       struct super_block *sb;
> +       struct super_block *sb, *lower_sb;
> +       struct nameidata nd;
> +
> +       rc = path_lookup(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &nd);
> +       if (rc) {
> +               printk(KERN_WARNING
> +                       "path_lookup() failed on dev_name = [%s]\n", dev_name);
> +               goto out;
> +       }
> +       lower_sb = nd.path.dentry->d_sb;
> +       if (strcmp(lower_sb->s_type->name, "ecryptfs") == 0) {
> +               rc = -EINVAL;
> +               printk(KERN_ERR "Mount on filesystem of type "
> +                       "eCryptfs explicitly disallowed due to "
> +                       "known incompatibilities\n");
> +               goto out_pathput;
> +       }
>
>        rc = get_sb_nodev(fs_type, flags, raw_data, ecryptfs_fill_super, mnt);
>        if (rc < 0) {
>                printk(KERN_ERR "Getting sb failed; rc = [%d]\n", rc);
> -               goto out;
> +               goto out_pathput;
>        }
>        sb = mnt->mnt_sb;
>        rc = ecryptfs_parse_options(sb, raw_data);
>        if (rc) {
>                printk(KERN_ERR "Error parsing options; rc = [%d]\n", rc);
> -               goto out_abort;
> +               goto out_dput;
>        }
>        rc = ecryptfs_read_super(sb, dev_name);
>        if (rc) {
>                printk(KERN_ERR "Reading sb failed; rc = [%d]\n", rc);
> -               goto out_abort;
> +               goto out_dput;
>        }
>        goto out;
> -out_abort:
> +out_dput:
>        dput(sb->s_root); /* aka mnt->mnt_root, as set by get_sb_nodev() */
>        deactivate_locked_super(sb);
> +out_pathput:
> +       path_put(&nd.path);
>  out:
>        return rc;
>  }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/