Skip to content

Commit 28a5d3d

Browse files
Christian BraunerSteve French
authored andcommitted
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 <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent db7fb6f commit 28a5d3d

File tree

1 file changed

+16
-7
lines changed

1 file changed

+16
-7
lines changed

fs/ksmbd/smbacl.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,22 +1334,31 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
13341334
newattrs.ia_valid |= ATTR_MODE;
13351335
newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777);
13361336

1337-
inode_lock(inode);
1338-
rc = notify_change(user_ns, path->dentry, &newattrs, NULL);
1339-
inode_unlock(inode);
1340-
if (rc)
1341-
goto out;
1342-
13431337
ksmbd_vfs_remove_acl_xattrs(user_ns, path->dentry);
13441338
/* Update posix acls */
13451339
if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && fattr.cf_dacls) {
13461340
rc = set_posix_acl(user_ns, inode,
13471341
ACL_TYPE_ACCESS, fattr.cf_acls);
1348-
if (S_ISDIR(inode->i_mode) && fattr.cf_dacls)
1342+
if (rc < 0)
1343+
ksmbd_debug(SMB,
1344+
"Set posix acl(ACL_TYPE_ACCESS) failed, rc : %d\n",
1345+
rc);
1346+
if (S_ISDIR(inode->i_mode) && fattr.cf_dacls) {
13491347
rc = set_posix_acl(user_ns, inode,
13501348
ACL_TYPE_DEFAULT, fattr.cf_dacls);
1349+
if (rc)
1350+
ksmbd_debug(SMB,
1351+
"Set posix acl(ACL_TYPE_DEFAULT) failed, rc : %d\n",
1352+
rc);
1353+
}
13511354
}
13521355

1356+
inode_lock(inode);
1357+
rc = notify_change(user_ns, path->dentry, &newattrs, NULL);
1358+
inode_unlock(inode);
1359+
if (rc)
1360+
goto out;
1361+
13531362
/* Check it only calling from SD BUFFER context */
13541363
if (type_check && !(le16_to_cpu(pntsd->type) & DACL_PRESENT))
13551364
goto out;

0 commit comments

Comments
 (0)