Skip to content

Commit f56535d

Browse files
dhkts1gregkh
authored andcommitted
ksmbd: add a WRITE_DAC/WRITE_OWNER check to SMB2 SET_INFO SECURITY
commit 44df157 upstream. commit cc57232 ("ksmbd: fix FSCTL permission bypass by adding a permission check for FSCTL_SET_SPARSE") added a fp->daccess gate to fsctl_set_sparse and noted that "similar handle-level checks exist in other functions but are missing here." The SMB2 SET_INFO SECURITY arm is one of the missing ones, and the most security-relevant: smb2_set_info_sec() calls set_info_sec() with no per-handle access check. set_info_sec() (fs/smb/server/smbacl.c) re-permissions the file: it rewrites owner/group/mode via notify_change(), rewrites the POSIX ACL via set_posix_acl(), and on KSMBD_SHARE_FLAG_ACL_XATTR shares removes and rewrites the Windows security descriptor via ksmbd_vfs_set_sd_xattr(). Every other persistent-mutation arm of the sibling handler smb2_set_info_file() checks fp->daccess first (FILE_WRITE_DATA / FILE_DELETE / FILE_WRITE_EA / FILE_WRITE_ATTRIBUTES); the SECURITY arm — which mutates the access control itself — is the only one with no gate. A client can therefore open a handle with FILE_WRITE_ATTRIBUTES only (no FILE_WRITE_DAC / FILE_WRITE_OWNER) and use SMB2_SET_INFO with InfoType SMB2_O_INFO_SECURITY to rewrite the file's DACL and owner, granting itself access the handle's daccess never carried. Unlike the FSCTL data arms this is a metadata/xattr operation, so there is no FMODE_WRITE VFS backstop — the missing fp->daccess check is the entire gate. Setting a security descriptor is the WRITE_DAC / WRITE_OWNER operation, so require at least one of those on the handle before re-permissioning the file. -EACCES is mapped to STATUS_ACCESS_DENIED by smb2_set_info(). Cc: stable@vger.kernel.org Signed-off-by: Gil Portnoy <dddhkts1@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent db231af commit f56535d

1 file changed

Lines changed: 3 additions & 0 deletions

File tree

fs/smb/server/smb2pdu.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6623,6 +6623,9 @@ static int smb2_set_info_sec(struct ksmbd_file *fp, int addition_info,
66236623

66246624
fp->saccess |= FILE_SHARE_DELETE_LE;
66256625

6626+
if (!(fp->daccess & (FILE_WRITE_DAC_LE | FILE_WRITE_OWNER_LE)))
6627+
return -EACCES;
6628+
66266629
return set_info_sec(fp->conn, fp->tcon, &fp->filp->f_path, pntsd,
66276630
buf_len, false, true);
66286631
}

0 commit comments

Comments
 (0)