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

Sedat Dilek
2012-08-16T08:30:23Z

On Thu, Aug 16, 2012 at 10:02 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Wed, Aug 15, 2012 at 5:48 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> From: Miklos Szeredi <mszeredi@suse.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
>

[ Removing <hramrach@centrum.cz> (Email-address seems to be dead) ]

Just for the sake of completeness and some notes:

Jordi dropped your "fs-stack-depth" patch completely in his tests.
AFAICS he uses OverlayFS for his debian-based own distro.
I put some references to his OverlayFS work in [1] and [2].

Pesonally, I have tested against Debian/sid.
With Ubuntu/precise I got a rebel (just wanting a stable development
environment than wasting my time with fixing unstable packages - as
there is Debian/wheezy freeze sid (unstable) branch/distribution
cannot be called a "working environment") and did my recent testings
on this platform.

- Sedat -

[1] http://livenet.selfip.com/ftp/debian/overlayfs/
[2] http://livenet.selfip.com/ftp/debian/overlayfs/test/

>> Signed-off-by: Miklos Szeredi <mszeredi@suse.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
>>