Patchwork [Lucid,SRU] eCryptfs: Copy up POSIX ACL and read-only flags from lower mount

login
register
mail settings
Submitter Tim Gardner
Date Aug. 7, 2012, 2:15 p.m.
Message ID <1344348912-49930-1-git-send-email-tim.gardner@canonical.com>
Download mbox | patch
Permalink /patch/175643/
State New
Headers show

Comments

Tim Gardner - Aug. 7, 2012, 2:15 p.m.
From: Tyler Hicks <tyhicks@canonical.com>

When the eCryptfs mount options do not include '-o acl', but the lower
filesystem's mount options do include 'acl', the MS_POSIXACL flag is not
flipped on in the eCryptfs super block flags. This flag is what the VFS
checks in do_last() when deciding if the current umask should be applied
to a newly created inode's mode or not. When a default POSIX ACL mask is
set on a directory, the current umask is incorrectly applied to new
inodes created in the directory. This patch ignores the MS_POSIXACL flag
passed into ecryptfs_mount() and sets the flag on the eCryptfs super
block depending on the flag's presence on the lower super block.

Additionally, it is incorrect to allow a writeable eCryptfs mount on top
of a read-only lower mount. This missing check did not allow writes to
the read-only lower mount because permissions checks are still performed
on the lower filesystem's objects but it is best to simply not allow a
rw mount on top of ro mount. However, a ro eCryptfs mount on top of a rw
mount is valid and still allowed.

https://launchpad.net/bugs/1009207

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Stefan Beller <stefanbeller@googlemail.com>
Cc: John Johansen <john.johansen@canonical.com>
(back ported from commit 069ddcda37b2cf5bb4b6031a944c0e9359213262)
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 fs/ecryptfs/main.c |    9 +++++++++
 1 file changed, 9 insertions(+)
Stefan Bader - Aug. 7, 2012, 3:51 p.m.
This one feels uncomfortable to ack. Minor nitpick the BugLink keyword is
missing. It does overall look similar to the upstream patch but applies to a
completely different function. I suppose it must be ok if Tyler did the backport
and hope testcases will prove its correctness...

On 07.08.2012 16:15, Tim Gardner wrote:
> From: Tyler Hicks <tyhicks@canonical.com>
> 
> When the eCryptfs mount options do not include '-o acl', but the lower
> filesystem's mount options do include 'acl', the MS_POSIXACL flag is not
> flipped on in the eCryptfs super block flags. This flag is what the VFS
> checks in do_last() when deciding if the current umask should be applied
> to a newly created inode's mode or not. When a default POSIX ACL mask is
> set on a directory, the current umask is incorrectly applied to new
> inodes created in the directory. This patch ignores the MS_POSIXACL flag
> passed into ecryptfs_mount() and sets the flag on the eCryptfs super
> block depending on the flag's presence on the lower super block.
> 
> Additionally, it is incorrect to allow a writeable eCryptfs mount on top
> of a read-only lower mount. This missing check did not allow writes to
> the read-only lower mount because permissions checks are still performed
> on the lower filesystem's objects but it is best to simply not allow a
> rw mount on top of ro mount. However, a ro eCryptfs mount on top of a rw
> mount is valid and still allowed.
> 
> https://launchpad.net/bugs/1009207
> 
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> Reported-by: Stefan Beller <stefanbeller@googlemail.com>
> Cc: John Johansen <john.johansen@canonical.com>
> (back ported from commit 069ddcda37b2cf5bb4b6031a944c0e9359213262)
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  fs/ecryptfs/main.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index e2f18ad..783f7fc 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -589,6 +589,15 @@ static int ecryptfs_read_super(struct super_block *sb, const char *dev_name,
>  	}
>  
>  	ecryptfs_set_superblock_lower(sb, path.dentry->d_sb);
> +
> +	/**
> +	 * Set the POSIX ACL flag based on whether they're enabled in the lower
> +	 * mount. Force a read-only eCryptfs mount if the lower mount is ro.
> +	 * Allow a ro eCryptfs mount even when the lower mount is rw.
> +	 */
> +	sb->s_flags &= ~MS_POSIXACL;
> +	sb->s_flags |= path.dentry->d_sb->s_flags & (MS_RDONLY | MS_POSIXACL);
> +
>  	sb->s_maxbytes = path.dentry->d_sb->s_maxbytes;
>  	sb->s_blocksize = path.dentry->d_sb->s_blocksize;
>  	ecryptfs_set_dentry_lower(sb->s_root, path.dentry);
>
Herton Ronaldo Krzesinski - Aug. 7, 2012, 11:33 p.m.

Tim Gardner - Aug. 8, 2012, 12:09 p.m.

Patch

diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index e2f18ad..783f7fc 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -589,6 +589,15 @@  static int ecryptfs_read_super(struct super_block *sb, const char *dev_name,
 	}
 
 	ecryptfs_set_superblock_lower(sb, path.dentry->d_sb);
+
+	/**
+	 * Set the POSIX ACL flag based on whether they're enabled in the lower
+	 * mount. Force a read-only eCryptfs mount if the lower mount is ro.
+	 * Allow a ro eCryptfs mount even when the lower mount is rw.
+	 */
+	sb->s_flags &= ~MS_POSIXACL;
+	sb->s_flags |= path.dentry->d_sb->s_flags & (MS_RDONLY | MS_POSIXACL);
+
 	sb->s_maxbytes = path.dentry->d_sb->s_maxbytes;
 	sb->s_blocksize = path.dentry->d_sb->s_blocksize;
 	ecryptfs_set_dentry_lower(sb->s_root, path.dentry);