All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions
@ 2026-09-06 20:05 Paulo Alcantara
  2026-09-06 20:05 ` [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid Paulo Alcantara
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Arthur Lesuisse, Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, stable

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.

Reported-by: Arthur Lesuisse <arthur.lesuisse@ulb.be>
Closes: https://lore.kernel.org/r/DB9P190MB2012266F6B8DECBE5D26A1798DB52@DB9P190MB2012.EURP190.PROD.OUTLOOK.COM
Suggested-by: Arthur Lesuisse <arthur.lesuisse@ulb.be>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/inode.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 12ed8db10e00..49f9993ad567 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -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))
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
@ 2026-09-06 20:05 ` Paulo Alcantara
  2026-09-07 22:59   ` Namjae Jeon
  2026-09-06 20:05 ` [PATCH v4 3/7] smb: client: fix WSL reparse point uid/gid override Paulo Alcantara
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

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.

Closes: https://sashiko.dev/#/patchset/20260906155816.603278-1-pc%40manguebit.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 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(-)

diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 213a421bf8e9..def8908dd7e9 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -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) {
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 49f9993ad567..1fe0ef0a95db 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -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);
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 32a75afca8f5..1ea84f4ada39 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -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)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 3/7] smb: client: fix WSL reparse point uid/gid override
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
  2026-09-06 20:05 ` [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid Paulo Alcantara
@ 2026-09-06 20:05 ` Paulo Alcantara
  2026-09-07 22:59   ` Namjae Jeon
  2026-09-06 20:05 ` [PATCH v4 4/7] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr() Paulo Alcantara
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

wsl_to_fattr() unconditionally overwrites cf_uid/cf_gid with values
from WSL extended attributes ($LXUID/$LXGID), ignoring the forceuid
and forcegid mount options.

Fix this by initializing cf_uid/cf_gid to the mount defaults and
gating the $LXUID/$LXGID EA parsing on forceuid/forcegid.

Closes: https://sashiko.dev/#/patchset/20260906190803.667489-1-pc%40manguebit.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/reparse.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 5cc5b0410d48..178da801e775 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1137,10 +1137,14 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data,
 			 struct cifs_sb_info *cifs_sb,
 			 u32 tag, struct cifs_fattr *fattr)
 {
+	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 +1181,13 @@ 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)) {
+		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;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/7] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr()
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
  2026-09-06 20:05 ` [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid Paulo Alcantara
  2026-09-06 20:05 ` [PATCH v4 3/7] smb: client: fix WSL reparse point uid/gid override Paulo Alcantara
@ 2026-09-06 20:05 ` Paulo Alcantara
  2026-09-07 23:00   ` Namjae Jeon
  2026-09-06 20:05 ` [PATCH v4 5/7] smb: client: fix file type corruption in wsl_to_fattr() Paulo Alcantara
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

cifs_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.

Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/readdir.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 1ea84f4ada39..9530e5b01564 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -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 < 0) {
+		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)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 5/7] smb: client: fix file type corruption in wsl_to_fattr()
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
                   ` (2 preceding siblings ...)
  2026-09-06 20:05 ` [PATCH v4 4/7] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr() Paulo Alcantara
@ 2026-09-06 20:05 ` Paulo Alcantara
  2026-09-07 23:00   ` Namjae Jeon
  2026-09-06 20:05 ` [PATCH v4 6/7] smb: client: fix file type corruption in posix_reparse_to_fattr() Paulo Alcantara
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

Setting the file type in cf_mode without clearing the existing S_IFMT
bits first is wrong as it corrupts the file type when cf_mode already
has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK).

Clear S_IFMT before the switch statement.

Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/reparse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 178da801e775..8a19dee564b8 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1145,6 +1145,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;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 6/7] smb: client: fix file type corruption in posix_reparse_to_fattr()
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
                   ` (3 preceding siblings ...)
  2026-09-06 20:05 ` [PATCH v4 5/7] smb: client: fix file type corruption in wsl_to_fattr() Paulo Alcantara
@ 2026-09-06 20:05 ` Paulo Alcantara
  2026-09-07 23:00   ` Namjae Jeon
  2026-09-06 20:05 ` [PATCH v4 7/7] smb: client: fix file type corruption in cifs_reparse_point_to_fattr() Paulo Alcantara
  2026-09-07 22:58 ` [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Namjae Jeon
  6 siblings, 1 reply; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

Setting the file type in cf_mode without clearing the existing S_IFMT
bits first is wrong as it corrupts the file type when cf_mode already
has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK).

Use a local ftype variable to collect the new file type and apply it
after validation succeeds, clearing S_IFMT and setting the new type in
a single assignment.  This avoids stripping cf_mode on malformed
reparse points where the function returns false early.

Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/reparse.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 8a19dee564b8..616ca2dbfac4 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1212,6 +1212,7 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb,
 				   struct cifs_open_info_data *data)
 {
 	struct reparse_nfs_data_buffer *buf = (struct reparse_nfs_data_buffer *)data->reparse.buf;
+	umode_t ftype;
 
 	if (buf == NULL)
 		return true;
@@ -1227,7 +1228,7 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb,
 			WARN_ON_ONCE(1);
 			return false;
 		}
-		fattr->cf_mode |= S_IFCHR;
+		ftype = S_IFCHR;
 		fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
 		break;
 	case NFS_SPECFILE_BLK:
@@ -1235,22 +1236,23 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb,
 			WARN_ON_ONCE(1);
 			return false;
 		}
-		fattr->cf_mode |= S_IFBLK;
+		ftype = S_IFBLK;
 		fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
 		break;
 	case NFS_SPECFILE_FIFO:
-		fattr->cf_mode |= S_IFIFO;
+		ftype = S_IFIFO;
 		break;
 	case NFS_SPECFILE_SOCK:
-		fattr->cf_mode |= S_IFSOCK;
+		ftype = S_IFSOCK;
 		break;
 	case NFS_SPECFILE_LNK:
-		fattr->cf_mode |= S_IFLNK;
+		ftype = S_IFLNK;
 		break;
 	default:
 		WARN_ON_ONCE(1);
 		return false;
 	}
+	fattr->cf_mode = (fattr->cf_mode & ~S_IFMT) | ftype;
 	return true;
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 7/7] smb: client: fix file type corruption in cifs_reparse_point_to_fattr()
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
                   ` (4 preceding siblings ...)
  2026-09-06 20:05 ` [PATCH v4 6/7] smb: client: fix file type corruption in posix_reparse_to_fattr() Paulo Alcantara
@ 2026-09-06 20:05 ` Paulo Alcantara
  2026-09-07 23:01   ` Namjae Jeon
  2026-09-07 22:58 ` [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Namjae Jeon
  6 siblings, 1 reply; 14+ messages in thread
From: Paulo Alcantara @ 2026-09-06 20:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

Setting the file type in cf_mode without clearing the existing S_IFMT
bits first is wrong as it corrupts the file type when cf_mode already
has type bits set (e.g. S_IFREG | S_IFLNK == S_IFDIR | S_IFREG).

Clear S_IFMT before setting S_IFLNK for native and SMB1 symlinks.

Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/reparse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 616ca2dbfac4..b6bded042e78 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1280,6 +1280,7 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 		break;
 	case 0: /* SMB1 symlink */
 	case IO_REPARSE_TAG_SYMLINK:
+		fattr->cf_mode &= ~S_IFMT;
 		fattr->cf_mode |= S_IFLNK;
 		break;
 	default:
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions
  2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
                   ` (5 preceding siblings ...)
  2026-09-06 20:05 ` [PATCH v4 7/7] smb: client: fix file type corruption in cifs_reparse_point_to_fattr() Paulo Alcantara
@ 2026-09-07 22:58 ` Namjae Jeon
  6 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 22:58 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Arthur Lesuisse, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> 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.
>
> Reported-by: Arthur Lesuisse <arthur.lesuisse@ulb.be>
> Closes: https://lore.kernel.org/r/DB9P190MB2012266F6B8DECBE5D26A1798DB52@DB9P190MB2012.EURP190.PROD.OUTLOOK.COM
> Suggested-by: Arthur Lesuisse <arthur.lesuisse@ulb.be>
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid
  2026-09-06 20:05 ` [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid Paulo Alcantara
@ 2026-09-07 22:59   ` Namjae Jeon
  0 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 22:59 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> 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.
>
> Closes: https://sashiko.dev/#/patchset/20260906155816.603278-1-pc%40manguebit.org
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/7] smb: client: fix WSL reparse point uid/gid override
  2026-09-06 20:05 ` [PATCH v4 3/7] smb: client: fix WSL reparse point uid/gid override Paulo Alcantara
@ 2026-09-07 22:59   ` Namjae Jeon
  0 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 22:59 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> wsl_to_fattr() unconditionally overwrites cf_uid/cf_gid with values
> from WSL extended attributes ($LXUID/$LXGID), ignoring the forceuid
> and forcegid mount options.
>
> Fix this by initializing cf_uid/cf_gid to the mount defaults and
> gating the $LXUID/$LXGID EA parsing on forceuid/forcegid.
>
> Closes: https://sashiko.dev/#/patchset/20260906190803.667489-1-pc%40manguebit.org
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/7] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr()
  2026-09-06 20:05 ` [PATCH v4 4/7] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr() Paulo Alcantara
@ 2026-09-07 23:00   ` Namjae Jeon
  0 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 23:00 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> cifs_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.
>
> Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
> Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 5/7] smb: client: fix file type corruption in wsl_to_fattr()
  2026-09-06 20:05 ` [PATCH v4 5/7] smb: client: fix file type corruption in wsl_to_fattr() Paulo Alcantara
@ 2026-09-07 23:00   ` Namjae Jeon
  0 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 23:00 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Setting the file type in cf_mode without clearing the existing S_IFMT
> bits first is wrong as it corrupts the file type when cf_mode already
> has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK).
>
> Clear S_IFMT before the switch statement.
>
> Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 6/7] smb: client: fix file type corruption in posix_reparse_to_fattr()
  2026-09-06 20:05 ` [PATCH v4 6/7] smb: client: fix file type corruption in posix_reparse_to_fattr() Paulo Alcantara
@ 2026-09-07 23:00   ` Namjae Jeon
  0 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 23:00 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Setting the file type in cf_mode without clearing the existing S_IFMT
> bits first is wrong as it corrupts the file type when cf_mode already
> has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK).
>
> Use a local ftype variable to collect the new file type and apply it
> after validation succeeds, clearing S_IFMT and setting the new type in
> a single assignment.  This avoids stripping cf_mode on malformed
> reparse points where the function returns false early.
>
> Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 7/7] smb: client: fix file type corruption in cifs_reparse_point_to_fattr()
  2026-09-06 20:05 ` [PATCH v4 7/7] smb: client: fix file type corruption in cifs_reparse_point_to_fattr() Paulo Alcantara
@ 2026-09-07 23:01   ` Namjae Jeon
  0 siblings, 0 replies; 14+ messages in thread
From: Namjae Jeon @ 2026-09-07 23:01 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: linux-cifs, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, stable

On Mon, Sep 7, 2026 at 5:05 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Setting the file type in cf_mode without clearing the existing S_IFMT
> bits first is wrong as it corrupts the file type when cf_mode already
> has type bits set (e.g. S_IFREG | S_IFLNK == S_IFDIR | S_IFREG).
>
> Clear S_IFMT before setting S_IFLNK for native and SMB1 symlinks.
>
> Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-09-07 23:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-06 20:05 [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Paulo Alcantara
2026-09-06 20:05 ` [PATCH v4 2/7] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid Paulo Alcantara
2026-09-07 22:59   ` Namjae Jeon
2026-09-06 20:05 ` [PATCH v4 3/7] smb: client: fix WSL reparse point uid/gid override Paulo Alcantara
2026-09-07 22:59   ` Namjae Jeon
2026-09-06 20:05 ` [PATCH v4 4/7] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr() Paulo Alcantara
2026-09-07 23:00   ` Namjae Jeon
2026-09-06 20:05 ` [PATCH v4 5/7] smb: client: fix file type corruption in wsl_to_fattr() Paulo Alcantara
2026-09-07 23:00   ` Namjae Jeon
2026-09-06 20:05 ` [PATCH v4 6/7] smb: client: fix file type corruption in posix_reparse_to_fattr() Paulo Alcantara
2026-09-07 23:00   ` Namjae Jeon
2026-09-06 20:05 ` [PATCH v4 7/7] smb: client: fix file type corruption in cifs_reparse_point_to_fattr() Paulo Alcantara
2026-09-07 23:01   ` Namjae Jeon
2026-09-07 22:58 ` [PATCH v4 1/7] smb: client: fix uid/gid override in getattr with posix extensions Namjae Jeon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.