[PATCH 0/2] ext4: ACL fixes for 4.12

[PATCH 0/2] ext4: ACL fixes for 4.12

[Jan Kara]

Hello,

here are two ext4 ACL fixes with context fixed up so that they apply to
4.12 kernel. Please apply them to 4.12-stable. Thanks!

								Honza

[PATCH 1/2] ext4: preserve i_mode if __ext4_set_acl() fails

[Jan Kara]

From: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>

When changing a file's acl mask, __ext4_set_acl() will first set the group
bits of i_mode to the value of the mask, and only then set the actual
extended attribute representing the new acl.

If the second part fails (due to lack of space, for example) and the file
had no acl attribute to begin with, the system will from now on assume
that the mask permission bits are actual group permission bits, potentially
granting access to the wrong users.

Prevent this by only changing the inode mode after the acl has been set.

Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Jan Kara <jack@suse.cz>

---
 fs/ext4/acl.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -189,16 +189,17 @@ __ext4_set_acl(handle_t *handle, struct
 	void *value = NULL;
 	size_t size = 0;
 	int error;
+	int update_mode = 0;
+	umode_t mode = inode->i_mode;
 
 	switch (type) {
 	case ACL_TYPE_ACCESS:
 		name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;
 		if (acl) {
-			error = posix_acl_update_mode(inode, &inode->i_mode, &acl);
+			error = posix_acl_update_mode(inode, &mode, &acl);
 			if (error)
 				return error;
-			inode->i_ctime = current_time(inode);
-			ext4_mark_inode_dirty(handle, inode);
+			update_mode = 1;
 		}
 		break;
 
@@ -221,8 +222,14 @@ __ext4_set_acl(handle_t *handle, struct
 				      value, size, 0);
 
 	kfree(value);
-	if (!error)
+	if (!error) {
 		set_cached_acl(inode, type, acl);
+		if (update_mode) {
+			inode->i_mode = mode;
+			inode->i_ctime = current_time(inode);
+			ext4_mark_inode_dirty(handle, inode);
+		}
+	}
 
 	return error;
 }

[PATCH] ext4: Don't clear SGID when inheriting ACLs

[Jan Kara]

When new directory 'DIR1' is created in a directory 'DIR0' with SGID bit
set, DIR1 is expected to have SGID bit set (and owning group equal to
the owning group of 'DIR0'). However when 'DIR0' also has some default
ACLs that 'DIR1' inherits, setting these ACLs will result in SGID bit on
'DIR1' to get cleared if user is not member of the owning group.

Fix the problem by moving posix_acl_update_mode() out of
__ext4_set_acl() into ext4_set_acl(). That way the function will not be
called when inheriting ACLs which is what we want as it prevents SGID
bit clearing and the mode has been properly set by posix_acl_create()
anyway.

Fixes: 073931017b49d9458aa351605b43a7e34598caef
Cc: stable@vger.kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>

---
 fs/ext4/acl.c |   28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -189,18 +189,10 @@ __ext4_set_acl(handle_t *handle, struct
 	void *value = NULL;
 	size_t size = 0;
 	int error;
-	int update_mode = 0;
-	umode_t mode = inode->i_mode;
 
 	switch (type) {
 	case ACL_TYPE_ACCESS:
 		name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;
-		if (acl) {
-			error = posix_acl_update_mode(inode, &mode, &acl);
-			if (error)
-				return error;
-			update_mode = 1;
-		}
 		break;
 
 	case ACL_TYPE_DEFAULT:
@@ -224,11 +216,6 @@ __ext4_set_acl(handle_t *handle, struct
 	kfree(value);
 	if (!error) {
 		set_cached_acl(inode, type, acl);
-		if (update_mode) {
-			inode->i_mode = mode;
-			inode->i_ctime = current_time(inode);
-			ext4_mark_inode_dirty(handle, inode);
-		}
 	}
 
 	return error;
@@ -239,6 +226,8 @@ ext4_set_acl(struct inode *inode, struct
 {
 	handle_t *handle;
 	int error, retries = 0;
+	umode_t mode = inode->i_mode;
+	int update_mode = 0;
 
 	error = dquot_initialize(inode);
 	if (error)
@@ -249,7 +238,20 @@ retry:
 	if (IS_ERR(handle))
 		return PTR_ERR(handle);
 
+	if ((type == ACL_TYPE_ACCESS) && acl) {
+		error = posix_acl_update_mode(inode, &mode, &acl);
+		if (error)
+			goto out_stop;
+		update_mode = 1;
+	}
+
 	error = __ext4_set_acl(handle, inode, type, acl);
+	if (!error && update_mode) {
+		inode->i_mode = mode;
+		inode->i_ctime = current_time(inode);
+		ext4_mark_inode_dirty(handle, inode);
+	}
+out_stop:
 	ext4_journal_stop(handle);
 	if (error == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
 		goto retry;

Re: [PATCH 0/2] ext4: ACL fixes for 4.12

[Greg KH]

On Tue, Aug 08, 2017 at 02:44:40PM +0200, Jan Kara wrote:
> 
> Hello,
> 
> here are two ext4 ACL fixes with context fixed up so that they apply to
> 4.12 kernel. Please apply them to 4.12-stable. Thanks!
> 
> 								Honza

Providing the git commit ids of the patches would have been more
helpful.  Please do that in the future, I had to dig for them...

thanks,

greg k-h

Re: [PATCH 0/2] ext4: ACL fixes for 4.12

[Jan Kara]

On Tue 08-08-17 09:11:59, Greg KH wrote:
> On Tue, Aug 08, 2017 at 02:44:40PM +0200, Jan Kara wrote:
> > 
> > Hello,
> > 
> > here are two ext4 ACL fixes with context fixed up so that they apply to
> > 4.12 kernel. Please apply them to 4.12-stable. Thanks!
> > 
> > 								Honza
> 
> Providing the git commit ids of the patches would have been more
> helpful.  Please do that in the future, I had to dig for them...

Sorry, they were part of the patches as 'Git-commit' header which is
somewhat unusual I guess. I'll add them in some more standard format next
time.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] ext4: ACL fixes for 4.12

[Greg KH]

On Tue, Aug 08, 2017 at 06:27:42PM +0200, Jan Kara wrote:
> On Tue 08-08-17 09:11:59, Greg KH wrote:
> > On Tue, Aug 08, 2017 at 02:44:40PM +0200, Jan Kara wrote:
> > > 
> > > Hello,
> > > 
> > > here are two ext4 ACL fixes with context fixed up so that they apply to
> > > 4.12 kernel. Please apply them to 4.12-stable. Thanks!
> > > 
> > > 								Honza
> > 
> > Providing the git commit ids of the patches would have been more
> > helpful.  Please do that in the future, I had to dig for them...
> 
> Sorry, they were part of the patches as 'Git-commit' header which is
> somewhat unusual I guess. I'll add them in some more standard format next
> time.

Ah, I should have looked at the email header, didn't think about that,
my coffee hasn't kicked in yet.  Will do that next time.

thanks,

greg k-h

Re: [PATCH 0/2] ext4: ACL fixes for 4.12

[Theodore Ts'o]

Jan, thanks for taking care of the back port!

     	    	       	    - Ted