From: Sasha Levin <sashal@kernel.org>
To: stable@vger.kernel.org
Cc: Michael Bommarito <michael.bommarito@gmail.com>,
Steve French <stfrench@microsoft.com>,
Sasha Levin <sashal@kernel.org>
Subject: [PATCH 6.1.y] smb: client: validate dacloffset before building DACL pointers
Date: Fri, 15 May 2026 17:36:00 -0400 [thread overview]
Message-ID: <20260515213600.3513360-1-sashal@kernel.org> (raw)
In-Reply-To: <2026051249-grandson-underling-d82b@gregkh>
From: Michael Bommarito <michael.bommarito@gmail.com>
[ Upstream commit f98b48151cc502ada59d9778f0112d21f2586ca3 ]
parse_sec_desc(), build_sec_desc(), and the chown path in
id_mode_to_cifs_acl() all add the server-supplied dacloffset to pntsd
before proving a DACL header fits inside the returned security
descriptor.
On 32-bit builds a malicious server can return dacloffset near
U32_MAX, wrap the derived DACL pointer below end_of_acl, and then slip
past the later pointer-based bounds checks. build_sec_desc() and
id_mode_to_cifs_acl() can then dereference DACL fields from the wrapped
pointer in the chmod/chown rewrite paths.
Validate dacloffset numerically before building any DACL pointer and
reuse the same helper at the three DACL entry points.
Fixes: bc3e9dd9d104 ("cifs: Change SIDs in ACEs while transferring file ownership.")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
[ renamed smb_ntsd/smb_acl structs to cifs_ntsd/cifs_acl and kept existing inline ACL size check instead of using missing validate_dacl() helper ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/smb/client/cifsacl.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index a6c7566a01821..1707aaeecd551 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -1183,6 +1183,17 @@ static int parse_sid(struct cifs_sid *psid, char *end_of_acl)
return 0;
}
+static bool dacl_offset_valid(unsigned int acl_len, __u32 dacloffset)
+{
+ if (acl_len < sizeof(struct cifs_acl))
+ return false;
+
+ if (dacloffset < sizeof(struct cifs_ntsd))
+ return false;
+
+ return dacloffset <= acl_len - sizeof(struct cifs_acl);
+}
+
/* Convert CIFS ACL to POSIX form */
static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
@@ -1203,7 +1214,6 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
group_sid_ptr = (struct cifs_sid *)((char *)pntsd +
le32_to_cpu(pntsd->gsidoffset));
dacloffset = le32_to_cpu(pntsd->dacloffset);
- dacl_ptr = (struct cifs_acl *)((char *)pntsd + dacloffset);
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, le32_to_cpu(pntsd->osidoffset),
le32_to_cpu(pntsd->gsidoffset),
@@ -1234,11 +1244,18 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
return rc;
}
- if (dacloffset)
+ if (dacloffset) {
+ if (!dacl_offset_valid(acl_len, dacloffset)) {
+ cifs_dbg(VFS, "Server returned illegal DACL offset\n");
+ return -EINVAL;
+ }
+
+ dacl_ptr = (struct cifs_acl *)((char *)pntsd + dacloffset);
parse_dacl(dacl_ptr, end_of_acl, owner_sid_ptr,
group_sid_ptr, fattr, get_mode_from_special_sid);
- else
+ } else {
cifs_dbg(FYI, "no ACL\n"); /* BB grant all or default perms? */
+ }
return rc;
}
@@ -1261,6 +1278,11 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd,
dacloffset = le32_to_cpu(pntsd->dacloffset);
if (dacloffset) {
+ if (!dacl_offset_valid(secdesclen, dacloffset)) {
+ cifs_dbg(VFS, "Server returned illegal DACL offset\n");
+ return -EINVAL;
+ }
+
dacl_ptr = (struct cifs_acl *)((char *)pntsd + dacloffset);
if (end_of_acl < (char *)dacl_ptr + le16_to_cpu(dacl_ptr->size)) {
cifs_dbg(VFS, "Server returned illegal ACL size\n");
@@ -1628,6 +1650,12 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
nsecdesclen = sizeof(struct cifs_ntsd) + (sizeof(struct cifs_sid) * 2);
dacloffset = le32_to_cpu(pntsd->dacloffset);
if (dacloffset) {
+ if (!dacl_offset_valid(secdesclen, dacloffset)) {
+ cifs_dbg(VFS, "Server returned illegal DACL offset\n");
+ rc = -EINVAL;
+ goto id_mode_to_cifs_acl_exit;
+ }
+
dacl_ptr = (struct cifs_acl *)((char *)pntsd + dacloffset);
if (mode_from_sid)
nsecdesclen +=
@@ -1664,6 +1692,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
rc = ops->set_acl(pnntsd, nsecdesclen, inode, path, aclflag);
cifs_dbg(NOISY, "set_cifs_acl rc: %d\n", rc);
}
+id_mode_to_cifs_acl_exit:
cifs_put_tlink(tlink);
kfree(pnntsd);
--
2.53.0
prev parent reply other threads:[~2026-05-15 21:36 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-12 14:04 FAILED: patch "[PATCH] smb: client: validate dacloffset before building DACL" failed to apply to 6.1-stable tree gregkh
2026-05-15 21:36 ` Sasha Levin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260515213600.3513360-1-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=michael.bommarito@gmail.com \
--cc=stable@vger.kernel.org \
--cc=stfrench@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.