Message ID | 50202FA9.2070204@canonical.com |
---|---|
State | New |
Headers | show |
On 2012-08-06 14:57:13, Tim Gardner wrote: > On 08/06/2012 02:50 PM, Tim Gardner wrote: > > Tyler - as far as I can tell the super block flags get initialized > > outside of the ecryptfs code. Is this patch sufficient for 2.6.32.y ? Yeah, I see where they're getting initialized in get_sb_nodev(), which is called just before ecryptfs_read_super(). I do have a simple testcase for this bug in ecryptfs-utils. Let me know if you want instructions on how to build and run the test. Tyler > > > > rtg > > > > Doh! see attached. > > -- > Tim Gardner tim.gardner@canonical.com > From e573cb89603f26bfcdab92d510eb92a45d0d75b0 Mon Sep 17 00:00:00 2001 > From: Tyler Hicks <tyhicks@canonical.com> > Date: Mon, 11 Jun 2012 15:42:32 -0700 > Subject: [PATCH] eCryptfs: Copy up POSIX ACL and read-only flags from lower > mount > > BugLink: http://bugs.launchpad.net/bugs/1009207 > > 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); > -- > 1.7.9.5 >
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 08/06/2012 04:40 PM, Tyler Hicks wrote: > On 2012-08-06 14:57:13, Tim Gardner wrote: >> On 08/06/2012 02:50 PM, Tim Gardner wrote: >>> Tyler - as far as I can tell the super block flags get >>> initialized outside of the ecryptfs code. Is this patch >>> sufficient for 2.6.32.y ? > > Yeah, I see where they're getting initialized in get_sb_nodev(), > which is called just before ecryptfs_read_super(). > > I do have a simple testcase for this bug in ecryptfs-utils. Let me > know if you want instructions on how to build and run the test. > I've assigned the bug to Colin so I'm sure he'll figure it out. rtg - -- Tim Gardner tim.gardner@canonical.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQIcBAEBCgAGBQJQISKyAAoJED12yEX6FEfKqSAP/2S9Bbq+SEdHEc59SpIj6nk/ jT8hvbznBmz2eqFP7hhIBjBD37wo3AeubeQBU8Y4lblu/SUWWu3wn0ATBMzoZ0rl +YADvpQ+cy4ke08aQn+ljEhQ9kBZmMn1iklwo1wYR/jW4b0aWI2drC0C6cq6knu0 +j2DtzTRIPdFJ9gnOkgVptoZD+B1XYbeKQ0JPuy4VusFQo04z11p4X5E6r+2kV6W pGCpjAvmxEWEr//zje0f2CXjc0085spM5Ktyd2XS6OMiaHYkOuI0oEfrpIx4AE3X M3P0W+M4IbZhYu3bF4zZWN+fZc9otqdLC3ijHNcdVtsbV9j5DO0NiECTFoe54j8L rXt5srLlnRITWH0dFKx3xDtgklyh6FkoKlJWrlSRGNNhp7kuoeOidlrGOiHA78dQ 4uiYeQNj5HAX5SXXWmqlG37ifzPXFZcUAieVjxju+IxjTskvQO48EuRFO8BBDzCr RlVTPtRfD3ZCAhpEGwID0BkasRsK3Wz8BeOMC25NeevYEMvVkJ1FmGwRpG3WW9nR 6RCCUa276mNDmbtxVfa4+K5douJeJlGXJhMyqgLnCmSvTmjdmUo8SFRlS+wYXucA 22EzpwhpEWuzzWBIRRoAHuZdJszso5ArB0OmlfDpiA88/n+DzP8ITBGGefkV5iB6 InOx5VtVaVDNXXIZkyt0 =j4F/ -----END PGP SIGNATURE-----
On 2012-08-07 08:14:19, Tim Gardner wrote: > On 08/06/2012 04:40 PM, Tyler Hicks wrote: > > On 2012-08-06 14:57:13, Tim Gardner wrote: > >> On 08/06/2012 02:50 PM, Tim Gardner wrote: > >>> Tyler - as far as I can tell the super block flags get > >>> initialized outside of the ecryptfs code. Is this patch > >>> sufficient for 2.6.32.y ? > > > > Yeah, I see where they're getting initialized in get_sb_nodev(), > > which is called just before ecryptfs_read_super(). > > > > I do have a simple testcase for this bug in ecryptfs-utils. Let me > > know if you want instructions on how to build and run the test. > > > > I've assigned the bug to Colin so I'm sure he'll figure it out. Ah, good. He's very familiar with it. Tyler > > rtg > -- > Tim Gardner tim.gardner@canonical.com
From e573cb89603f26bfcdab92d510eb92a45d0d75b0 Mon Sep 17 00:00:00 2001 From: Tyler Hicks <tyhicks@canonical.com> Date: Mon, 11 Jun 2012 15:42:32 -0700 Subject: [PATCH] eCryptfs: Copy up POSIX ACL and read-only flags from lower mount BugLink: http://bugs.launchpad.net/bugs/1009207 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); -- 1.7.9.5