smb: client: fix uid/gid override in getattr with posix extensions
Subsystems (3)
Paulo Alcantara
maintainerPATCH v2 1/5 smb: client: fix uid/gid override in getattr with posix extensions
When mounting with 'multiuser,posix' options, cifs_getattr() overrides the server-provided uid/gid with the current process's fsuid/fsgid. This is because the condition only checks for unix extensions (tcon->unix_ext) but not posix extensions (tcon->posix_extensions).
With SMB3 POSIX extensions, the server provides real uid/gid values just like with unix extensions, so they should be preserved rather than replaced with the caller's credentials.
Add a tcon->posix_extensions check to the condition so that uid/gid from the server are properly reported in stat results.
--- fs/smb/client/inode.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/smb/client/inode.c +4 -4
@@ -2992,14 +2992,14 @@ int cifs_getattr(struct mnt_idmap *idmap, const struct path *path, stat->attributes |=STATX_ATTR_ENCRYPTED ;/* -* If on a multiuser mount without unix extensions or cifsacl being -* enabled, and the admin hasn't overridden them, set the ownership -* to the fsuid/fsgid of the current process. +* If on a multiuser mount without unix extensions, posix extensions +* or cifsacl being enabled, and the admin hasn't overridden them, +* set the ownership to the fsuid/fsgid of the current process. */ sbflags =cifs_sb_flags (cifs_sb);if ((sbflags &CIFS_MOUNT_MULTIUSER ) && !(sbflags &CIFS_MOUNT_CIFS_ACL ) && - !tcon->unix_ext) { + !tcon->unix_ext && !tcon->posix_extensions) {if (!(sbflags &CIFS_MOUNT_OVERR_UID )) stat->uid =current_fsuid ();if (!(sbflags &CIFS_MOUNT_OVERR_GID ))
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH v2 2/5 smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid
When the administrator mounts with forceuid or forcegid (uid=/gid= mount options), they expect all files to appear owned by the specified user/group. However, several code paths unconditionally called sid_to_id() to overwrite cf_uid/cf_gid with server-provided values, ignoring the administrator's explicit override:
- smb311_posix_info_to_fattr() (stat via POSIX extensions)
- cifs_posix_to_fattr() (readdir via POSIX extensions)
- parse_sec_desc() (CIFS ACL ownership mapping)
This allowed an untrusted server to dictate local file ownership even when the mount was configured to force specific uid/gid values.
Fix all three call sites to check CIFS_MOUNT_OVERR_UID and CIFS_MOUNT_OVERR_GID before calling sid_to_id(), following the same pattern already used by cifs_unix_basic_to_fattr() for unix extensions.
--- fs/smb/client/cifsacl.c | 29 ++++++++++++++++++----------- fs/smb/client/inode.c | 9 +++++++-- fs/smb/client/readdir.c | 9 +++++++-- 3 files changed, 32 insertions(+), 15 deletions(-)
fs/smb/client/cifsacl.c +18 -11
@@ -1346,6 +1346,7 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, {int rc =0 ;struct smb_sid *owner_sid_ptr, *group_sid_ptr; +unsigned int sbflags =cifs_sb_flags (cifs_sb);struct smb_acl *dacl_ptr;/* no need for SACL ptr */ char *end_of_acl;__u32 dacloffset, osidoffset, gsidoffset;
@@ -1364,17 +1365,21 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,cifs_dbg (NOISY ,"revision %d type 0x%x ooffset 0x%x goffset 0x%x sacloffset 0x%x dacloffset 0x%x\n" , pntsd->revision, pntsd->type, osidoffset, gsidoffset,le32_to_cpu (pntsd->sacloffset), dacloffset); -/* cifs_dump_mem("owner_sid: ", owner_sid_ptr, 64); */ + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; + rc =sid_from_sd (pntsd, acl_len, osidoffset, &owner_sid_ptr);if (rc) {cifs_dbg (FYI ,"%s: Error %d parsing Owner SID\n" , __func__, rc);return rc; } - rc =sid_to_id (cifs_sb, owner_sid_ptr, fattr,SIDOWNER ); -if (rc) { -cifs_dbg (FYI ,"%s: Error %d mapping Owner SID to uid\n" , - __func__, rc); -return rc; +if (!(sbflags &CIFS_MOUNT_OVERR_UID )) { + rc =sid_to_id (cifs_sb, owner_sid_ptr, fattr,SIDOWNER ); +if (rc) { +cifs_dbg (FYI ,"%s: Error %d mapping Owner SID to uid\n" , + __func__, rc); +return rc; + } } rc =sid_from_sd (pntsd, acl_len, gsidoffset, &group_sid_ptr);
@@ -1383,11 +1388,13 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, __func__, rc);return rc; } - rc =sid_to_id (cifs_sb, group_sid_ptr, fattr,SIDGROUP ); -if (rc) { -cifs_dbg (FYI ,"%s: Error %d mapping Group SID to gid\n" , - __func__, rc); -return rc; +if (!(sbflags &CIFS_MOUNT_OVERR_GID )) { + rc =sid_to_id (cifs_sb, group_sid_ptr, fattr,SIDGROUP ); +if (rc) { +cifs_dbg (FYI ,"%s: Error %d mapping Group SID to gid\n" , + __func__, rc); +return rc; + } }if (dacloffset) {
fs/smb/client/inode.c +7 -2
@@ -851,6 +851,7 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,struct smb311_posix_qinfo *info = &data->posix_fi;struct cifs_sb_info *cifs_sb =CIFS_SB (sb);struct cifs_tcon *tcon =cifs_sb_master_tcon (cifs_sb); +unsigned int sbflags =cifs_sb_flags (cifs_sb);memset (fattr,0 ,sizeof (*fattr));
@@ -895,8 +896,12 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr, fattr->cf_symlink_target = data->symlink_target; data->symlink_target =NULL ; } -sid_to_id (cifs_sb, &data->posix_owner, fattr,SIDOWNER ); -sid_to_id (cifs_sb, &data->posix_group, fattr,SIDGROUP ); + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; +if (!(sbflags &CIFS_MOUNT_OVERR_UID )) +sid_to_id (cifs_sb, &data->posix_owner, fattr,SIDOWNER ); +if (!(sbflags &CIFS_MOUNT_OVERR_GID )) +sid_to_id (cifs_sb, &data->posix_group, fattr,SIDGROUP );cifs_dbg (FYI ,"POSIX query info: mode 0x%x uniqueid 0x%llx nlink %d\n" , fattr->cf_mode, fattr->cf_uniqueid, fattr->cf_nlink);
fs/smb/client/readdir.c +7 -2
@@ -242,6 +242,7 @@ static void cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, struct cifs_sb_info *cifs_sb) { + unsigned int sbflags = cifs_sb_flags(cifs_sb);struct smb2_posix_info_parsed parsed;posix_info_parse (info,NULL , &parsed);
@@ -281,8 +282,12 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,le32_to_cpu (info->ReparseTag), le32_to_cpu(info->Mode)); -sid_to_id (cifs_sb, &parsed.owner, fattr,SIDOWNER ); -sid_to_id (cifs_sb, &parsed.group, fattr,SIDGROUP ); + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; +if (!(sbflags &CIFS_MOUNT_OVERR_UID )) +sid_to_id (cifs_sb, &parsed.owner, fattr,SIDOWNER ); +if (!(sbflags &CIFS_MOUNT_OVERR_GID )) +sid_to_id (cifs_sb, &parsed.group, fattr,SIDGROUP ); }static void __dir_info_to_fattr (struct cifs_fattr *fattr,const void *info)
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH v2 3/5 smb: client: fix WSL reparse point uid/gid override
In reply to Paulo Alcantarawsl_to_fattr() unconditionally overwrites cf_uid/cf_gid with values from WSL extended attributes ($LXUID/$LXGID), ignoring the administrator's forceuid/forcegid mount options. It also doesn't initialize the uid/gid defaults, so callers that invoke it before setting cf_uid/cf_gid (e.g. smb311_posix_info_to_fattr()) would leave zeroed values when forceuid/forcegid is set.
Additionally, when UNIX or SMB3 POSIX extensions are negotiated, the server already provides authoritative uid/gid and mode values through the extensions themselves, making the WSL reparse point EA values ($LXUID, $LXGID, $LXMOD) redundant and potentially conflicting.
Fix this by:
- Initializing cf_uid/cf_gid to the mount-specified linux_uid/ linux_gid defaults
- Gating the $LXUID/$LXGID EA parsing on CIFS_MOUNT_OVERR_UID/ CIFS_MOUNT_OVERR_GID
- Skipping $LXUID, $LXGID, and $LXMOD EA parsing entirely when UNIX or POSIX extensions are active (only $LXDEV is still parsed for device major/minor numbers)
--- fs/smb/client/reparse.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
fs/smb/client/reparse.c +25 -5
@@ -1137,10 +1137,15 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data,struct cifs_sb_info *cifs_sb, u32 tag, struct cifs_fattr *fattr) { + struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); +unsigned int sbflags =cifs_sb_flags (cifs_sb);struct smb2_file_full_ea_info *ea;bool have_xattr_dev = false;u32 next =0 ; + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; +switch (tag) {case IO_REPARSE_TAG_LX_SYMLINK : fattr->cf_mode |=S_IFLNK ;
@@ -1177,11 +1182,26 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data, nlen = ea->ea_name_length; v = (void *)((u8 *)ea->ea_data + ea->ea_name_length +1 ); -if (!strncmp (name,SMB2_WSL_XATTR_UID , nlen)) - fattr->cf_uid =wsl_make_kuid (cifs_sb, v); -else if (!strncmp (name,SMB2_WSL_XATTR_GID , nlen)) - fattr->cf_gid =wsl_make_kgid (cifs_sb, v); -else if (!strncmp (name,SMB2_WSL_XATTR_MODE , nlen)) { +/* +* Skip uid/gid/mode when UNIX or POSIX extensions are +* active since the server provides these values through +* the extensions themselves. +*/ +if (tcon->unix_ext || tcon->posix_extensions) { +if (!strncmp (name,SMB2_WSL_XATTR_DEV , nlen)) { + fattr->cf_rdev =reparse_mkdev (v); + have_xattr_dev = true; + } +continue ; + } + +if (!strncmp (name,SMB2_WSL_XATTR_UID , nlen)) { +if (!(sbflags &CIFS_MOUNT_OVERR_UID )) + fattr->cf_uid =wsl_make_kuid (cifs_sb, v); + }else if (!strncmp (name,SMB2_WSL_XATTR_GID , nlen)) { +if (!(sbflags &CIFS_MOUNT_OVERR_GID )) + fattr->cf_gid =wsl_make_kgid (cifs_sb, v); + }else if (!strncmp (name,SMB2_WSL_XATTR_MODE , nlen)) {/* File type in reparse point tag and in xattr mode must match. */ if (S_DT (fattr->cf_mode) !=S_DT (le32_to_cpu (*(__le32 *)v)))return false;
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH v2 4/5 smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr()
In reply to Paulo Alcantaracifs_posix_to_fattr() ignores the return value of posix_info_parse(). When a malformed POSIX directory entry is encountered (e.g. invalid SID lengths from an untrusted server), posix_info_parse() returns -1 without populating the 'parsed' struct. The uninitialized stack memory in parsed.owner and parsed.group is then passed to sid_to_id(), which processes the garbage bytes and passes them to request_key() to construct a SID string, potentially leaking kernel stack contents to the userspace idmap daemon.
Fix this by checking the return value and skipping the SID-to-id mapping when parsing fails. The remaining fattr fields (timestamps, mode, etc.) are populated directly from the 'info' pointer so they are unaffected.
--- fs/smb/client/readdir.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
fs/smb/client/readdir.c +11 -5
@@ -244,8 +244,9 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, {unsigned int sbflags =cifs_sb_flags (cifs_sb);struct smb2_posix_info_parsed parsed; +int rc; -posix_info_parse (info,NULL , &parsed); + rc =posix_info_parse (info,NULL , &parsed);memset (fattr,0 ,sizeof (*fattr)); fattr->cf_uniqueid =le64_to_cpu (info->Inode);
@@ -284,10 +285,15 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, fattr->cf_uid = cifs_sb->ctx->linux_uid; fattr->cf_gid = cifs_sb->ctx->linux_gid; -if (!(sbflags &CIFS_MOUNT_OVERR_UID )) -sid_to_id (cifs_sb, &parsed.owner, fattr,SIDOWNER ); -if (!(sbflags &CIFS_MOUNT_OVERR_GID )) -sid_to_id (cifs_sb, &parsed.group, fattr,SIDGROUP ); +if (rc) { +cifs_dbg (VFS ,"%s: failed to parse SIDs: %d\n" , + __func__, rc); + }else { +if (!(sbflags &CIFS_MOUNT_OVERR_UID )) +sid_to_id (cifs_sb, &parsed.owner, fattr,SIDOWNER ); +if (!(sbflags &CIFS_MOUNT_OVERR_GID )) +sid_to_id (cifs_sb, &parsed.group, fattr,SIDGROUP ); + } }static void __dir_info_to_fattr (struct cifs_fattr *fattr,const void *info)
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH v2 5/5 smb: client: fix reparse point file type corruption
In reply to Paulo Alcantarawsl_to_fattr() and posix_reparse_to_fattr() set the file type from reparse point tags (e.g. fattr->cf_mode |= S_IFCHR). When cf_mode already has S_IFMT bits set by the caller, this corrupts the file type.
For example, smb311_posix_info_to_fattr() sets cf_mode via wire_mode_to_posix() which includes S_IFREG, then calls cifs_reparse_point_to_fattr(). If the reparse tag indicates a character device, the result is S_IFREG | S_IFCHR = S_IFLNK, incorrectly turning the inode into a symlink.
Fix both functions by clearing the S_IFMT bits before the switch statement, so the reparse point tag can set the correct file type without interference from previously set bits.
--- fs/smb/client/reparse.c | 2 ++ 1 file changed, 2 insertions(+)
fs/smb/client/reparse.c +2 -0
@@ -1146,6 +1146,7 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data, fattr->cf_uid = cifs_sb->ctx->linux_uid; fattr->cf_gid = cifs_sb->ctx->linux_gid; + fattr->cf_mode &= ~S_IFMT ;switch (tag) {case IO_REPARSE_TAG_LX_SYMLINK : fattr->cf_mode |=S_IFLNK ;
@@ -1234,6 +1235,7 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb,return false; } + fattr->cf_mode &= ~S_IFMT ;switch (le64_to_cpu (buf->InodeType)) {case NFS_SPECFILE_CHR :if (le16_to_cpu (buf->ReparseDataLength) !=sizeof (buf->InodeType) +8 ) {
Signature · 1 line
2.55.0