On 2020/12/14 11:54, Weichao Guo wrote:
We should update the ~S_IRWXUGO part of inode->i_mode in __setattr_copy,
because posix_acl_update_mode updates mode based on inode->i_mode,
which finally overwrites the ~S_IRWXUGO part of i_acl_mode with old i_mode.
Testcase to reproduce this bug:
0. adduser abc
1. mkfs.f2fs /dev/sdd
2. mount -t f2fs /dev/sdd /mnt/f2fs
3. mkdir /mnt/f2fs/test
4. setfacl -m u:abc:r /mnt/f2fs/test
5. chmod +s /mnt/f2fs/test
Signed-off-by: Weichao Guo <[email protected]>
Signed-off-by: Bin Shu <[email protected]>
---
fs/f2fs/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 16ea10f..4d355f9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -850,6 +850,7 @@ static void __setattr_copy(struct inode *inode, const
struct iattr *attr)
if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
mode &= ~S_ISGID;
+ inode->i_mode = (inode->i_mode & S_IRWXUGO) | (mode &
~S_IRWXUGO);
Sorry, I still have problem with this patch.
I think this equals to inode->i_mode = mode;
Because in chmod_common(), @mode was assigned as:
newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
and only S_ISUID and S_ISGID bits of newattrs.ia_mode can be changed during
chmod()
That's why setattr_copy() in fs/attr.c just uses "inode->i_mode = mode;"
set_acl_inode(inode, mode);
Another problem is if i_acl_mode is used for error path handling, here
i_acl_mode
and i_mode have the same value, that's not correct?
Jaegeuk,
IIUC, i_acl_mode was introduced for i_mode recovery once acl progress fails?
Thanks,
}
}
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
