FAILED: patch "[PATCH] ksmbd: require minimum ACE size in smb_check_perm_dacl()" failed to apply to 6.6-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x d07b26f39246a82399661936dd0c853983cfade7
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026042426-growing-dime-ce6e@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From d07b26f39246a82399661936dd0c853983cfade7 Mon Sep 17 00:00:00 2001
From: Michael Bommarito <[email protected]>
Date: Tue, 14 Apr 2026 15:15:33 -0400
Subject: [PATCH] ksmbd: require minimum ACE size in smb_check_perm_dacl()
Both ACE-walk loops in smb_check_perm_dacl() only guard against an under-sized remaining buffer, not against an ACE whose declared ace->size is smaller than the struct it claims to describe:
if (offsetof(struct smb_ace, access_req) > aces_size)
break;
ace_size = le16_to_cpu(ace->size);
if (ace_size > aces_size)
break;The first check only requires the 4-byte ACE header to be in bounds; it does not require access_req (4 bytes at offset 4) to be readable. An attacker who has set a crafted DACL on a file they own can declare ace->size == 4 with aces_size == 4, pass both checks, and then
granted |= le32_to_cpu(ace->access_req); /* upper loop */
compare_sids(&sid, &ace->sid); /* lower loop */
reads access_req at offset 4 (OOB by up to 4 bytes) and ace->sid at offset 8 (OOB by up to CIFS_SID_BASE_SIZE + SID_MAX_SUB_AUTHORITIES
* 4 bytes).
Tighten both loops to require
ace_size >= offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE
which is the smallest valid on-wire ACE layout (4-byte header + 4-byte access_req + 8-byte sid base with zero sub-auths). Also reject ACEs whose sid.num_subauth exceeds SID_MAX_SUB_AUTHORITIES before letting compare_sids() dereference sub_auth[] entries.
parse_sec_desc() already enforces an equivalent check (lines 441-448); smb_check_perm_dacl() simply grew weaker validation over time.
Reachability: authenticated SMB client with permission to set an ACL on a file. On a subsequent CREATE against that file, the kernel walks the stored DACL via smb_check_perm_dacl() and triggers the OOB read. Not pre-auth, and the OOB read is not reflected to the attacker, but KASAN reports and kernel state corruption are possible.
fs/smb/server/smbacl.c +13 -4
@@ -1342,10 +1342,13 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl )); aces_size = acl_size -sizeof (struct smb_acl );for (i =0 ; i <le16_to_cpu (pdacl->num_aces); i++) { -if (offsetof(struct smb_ace , access_req) > aces_size) +if (offsetof(struct smb_ace , sid) + + aces_size <CIFS_SID_BASE_SIZE )break ; ace_size =le16_to_cpu (ace->size); -if (ace_size > aces_size) +if (ace_size > aces_size || + ace_size < offsetof(struct smb_ace , sid) + +CIFS_SID_BASE_SIZE )break ; aces_size -= ace_size; granted |=le32_to_cpu (ace->access_req);
@@ -1360,13 +1363,19 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl )); aces_size = acl_size -sizeof (struct smb_acl );for (i =0 ; i <le16_to_cpu (pdacl->num_aces); i++) { -if (offsetof(struct smb_ace , access_req) > aces_size) +if (offsetof(struct smb_ace , sid) + + aces_size <CIFS_SID_BASE_SIZE )break ; ace_size =le16_to_cpu (ace->size); -if (ace_size > aces_size) +if (ace_size > aces_size || + ace_size < offsetof(struct smb_ace , sid) + +CIFS_SID_BASE_SIZE )break ; aces_size -= ace_size; +if (ace->sid.num_subauth >SID_MAX_SUB_AUTHORITIES ) +break ; +if (!compare_sids (&sid, &ace->sid) || !compare_sids (&sid_unix_NFS_mode, &ace->sid)) { found =1 ;
Sasha Levin
maintainerPATCH 1/3 smb: move some duplicate definitions to common/smbacl.h
From: ChenXiaoSong <[email protected]>
[ Upstream commit b51174da743b6b7cd87c02e882ebe60dcb99f8bf ]
In order to maintain the code more easily, move duplicate definitions to new common header file.
Stable-dep-of: d07b26f39246 ("ksmbd: require minimum ACE size in smb_check_perm_dacl()")
--- fs/smb/client/cifsacl.h | 91 +----------------------------- fs/smb/common/smbacl.h | 121 ++++++++++++++++++++++++++++++++++++++++ fs/smb/server/smbacl.h | 111 +----------------------------------- 3 files changed, 123 insertions(+), 200 deletions(-) create mode 100644 fs/smb/common/smbacl.h
fs/smb/client/cifsacl.h +1 -90
@@ -9,8 +9,7 @@#ifndef _CIFSACL_H#define _CIFSACL_H -#define NUM_AUTHS (6)/* number of authority fields */ -#define SID_MAX_SUB_AUTHORITIES (15)/* max number of sub authority fields */ +#include "../common/smbacl.h" #define READ_BIT 0x4#define WRITE_BIT 0x2
@@ -23,12 +22,6 @@#define UBITSHIFT 6#define GBITSHIFT 3 -#define ACCESS_ALLOWED 0 -#define ACCESS_DENIED 1 - -#define SIDOWNER 1 -#define SIDGROUP 2 -/* * Security Descriptor length containing DACL with 3 ACEs (one each for * owner, group and world).
@@ -37,88 +30,6 @@sizeof (struct smb_acl ) + \ (sizeof(struct smb_ace) * 4)) -/* -* Maximum size of a string representation of a SID: -* -* The fields are unsigned values in decimal. So: -* -* u8: max 3 bytes in decimal -* u32: max 10 bytes in decimal -* -* "S-" + 3 bytes for version field + 15 for authority field + NULL terminator -* -* For authority field, max is when all 6 values are non-zero and it must be -* represented in hex. So "-0x" + 12 hex digits. -* -* Add 11 bytes for each subauthority field (10 bytes each + 1 for '-') -*/ -#define SID_STRING_BASE_SIZE (2 + 3 + 15 + 1) -#define SID_STRING_SUBAUTH_SIZE (11)/* size of a single subauth string */ - -struct smb_ntsd { -__le16 revision;/* revision level */ -__le16 type; -__le32 osidoffset; -__le32 gsidoffset; -__le32 sacloffset; -__le32 dacloffset; -} __attribute__((packed)); - -struct smb_sid { -__u8 revision;/* revision level */ -__u8 num_subauth; -__u8 authority[NUM_AUTHS ]; -__le32 sub_auth[SID_MAX_SUB_AUTHORITIES ];/* sub_auth[num_subauth] */ -} __attribute__((packed)); - -/* size of a struct smb_sid, sans sub_auth array */ -#define CIFS_SID_BASE_SIZE (1 + 1 + NUM_AUTHS) - -struct smb_acl { -__le16 revision;/* revision level */ -__le16 size; -__le32 num_aces; -} __attribute__((packed)); - -/* ACE types - see MS-DTYP 2.4.4.1 */ -#define ACCESS_ALLOWED_ACE_TYPE 0x00 -#define ACCESS_DENIED_ACE_TYPE 0x01 -#define SYSTEM_AUDIT_ACE_TYPE 0x02 -#define SYSTEM_ALARM_ACE_TYPE 0x03 -#define ACCESS_ALLOWED_COMPOUND_ACE_TYPE 0x04 -#define ACCESS_ALLOWED_OBJECT_ACE_TYPE 0x05 -#define ACCESS_DENIED_OBJECT_ACE_TYPE 0x06 -#define SYSTEM_AUDIT_OBJECT_ACE_TYPE 0x07 -#define SYSTEM_ALARM_OBJECT_ACE_TYPE 0x08 -#define ACCESS_ALLOWED_CALLBACK_ACE_TYPE 0x09 -#define ACCESS_DENIED_CALLBACK_ACE_TYPE 0x0A -#define ACCESS_ALLOWED_CALLBACK_OBJECT_ACE_TYPE 0x0B -#define ACCESS_DENIED_CALLBACK_OBJECT_ACE_TYPE 0x0C -#define SYSTEM_AUDIT_CALLBACK_ACE_TYPE 0x0D -#define SYSTEM_ALARM_CALLBACK_ACE_TYPE 0x0E/* Reserved */ -#define SYSTEM_AUDIT_CALLBACK_OBJECT_ACE_TYPE 0x0F -#define SYSTEM_ALARM_CALLBACK_OBJECT_ACE_TYPE 0x10/* reserved */ -#define SYSTEM_MANDATORY_LABEL_ACE_TYPE 0x11 -#define SYSTEM_RESOURCE_ATTRIBUTE_ACE_TYPE 0x12 -#define SYSTEM_SCOPED_POLICY_ID_ACE_TYPE 0x13 - -/* ACE flags */ -#define OBJECT_INHERIT_ACE 0x01 -#define CONTAINER_INHERIT_ACE 0x02 -#define NO_PROPAGATE_INHERIT_ACE 0x04 -#define INHERIT_ONLY_ACE 0x08 -#define INHERITED_ACE 0x10 -#define SUCCESSFUL_ACCESS_ACE_FLAG 0x40 -#define FAILED_ACCESS_ACE_FLAG 0x80 - -struct smb_ace { -__u8 type;/* see above and MS-DTYP 2.4.4.1 */ -__u8 flags; -__le16 size; -__le32 access_req; -struct smb_sid sid;/* ie UUID of user or group who gets these perms */ -} __attribute__((packed)); -/* * The current SMB3 form of security descriptor is similar to what was used for * cifs (see above) but some fields are split, and fields in the struct below
fs/smb/common/smbacl.h new file +121 -0
121 lines not on this page · raw message
fs/smb/server/smbacl.h +1 -110
131 lines not on this page · raw message
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/3 smb: common: change the data type of num_aces to le16
From: Namjae Jeon <[email protected]>
[ Upstream commit 62e7dd0a39c2d0d7ff03274c36df971f1b3d2d0d ]
2.4.5 in [MS-DTYP].pdf describe the data type of num_aces as le16.
AceCount (2 bytes): An unsigned 16-bit integer that specifies the count of the number of ACE records in the ACL.
Change it to le16 and add reserved field to smb_acl struct.
Stable-dep-of: d07b26f39246 ("ksmbd: require minimum ACE size in smb_check_perm_dacl()")
--- fs/smb/client/cifsacl.c | 26 +++++++++++++------------- fs/smb/common/smbacl.h | 3 ++- fs/smb/server/smbacl.c | 31 ++++++++++++++++--------------- fs/smb/server/smbacl.h | 2 +- 4 files changed, 32 insertions(+), 30 deletions(-)
fs/smb/client/cifsacl.c +13 -13
@@ -763,7 +763,7 @@ static void parse_dacl(struct smb_acl *pdacl, char *end_of_acl, struct cifs_fattr *fattr, bool mode_from_special_sid) { int i; -int num_aces =0 ; +u16 num_aces =0 ;int acl_size;char *acl_base;struct smb_ace **ppace;
@@ -786,7 +786,7 @@ static void parse_dacl(struct smb_acl *pdacl, char *end_of_acl,cifs_dbg (NOISY ,"DACL revision %d size %d num aces %d\n" ,le16_to_cpu (pdacl->revision),le16_to_cpu (pdacl->size), -le32_to_cpu (pdacl->num_aces)); +le16_to_cpu (pdacl->num_aces));/* reset rwx permissions for user/group/other. Also, if num_aces is 0 i.e. DACL has no ACEs,
@@ -796,7 +796,7 @@ static void parse_dacl(struct smb_acl *pdacl, char *end_of_acl, acl_base = (char *)pdacl; acl_size =sizeof (struct smb_acl ); - num_aces =le32_to_cpu (pdacl->num_aces); + num_aces =le16_to_cpu (pdacl->num_aces);if (num_aces >0 ) {umode_t denied_mode =0 ;
@@ -956,12 +956,12 @@ unsigned int setup_special_user_owner_ACE(struct smb_ace *pntace)static void populate_new_aces (char *nacl_base,struct smb_sid *pownersid,struct smb_sid *pgrpsid, -__u64 *pnmode,u32 *pnum_aces,u16 *pnsize, +__u64 *pnmode,u16 *pnum_aces,u16 *pnsize,bool modefromsid,bool posix) {__u64 nmode; -u32 num_aces =0 ; +u16 num_aces =0 ;u16 nsize =0 ;__u64 user_mode;__u64 group_mode;
@@ -1069,7 +1069,7 @@ static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *pu16 size =0 ;struct smb_ace *pntace =NULL ;char *acl_base =NULL ; -u32 src_num_aces =0 ; +u16 src_num_aces =0 ;u16 nsize =0 ;struct smb_ace *pnntace =NULL ;char *nacl_base =NULL ;
@@ -1077,7 +1077,7 @@ static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *p acl_base = (char *)pdacl; size =sizeof (struct smb_acl ); - src_num_aces =le32_to_cpu (pdacl->num_aces); + src_num_aces =le16_to_cpu (pdacl->num_aces); nacl_base = (char *)pndacl; nsize =sizeof (struct smb_acl );
@@ -1109,11 +1109,11 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,u16 size =0 ;struct smb_ace *pntace =NULL ;char *acl_base =NULL ; -u32 src_num_aces =0 ; +u16 src_num_aces =0 ;u16 nsize =0 ;struct smb_ace *pnntace =NULL ;char *nacl_base =NULL ; -u32 num_aces =0 ; +u16 num_aces =0 ;bool new_aces_set = false;/* Assuming that pndacl and pnmode are never NULL */
@@ -1131,7 +1131,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, acl_base = (char *)pdacl; size =sizeof (struct smb_acl ); - src_num_aces =le32_to_cpu (pdacl->num_aces); + src_num_aces =le16_to_cpu (pdacl->num_aces);/* Retain old ACEs which we can retain */ for (i =0 ; i < src_num_aces; ++i) {
@@ -1177,7 +1177,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, } finalize_dacl: - pndacl->num_aces =cpu_to_le32 (num_aces); + pndacl->num_aces =cpu_to_le16 (num_aces); pndacl->size =cpu_to_le16 (nsize);return 0 ;
@@ -1312,7 +1312,7 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, dacloffset ? dacl_ptr->revision :cpu_to_le16 (ACL_REVISION ); ndacl_ptr->size =cpu_to_le16 (0 ); - ndacl_ptr->num_aces =cpu_to_le32 (0 ); + ndacl_ptr->num_aces =cpu_to_le16 (0 ); rc =set_chmod_dacl (dacl_ptr, ndacl_ptr, owner_sid_ptr, group_sid_ptr, pnmode, mode_from_sid, posix);
@@ -1670,7 +1670,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, dacl_ptr = (struct smb_acl *)((char *)pntsd + dacloffset);if (mode_from_sid) nsecdesclen += -le32_to_cpu (dacl_ptr->num_aces) *sizeof (struct smb_ace ); +le16_to_cpu (dacl_ptr->num_aces) *sizeof (struct smb_ace );else /* cifsacl */ nsecdesclen +=le16_to_cpu (dacl_ptr->size); }
fs/smb/common/smbacl.h +2 -1
@@ -107,7 +107,8 @@ struct smb_sid {struct smb_acl {__le16 revision;/* revision level */ __le16 size; -__le32 num_aces; +__le16 num_aces; +__le16 reserved; } __attribute__((packed));struct smb_ace {
fs/smb/server/smbacl.c +16 -15
@@ -338,7 +338,7 @@ void posix_state_to_acl(struct posix_acl_state *state, pace->e_perm = state->other.allow; } -int init_acl_state (struct posix_acl_state *state,int cnt) +int init_acl_state (struct posix_acl_state *state,u16 cnt) {int alloc;
@@ -373,7 +373,7 @@ static void parse_dacl(struct mnt_idmap *idmap, struct smb_fattr *fattr) {int i, ret; -int num_aces =0 ; +u16 num_aces =0 ;unsigned int acl_size;char *acl_base;struct smb_ace **ppace;
@@ -394,12 +394,12 @@ static void parse_dacl(struct mnt_idmap *idmap,ksmbd_debug (SMB ,"DACL revision %d size %d num aces %d\n" ,le16_to_cpu (pdacl->revision),le16_to_cpu (pdacl->size), -le32_to_cpu (pdacl->num_aces)); +le16_to_cpu (pdacl->num_aces)); acl_base = (char *)pdacl; acl_size =sizeof (struct smb_acl ); - num_aces =le32_to_cpu (pdacl->num_aces); + num_aces =le16_to_cpu (pdacl->num_aces);if (num_aces <=0 )return ;
@@ -588,7 +588,7 @@ static void parse_dacl(struct mnt_idmap *idmap,static void set_posix_acl_entries_dacl (struct mnt_idmap *idmap,struct smb_ace *pndace, -struct smb_fattr *fattr,u32 *num_aces, +struct smb_fattr *fattr,u16 *num_aces,u16 *size,u32 nt_aces_num) {struct posix_acl_entry *pace;
@@ -709,7 +709,7 @@ static void set_ntacl_dacl(struct mnt_idmap *idmap, struct smb_fattr *fattr) {struct smb_ace *ntace, *pndace; -int nt_num_aces =le32_to_cpu (nt_dacl->num_aces), num_aces =0 ; +u16 nt_num_aces =le16_to_cpu (nt_dacl->num_aces), num_aces =0 ;unsigned short size =0 ;int i;
@@ -736,7 +736,7 @@ static void set_ntacl_dacl(struct mnt_idmap *idmap,set_posix_acl_entries_dacl (idmap, pndace, fattr, &num_aces, &size, nt_num_aces); - pndacl->num_aces =cpu_to_le32 (num_aces); + pndacl->num_aces =cpu_to_le16 (num_aces); pndacl->size =cpu_to_le16 (le16_to_cpu (pndacl->size) + size); }
@@ -744,7 +744,7 @@ static void set_mode_dacl(struct mnt_idmap *idmap, struct smb_acl *pndacl, struct smb_fattr *fattr) { struct smb_ace *pace, *pndace; -u32 num_aces =0 ; +u16 num_aces =0 ;u16 size =0 , ace_size =0 ;uid_t uid;const struct smb_sid *sid;
@@ -800,7 +800,7 @@ static void set_mode_dacl(struct mnt_idmap *idmap, fattr->cf_mode, 0007); out: - pndacl->num_aces =cpu_to_le32 (num_aces); + pndacl->num_aces =cpu_to_le16 (num_aces); pndacl->size =cpu_to_le16 (le16_to_cpu (pndacl->size) + size); }
@@ -1030,8 +1030,9 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,struct smb_sid owner_sid, group_sid;struct dentry *parent = path->dentry->d_parent;struct mnt_idmap *idmap =mnt_idmap (path->mnt); -int inherited_flags =0 , flags =0 , i, ace_cnt =0 , nt_size =0 , pdacl_size; -int rc =0 , num_aces, dacloffset, pntsd_type, pntsd_size, acl_len, aces_size; +int inherited_flags =0 , flags =0 , i, nt_size =0 , pdacl_size; +int rc =0 , dacloffset, pntsd_type, pntsd_size, acl_len, aces_size; +u16 num_aces, ace_cnt =0 ;char *aces_base;bool is_dir =S_ISDIR (d_inode (path->dentry)->i_mode);
@@ -1047,7 +1048,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, parent_pdacl = (struct smb_acl *)((char *)parent_pntsd + dacloffset); acl_len = pntsd_size - dacloffset; - num_aces =le32_to_cpu (parent_pdacl->num_aces); + num_aces =le16_to_cpu (parent_pdacl->num_aces); pntsd_type =le16_to_cpu (parent_pntsd->type); pdacl_size =le16_to_cpu (parent_pdacl->size);
@@ -1206,7 +1207,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, pdacl = (struct smb_acl *)((char *)pntsd +le32_to_cpu (pntsd->dacloffset)); pdacl->revision =cpu_to_le16 (2 ); pdacl->size =cpu_to_le16 (sizeof (struct smb_acl ) + nt_size); - pdacl->num_aces =cpu_to_le32 (ace_cnt); + pdacl->num_aces =cpu_to_le16 (ace_cnt); pace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl ));memcpy (pace, aces_base, nt_size); pntsd_size +=sizeof (struct smb_acl ) + nt_size;
@@ -1287,7 +1288,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl )); aces_size = acl_size -sizeof (struct smb_acl ); -for (i =0 ; i <le32_to_cpu (pdacl->num_aces); i++) { +for (i =0 ; i <le16_to_cpu (pdacl->num_aces); i++) {if (offsetof(struct smb_ace , access_req) > aces_size)break ; ace_size =le16_to_cpu (ace->size);
@@ -1308,7 +1309,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl )); aces_size = acl_size -sizeof (struct smb_acl ); -for (i =0 ; i <le32_to_cpu (pdacl->num_aces); i++) { +for (i =0 ; i <le16_to_cpu (pdacl->num_aces); i++) {if (offsetof(struct smb_ace , access_req) > aces_size)break ; ace_size =le16_to_cpu (ace->size);
fs/smb/server/smbacl.h +1 -1
@@ -86,7 +86,7 @@ int parse_sec_desc(struct mnt_idmap *idmap, struct smb_ntsd *pntsd,int build_sec_desc (struct mnt_idmap *idmap,struct smb_ntsd *pntsd,struct smb_ntsd *ppntsd,int ppntsd_size,int addition_info,__u32 *secdesclen,struct smb_fattr *fattr); -int init_acl_state (struct posix_acl_state *state,int cnt); +int init_acl_state (struct posix_acl_state *state,u16 cnt);void free_acl_state (struct posix_acl_state *state);void posix_state_to_acl (struct posix_acl_state *state,struct posix_acl_entry *pace);
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 3/3 ksmbd: require minimum ACE size in smb_check_perm_dacl()
In reply to Sasha LevinFrom: Michael Bommarito <[email protected]>
[ Upstream commit d07b26f39246a82399661936dd0c853983cfade7 ]
Both ACE-walk loops in smb_check_perm_dacl() only guard against an under-sized remaining buffer, not against an ACE whose declared ace->size is smaller than the struct it claims to describe:
if (offsetof(struct smb_ace, access_req) > aces_size)
break;
ace_size = le16_to_cpu(ace->size);
if (ace_size > aces_size)
break;The first check only requires the 4-byte ACE header to be in bounds; it does not require access_req (4 bytes at offset 4) to be readable. An attacker who has set a crafted DACL on a file they own can declare ace->size == 4 with aces_size == 4, pass both checks, and then
granted |= le32_to_cpu(ace->access_req); /* upper loop */
compare_sids(&sid, &ace->sid); /* lower loop */
reads access_req at offset 4 (OOB by up to 4 bytes) and ace->sid at offset 8 (OOB by up to CIFS_SID_BASE_SIZE + SID_MAX_SUB_AUTHORITIES
* 4 bytes).
Tighten both loops to require
ace_size >= offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE
which is the smallest valid on-wire ACE layout (4-byte header + 4-byte access_req + 8-byte sid base with zero sub-auths). Also reject ACEs whose sid.num_subauth exceeds SID_MAX_SUB_AUTHORITIES before letting compare_sids() dereference sub_auth[] entries.
parse_sec_desc() already enforces an equivalent check (lines 441-448); smb_check_perm_dacl() simply grew weaker validation over time.
Reachability: authenticated SMB client with permission to set an ACL on a file. On a subsequent CREATE against that file, the kernel walks the stored DACL via smb_check_perm_dacl() and triggers the OOB read. Not pre-auth, and the OOB read is not reflected to the attacker, but KASAN reports and kernel state corruption are possible.
--- fs/smb/server/smbacl.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
fs/smb/server/smbacl.c +13 -4
@@ -1289,10 +1289,13 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl )); aces_size = acl_size -sizeof (struct smb_acl );for (i =0 ; i <le16_to_cpu (pdacl->num_aces); i++) { -if (offsetof(struct smb_ace , access_req) > aces_size) +if (offsetof(struct smb_ace , sid) + + aces_size <CIFS_SID_BASE_SIZE )break ; ace_size =le16_to_cpu (ace->size); -if (ace_size > aces_size) +if (ace_size > aces_size || + ace_size < offsetof(struct smb_ace , sid) + +CIFS_SID_BASE_SIZE )break ; aces_size -= ace_size; granted |=le32_to_cpu (ace->access_req);
@@ -1310,13 +1313,19 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl +sizeof (struct smb_acl )); aces_size = acl_size -sizeof (struct smb_acl );for (i =0 ; i <le16_to_cpu (pdacl->num_aces); i++) { -if (offsetof(struct smb_ace , access_req) > aces_size) +if (offsetof(struct smb_ace , sid) + + aces_size <CIFS_SID_BASE_SIZE )break ; ace_size =le16_to_cpu (ace->size); -if (ace_size > aces_size) +if (ace_size > aces_size || + ace_size < offsetof(struct smb_ace , sid) + +CIFS_SID_BASE_SIZE )break ; aces_size -= ace_size; +if (ace->sid.num_subauth >SID_MAX_SUB_AUTHORITIES ) +break ; +if (!compare_sids (&sid, &ace->sid) || !compare_sids (&sid_unix_NFS_mode, &ace->sid)) { found =1 ;
Signature · 1 line
2.53.0