From 28a5d3de9d65058f7edf8e5aaaf6b0a8d8f4a29f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 26 Aug 2021 17:05:17 +0900 Subject: [PATCH] ksmbd: defer notify_change() call When ownership is changed we might in certain scenarios loose the ability to alter the inode after we changed ownership. This can e.g. happen when we are on an idmapped mount where uid 0 is mapped to uid 1000 and uid 1000 is mapped to uid 0. A caller with fs*id 1000 will be able to create files as *id 1000 on disk. They will also be able to change ownership of files owned by *id 0 to *id 1000 but they won't be able to change ownership in the other direction. This means acl operations following notify_change() would fail. Move the notify_change() call after the acls have been updated. This guarantees that we don't end up with spurious "hash value diff" warnings later on because we managed to change ownership but didn't manage to alter acls. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index ef5896297607..16da99a9963c 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -1334,22 +1334,31 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon, newattrs.ia_valid |= ATTR_MODE; newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777); - inode_lock(inode); - rc = notify_change(user_ns, path->dentry, &newattrs, NULL); - inode_unlock(inode); - if (rc) - goto out; - ksmbd_vfs_remove_acl_xattrs(user_ns, path->dentry); /* Update posix acls */ if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && fattr.cf_dacls) { rc = set_posix_acl(user_ns, inode, ACL_TYPE_ACCESS, fattr.cf_acls); - if (S_ISDIR(inode->i_mode) && fattr.cf_dacls) + if (rc < 0) + ksmbd_debug(SMB, + "Set posix acl(ACL_TYPE_ACCESS) failed, rc : %d\n", + rc); + if (S_ISDIR(inode->i_mode) && fattr.cf_dacls) { rc = set_posix_acl(user_ns, inode, ACL_TYPE_DEFAULT, fattr.cf_dacls); + if (rc) + ksmbd_debug(SMB, + "Set posix acl(ACL_TYPE_DEFAULT) failed, rc : %d\n", + rc); + } } + inode_lock(inode); + rc = notify_change(user_ns, path->dentry, &newattrs, NULL); + inode_unlock(inode); + if (rc) + goto out; + /* Check it only calling from SD BUFFER context */ if (type_check && !(le16_to_cpu(pntsd->type) & DACL_PRESENT)) goto out; -- 2.39.5