Skip to content

Commit

Permalink
ksmbd: defer notify_change() call
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Christian Brauner authored and Steve French committed Sep 4, 2021
1 parent db7fb6f commit 28a5d3d
Showing 1 changed file with 16 additions and 7 deletions.
23 changes: 16 additions & 7 deletions fs/ksmbd/smbacl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 28a5d3d

Please sign in to comment.