Patchwork [SRU,Natty/Oneiric/Precise] eCryptfs: Copy up POSIX ACL and read-only flags from lower mount

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

Comments

Tim Gardner - Aug. 6, 2012, 7:04 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>
(cherry picked from commit 069ddcda37b2cf5bb4b6031a944c0e9359213262)
---
 fs/ecryptfs/main.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
Herton Ronaldo Krzesinski - Aug. 6, 2012, 7:24 p.m.

Colin King - Aug. 6, 2012, 7:47 p.m.
On 06/08/12 20:04, 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>
> (cherry picked from commit 069ddcda37b2cf5bb4b6031a944c0e9359213262)
> ---
>   fs/ecryptfs/main.c |   10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index 703cda3..9e3d2ee 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -505,7 +505,6 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
>   		goto out;
>   	}
>
> -	s->s_flags = flags;
>   	rc = bdi_setup_and_register(&sbi->bdi, "ecryptfs", BDI_CAP_MAP_COPY);
>   	if (rc)
>   		goto out1;
> @@ -541,6 +540,15 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
>   	}
>
>   	ecryptfs_set_superblock_lower(s, 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.
> +	 */
> +	s->s_flags = flags & ~MS_POSIXACL;
> +	s->s_flags |= path.dentry->d_sb->s_flags & (MS_RDONLY | MS_POSIXACL);
> +
>   	s->s_maxbytes = path.dentry->d_sb->s_maxbytes;
>   	s->s_blocksize = path.dentry->d_sb->s_blocksize;
>   	s->s_magic = ECRYPTFS_SUPER_MAGIC;
>
John Johansen - Aug. 6, 2012, 11:43 p.m.
On 08/06/2012 12:04 PM, 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

looks good to me

Ackedy-by: John Johansen <john.johansen@canonical.com>
Tim Gardner - Aug. 7, 2012, 1:48 p.m.

Patch

diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 703cda3..9e3d2ee 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -505,7 +505,6 @@  static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 		goto out;
 	}
 
-	s->s_flags = flags;
 	rc = bdi_setup_and_register(&sbi->bdi, "ecryptfs", BDI_CAP_MAP_COPY);
 	if (rc)
 		goto out1;
@@ -541,6 +540,15 @@  static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	}
 
 	ecryptfs_set_superblock_lower(s, 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.
+	 */
+	s->s_flags = flags & ~MS_POSIXACL;
+	s->s_flags |= path.dentry->d_sb->s_flags & (MS_RDONLY | MS_POSIXACL);
+
 	s->s_maxbytes = path.dentry->d_sb->s_maxbytes;
 	s->s_blocksize = path.dentry->d_sb->s_blocksize;
 	s->s_magic = ECRYPTFS_SUPER_MAGIC;