linux-kernel - Re: [PATCH 08/13] fs: limit filesystem stacking depth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+icZUX32x5qiyxhAakxvo0Y4ypEA=DiurP5WCxbYmFi07+O4A@mail.gmail.com>
Date:	Thu, 16 Aug 2012 10:02:11 +0200
From:	Sedat Dilek <sedat.dilek@...il.com>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, hch@...radead.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	apw@...onical.com, nbd@...nwrt.org, neilb@...e.de,
	hramrach@...trum.cz, jordipujolp@...il.com, ezk@....cs.sunysb.edu,
	ricwheeler@...il.com, dhowells@...hat.com, hpj@...la.net,
	sedat.dilek@...glemail.com, penberg@...nel.org,
	goran.cetusic@...il.com, romain@...bokech.com, mszeredi@...e.cz
Subject: Re: [PATCH 08/13] fs: limit filesystem stacking depth

On Wed, Aug 15, 2012 at 5:48 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
> From: Miklos Szeredi <mszeredi@...e.cz>
>
> Add a simple read-only counter to super_block that indicates deep this
> is in the stack of filesystems.  Previously ecryptfs was the only
> stackable filesystem and it explicitly disallowed multiple layers of
> itself.
>
> Overlayfs, however, can be stacked recursively and also may be stacked
> on top of ecryptfs or vice versa.
>
> To limit the kernel stack usage we must limit the depth of the
> filesystem stack.  Initially the limit is set to 2.
>

Hi,

I have tested OverlayFS for a long time with "fs-stack-depth=3".
The original OverlayFS test-case script  from Jordi was slightly
modified (see "testcase-ovl-v3.sh").
I have sent my test-results to Andy and Jordi (tested with the
patchset from Andy against Linux-v3.4 [1] with Ext4-FS).
The attached test-case script *requires* "fs-stack-depth=3" to run
properly (patch attached).

So, I have 2 questions:

[1] FS-stack-limitation

Is a "fs-stack-depth>=2" (like "3") critical?
Is your setting to "2" just a defensive (and initial) one?
Can you explain your choice a bit more as ecryptFS is involved in this
limitation, too.

[2] Test-Case/Use-Case scripts

It would be *very very very* helpful if you could provide or even ship
in the Linux-kernel a test-case/use-case script, Thanks!
Maybe describe "Documentation/filesystems/overlayfs.txt" would be a good place?
Helpful could be a "simple" and a "complex" testing scenario?

- Sedat -


[1] http://git.kernel.org/?p=linux/kernel/git/apw/overlayfs.git;a=shortlog;h=refs/heads/overlayfs.v12apw1

> Signed-off-by: Miklos Szeredi <mszeredi@...e.cz>
> ---
>  fs/ecryptfs/main.c   |    7 +++++++
>  fs/overlayfs/super.c |   10 ++++++++++
>  include/linux/fs.h   |   11 +++++++++++
>  3 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index 2768138..344fb2c 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -565,6 +565,13 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
>         s->s_maxbytes = path.dentry->d_sb->s_maxbytes;
>         s->s_blocksize = path.dentry->d_sb->s_blocksize;
>         s->s_magic = ECRYPTFS_SUPER_MAGIC;
> +       s->s_stack_depth = path.dentry->d_sb->s_stack_depth + 1;
> +
> +       rc = -EINVAL;
> +       if (s->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
> +               printk(KERN_ERR "eCryptfs: maximum fs stacking depth exceeded\n");
> +               goto out_free;
> +       }
>
>         inode = ecryptfs_get_inode(path.dentry->d_inode, s);
>         rc = PTR_ERR(inode);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 69a2099..64d2695 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -551,6 +551,16 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>             !S_ISDIR(lowerpath.dentry->d_inode->i_mode))
>                 goto out_put_lowerpath;
>
> +       sb->s_stack_depth = max(upperpath.mnt->mnt_sb->s_stack_depth,
> +                               lowerpath.mnt->mnt_sb->s_stack_depth) + 1;
> +
> +       err = -EINVAL;
> +       if (sb->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
> +               printk(KERN_ERR "overlayfs: maximum fs stacking depth exceeded\n");
> +               goto out_put_lowerpath;
> +       }
> +
> +
>         ufs->upper_mnt = clone_private_mount(&upperpath);
>         err = PTR_ERR(ufs->upper_mnt);
>         if (IS_ERR(ufs->upper_mnt)) {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index abc7a53..1265e24 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -505,6 +505,12 @@ struct iattr {
>   */
>  #include <linux/quota.h>
>
> +/*
> + * Maximum number of layers of fs stack.  Needs to be limited to
> + * prevent kernel stack overflow
> + */
> +#define FILESYSTEM_MAX_STACK_DEPTH 2
> +
>  /**
>   * enum positive_aop_returns - aop return codes with specific semantics
>   *
> @@ -1579,6 +1585,11 @@ struct super_block {
>
>         /* Being remounted read-only */
>         int s_readonly_remount;
> +
> +       /*
> +        * Indicates how deep in a filesystem stack this SB is
> +        */
> +       int s_stack_depth;
>  };
>
>  /* superblock cache pruning functions */
> --
> 1.7.7
>

Download attachment "testcase-ovl-v3.sh" of type "application/x-sh" (4801 bytes)

Download attachment "0001-fs-Increase-limit-filesystem-stacking-depth-to-3.patch" of type "application/octet-stream" (796 bytes)