* [PATCH 1/2] ksmbd: let the owner read and change the DACL when no ACE names it
2026-11-02 6:15 [PATCH 0/2] ksmbd: fix DACL checks for owners and groups Ian Bridges
@ 2026-11-02 6:15 ` Ian Bridges
2026-11-02 9:25 ` Namjae Jeon
2026-11-02 6:15 ` [PATCH 2/2] ksmbd: grant a caller what its groups and OWNER RIGHTS ACEs allow Ian Bridges
1 sibling, 1 reply; 5+ messages in thread
From: Ian Bridges @ 2026-11-02 6:15 UTC (permalink / raw)
To: Namjae Jeon
Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs,
linux-kernel, Aamir Ahmed, Ian Bridges, stable
smb_check_perm_dacl() refuses the owner when neither its DACL lookup nor
the POSIX ACL fallback matches. The owner then cannot read or replace
the DACL over SMB. MS-DTYP 2.5.3.2 grants the owner READ_CONTROL and
WRITE_DAC unless an OWNER RIGHTS ACE applies.
An empty DACL allows the owner's explicit requests for these rights but
refuses MAXIMUM_ALLOWED. smb2_open() uses that check for the maximal
access reply (MS-SMB2 3.3.5.9.5), so even a READ_CONTROL open fails when
it asks for that reply.
Grant the owner's implicit rights on both paths, for explicit requests
and MAXIMUM_ALLOWED, unless an OWNER RIGHTS ACE applies.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
Notes:
Reachable on a share with "vfs objects = acl_xattr", for example after
the owner replaces a file's DACL with one that grants only another
user.
The empty-DACL check this patch changes came with de0cffef897c ("ksmbd:
deny access for an empty DACL instead of granting the request"), merged
for v7.4, so an older stable tree needs that commit first. The check
before it refused MAXIMUM_ALLOWED on an empty DACL as well.
Tested on ksmbd-for-next d13da43938d1 with KASAN. Only the owner's
results change. The owner of a file whose only ACE grants another user
can now open it for READ_CONTROL or WRITE_DAC, and is still refused
READ_DATA. On an empty DACL, the owner's MAXIMUM_ALLOWED handle gets
READ_CONTROL and WRITE_DAC when POSIX permits the open. A READ_CONTROL
open with a maximal access context also succeeds. Its handle holds
READ_CONTROL, and its reply reports both rights. Without POSIX read
permission, MAXIMUM_ALLOWED still fails in smb2_open(). An ACE stored
after a DACL whose ACE count is zero still grants nothing.
A reproducer is available on request.
This fix was developed with LLM assistance.
fs/smb/server/smbacl.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 664f01263611..83974643aa88 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1477,6 +1477,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
int i, found = 0;
unsigned int access_bits = 0, denied = 0;
struct smb_ace *others_ace = NULL;
+ bool or_ace = false;
struct posix_acl_entry *pa_entry;
unsigned int sid_type = SIDOWNER;
unsigned short ace_size;
@@ -1509,6 +1510,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
is_owner = uid == from_kuid(&init_user_ns, vfsuid_into_kuid(vfsuid));
if (!pdacl->num_aces) {
+ unsigned int allowed = 0;
+
/*
* An empty (present, zero-ACE) DACL grants no access to
* anyone except the object owner's implicit READ_CONTROL
@@ -1517,9 +1520,14 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
* not let trailing bytes after a zero-ACE DACL become an
* implicit grant.
*/
- if (!is_owner ||
- (*pdaccess & ~(FILE_READ_CONTROL_LE | FILE_WRITE_DAC_LE)))
+ if (is_owner)
+ allowed = READ_CONTROL | WRITE_DAC;
+ if (*pdaccess & FILE_MAXIMAL_ACCESS_LE)
+ requested |= allowed;
+ if (!allowed || requested & ~allowed)
rc = -EACCES;
+ else
+ *pdaccess = cpu_to_le32(requested);
goto err_out;
}
@@ -1604,6 +1612,9 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
found = 1;
break;
}
+ if (!compare_sids(&sid_owner_rights, &ace->sid) &&
+ !(ace->flags & INHERIT_ONLY_ACE))
+ or_ace = true;
if (!compare_sids(&sid_everyone, &ace->sid) ||
!compare_sids(&sid_authusers, &ace->sid))
others_ace = ace;
@@ -1645,8 +1656,23 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
if (others_ace) {
ace = others_ace;
} else {
- ksmbd_debug(SMB, "Can't find corresponding sid\n");
- rc = -EACCES;
+ unsigned int allowed = 0;
+
+ /*
+ * No ACE names the owner, who may still read and
+ * change the DACL unless OWNER RIGHTS applies
+ * (MS-DTYP 2.5.3.2).
+ */
+ if (is_owner && !or_ace)
+ allowed = READ_CONTROL | WRITE_DAC;
+ if (*pdaccess & FILE_MAXIMAL_ACCESS_LE)
+ requested |= allowed;
+ if (!allowed || requested & ~allowed) {
+ ksmbd_debug(SMB, "Can't find corresponding sid\n");
+ rc = -EACCES;
+ goto err_out;
+ }
+ *pdaccess = cpu_to_le32(requested);
goto err_out;
}
}
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] ksmbd: grant a caller what its groups and OWNER RIGHTS ACEs allow
2026-11-02 6:15 [PATCH 0/2] ksmbd: fix DACL checks for owners and groups Ian Bridges
2026-11-02 6:15 ` [PATCH 1/2] ksmbd: let the owner read and change the DACL when no ACE names it Ian Bridges
@ 2026-11-02 6:15 ` Ian Bridges
2026-11-02 9:15 ` Namjae Jeon
1 sibling, 1 reply; 5+ messages in thread
From: Ian Bridges @ 2026-11-02 6:15 UTC (permalink / raw)
To: Namjae Jeon
Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs,
linux-kernel, Aamir Ahmed, Ian Bridges, stable
smb_check_perm_dacl() does not match ACEs for the caller's groups. It
also refuses an owner named only by an OWNER RIGHTS ACE, although its
MAXIMUM_ALLOWED walk already counts that ACE. These callers cannot use
rights the DACL grants them under MS-DTYP 2.5.3.2.
Run the existing ordered walk for every request and match Unix group
ACEs (S-1-22-2) against the caller's credentials. Use its result when
the existing SID lookup and POSIX ACL fallback do not decide the open.
Group ACEs remain excluded from MAXIMUM_ALLOWED. Adding them requires
corresponding changes in smb2_open(), which derives open flags and the
maximal access reply from that answer.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
Notes:
Reachable on a share with "vfs objects = acl_xattr", for example on a
file whose DACL only has ACEs for Unix groups (S-1-22-2) or OWNER
RIGHTS.
MAXIMUM_ALLOWED does not count groups here because smb2_open() also
builds the open flags and the maximal access reply from that answer.
Counting them there makes some MAXIMUM_ALLOWED opens fail that work
today.
Tested on ksmbd-for-next d13da43938d1 with KASAN, on top of the earlier
patch of this series. Results change for members of the file's group and
of other groups the DACL names, and for the owner. A member of a group
that the DACL grants full access now reads the file. An owner that only
an OWNER RIGHTS ACE names now gets what that ACE allows. Its
MAXIMUM_ALLOWED opens, refused before, now succeed with the rights in
the maximal access reply. DACLs with no ACE give the same results as
before.
A reproducer is available on request.
This fix was developed with LLM assistance.
fs/smb/server/smbacl.c | 55 +++++++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 9 deletions(-)
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 83974643aa88..2abb7ead9948 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1459,6 +1459,22 @@ bool smb_inherit_flags(int flags, bool is_dir)
return false;
}
+/* Whether the caller's credentials hold the group of an S-1-22-2-<gid> SID */
+static bool smb_sid_in_group(const struct smb_sid *sid)
+{
+ if (sid->num_subauth != 2 || compare_sids(sid, &sid_unix_groups))
+ return false;
+ /*
+ * ksmbd_override_fsids() builds the caller's credentials with
+ * make_kgid(&init_user_ns, ...), so the group membership test must
+ * compare in that space. Applying the mount idmap here (as when
+ * resolving a SID to an on-disk gid) would mismatch on idmapped
+ * mounts.
+ */
+ return in_group_p(make_kgid(&init_user_ns,
+ le32_to_cpu(sid->sub_auth[1])));
+}
+
int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
__le32 *pdaccess, __le32 raw_daccess, int uid,
bool strict)
@@ -1475,13 +1491,16 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
int granted = requested;
struct smb_ace *ace;
int i, found = 0;
- unsigned int access_bits = 0, denied = 0;
+ unsigned int access_bits = 0, denied = 0, ordered = 0;
struct smb_ace *others_ace = NULL;
- bool or_ace = false;
+ bool or_ace = false, in_group = false;
struct posix_acl_entry *pa_entry;
unsigned int sid_type = SIDOWNER;
unsigned short ace_size;
bool is_owner, owner_rights = false;
+ bool maximal = *pdaccess & FILE_MAXIMAL_ACCESS_LE;
+ /* The answer to MAXIMUM_ALLOWED stays without the caller's groups */
+ bool groups = !maximal;
vfsuid_t vfsuid;
ksmbd_debug(SMB, "check permission using windows acl\n");
@@ -1522,7 +1541,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
*/
if (is_owner)
allowed = READ_CONTROL | WRITE_DAC;
- if (*pdaccess & FILE_MAXIMAL_ACCESS_LE)
+ if (maximal)
requested |= allowed;
if (!allowed || requested & ~allowed)
rc = -EACCES;
@@ -1531,7 +1550,12 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
goto err_out;
}
- if (*pdaccess & FILE_MAXIMAL_ACCESS_LE) {
+ /*
+ * Evaluate the DACL in order (MS-DTYP 2.5.3.2). MAXIMUM_ALLOWED takes
+ * its result, and so does a caller that only group or OWNER RIGHTS
+ * ACEs name.
+ */
+ {
ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
aces_size = pdacl_size - sizeof(struct smb_acl);
for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
@@ -1562,7 +1586,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
compare_sids(&sid_unix_NFS_mode, &ace->sid) &&
compare_sids(&sid_everyone, &ace->sid) &&
compare_sids(&sid_authusers, &ace->sid) &&
- compare_sids(&sid_owner_rights, &ace->sid)))
+ compare_sids(&sid_owner_rights, &ace->sid) &&
+ !(groups && smb_sid_in_group(&ace->sid))))
goto next_ace;
switch (ace->type) {
@@ -1578,15 +1603,17 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
next_ace:
ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size));
}
- if (is_owner && !owner_rights)
+ ordered = access_bits & ~denied;
+ if (maximal && is_owner && !owner_rights)
access_bits |= READ_CONTROL | WRITE_DAC |
FILE_READ_ATTRIBUTES | DELETE;
access_bits &= ~denied;
- if ((raw_daccess & FILE_GENERIC_EXECUTE_LE) &&
+ if (maximal && (raw_daccess & FILE_GENERIC_EXECUTE_LE) &&
S_ISREG(d_inode(path->dentry)->i_mode) &&
(access_bits & GENERIC_READ_FLAGS) == GENERIC_READ_FLAGS)
access_bits |= FILE_EXECUTE;
- granted = requested | access_bits;
+ if (maximal)
+ granted = requested | access_bits;
}
ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
@@ -1615,6 +1642,10 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
if (!compare_sids(&sid_owner_rights, &ace->sid) &&
!(ace->flags & INHERIT_ONLY_ACE))
or_ace = true;
+ if (!(ace->flags & INHERIT_ONLY_ACE) &&
+ (smb_sid_in_group(&ace->sid) ||
+ (is_owner && !compare_sids(&sid_owner_rights, &ace->sid))))
+ in_group = true;
if (!compare_sids(&sid_everyone, &ace->sid) ||
!compare_sids(&sid_authusers, &ace->sid))
others_ace = ace;
@@ -1665,7 +1696,13 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
*/
if (is_owner && !or_ace)
allowed = READ_CONTROL | WRITE_DAC;
- if (*pdaccess & FILE_MAXIMAL_ACCESS_LE)
+ /*
+ * A group the caller is in, or OWNER RIGHTS for the
+ * owner, gives what the DACL grants it in order.
+ */
+ if (in_group)
+ allowed |= ordered;
+ if (maximal)
requested |= allowed;
if (!allowed || requested & ~allowed) {
ksmbd_debug(SMB, "Can't find corresponding sid\n");
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread