[PATCH] f2fs: preserve i_mode if __f2fs_set_acl() fails

[PATCH] f2fs: preserve i_mode if __f2fs_set_acl() fails

[Ernesto A. Fernández]

When changing a file's acl mask, __f2fs_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>
---
This bug is covered by generic/449 in xfstests, though that test may not run
for f2fs unless you change the size in the call to _scratch_mkfs_sized.

Several filesystems are affected; some of them have already applied patches,
see for example: "fe26569 ext2: preserve i_mode if ext2_set_acl() fails".

 fs/f2fs/acl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
index b4b8438..436b3a1 100644
--- a/fs/f2fs/acl.c
+++ b/fs/f2fs/acl.c
@@ -207,15 +207,16 @@ static int __f2fs_set_acl(struct inode *inode, int type,
 	void *value = NULL;
 	size_t size = 0;
 	int error;
+	umode_t mode = inode->i_mode;
 
 	switch (type) {
 	case ACL_TYPE_ACCESS:
 		name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
 		if (acl && !ipage) {
-			error = posix_acl_update_mode(inode, &inode->i_mode, &acl);
+			error = posix_acl_update_mode(inode, &mode, &acl);
 			if (error)
 				return error;
-			set_acl_inode(inode, inode->i_mode);
+			set_acl_inode(inode, mode);
 		}
 		break;
 
-- 
2.1.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH] f2fs: preserve i_mode if __f2fs_set_acl() fails

[Chao Yu]

On 2017/7/24 9:32, Ernesto A. Fernández wrote:
> When changing a file's acl mask, __f2fs_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>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> ---
> This bug is covered by generic/449 in xfstests, though that test may not run
> for f2fs unless you change the size in the call to _scratch_mkfs_sized.
> 
> Several filesystems are affected; some of them have already applied patches,
> see for example: "fe26569 ext2: preserve i_mode if ext2_set_acl() fails".
> 
>  fs/f2fs/acl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index b4b8438..436b3a1 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -207,15 +207,16 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>  	void *value = NULL;
>  	size_t size = 0;
>  	int error;
> +	umode_t mode = inode->i_mode;
>  
>  	switch (type) {
>  	case ACL_TYPE_ACCESS:
>  		name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
>  		if (acl && !ipage) {
> -			error = posix_acl_update_mode(inode, &inode->i_mode, &acl);
> +			error = posix_acl_update_mode(inode, &mode, &acl);
>  			if (error)
>  				return error;
> -			set_acl_inode(inode, inode->i_mode);
> +			set_acl_inode(inode, mode);
>  		}
>  		break;
>  
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel