mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] ksmbd: fix explicit DACL checks and parent-derived rights
@ 2026-11-07 23:19 Ian Bridges
  2026-11-07 23:19 ` [PATCH v2 1/2] ksmbd: evaluate the DACL in order for explicit access requests Ian Bridges
  2026-11-07 23:19 ` [PATCH v2 2/2] ksmbd: let the parent's DACL grant DELETE and FILE_READ_ATTRIBUTES Ian Bridges
  0 siblings, 2 replies; 3+ messages in thread
From: Ian Bridges @ 2026-11-07 23:19 UTC (permalink / raw)
  To: linkinjeon
  Cc: senozhatsky, tom, chenxiaosong, linux-cifs, linux-kernel, Ian Bridges

For explicit access requests, smb_check_perm_dacl() uses one ACE: the
first for the caller or the last Everyone or Authenticated Users ACE.
When that ACE is a deny ACE, the function allows the complement of the
ACE's mask, so the deny grants every right it does not name. Deny ACEs
before that ACE are ignored. Guest and anonymous sessions also match
Authenticated Users. On acl_xattr shares, callers can therefore read
or write files that the DACL denies, and guest and anonymous sessions
can read files granted only to Authenticated Users.

The function also never checks the parent directory, whose
FILE_DELETE_CHILD grants DELETE on a file in it and whose
FILE_LIST_DIRECTORY grants FILE_READ_ATTRIBUTES. ksmbd can therefore
refuse the opens a caller needs to rename a file, set its delete
disposition or read its attributes, even when the parent grants those
rights.

- Patch 1 evaluates explicit requests in DACL order and fixes the
  Authenticated Users match.
- Patch 2 lets the parent's DACL grant DELETE and
  FILE_READ_ATTRIBUTES.

Patch 2 passes the authenticated argument that patch 1 adds to
smb_check_perm_dacl() on to the parent check. It also restores
parent-derived rights that patch 1 can remove from guests. Backport
both patches in order.

Older stable trees first need commit cc2f133e80eb ("ksmbd: fix maximum
allowed access checks") and the earlier DACL fixes through "ksmbd:
grant a caller what its groups and OWNER RIGHTS ACEs allow".

Changes since v1:
- Patch 2: in the parent check, ACEs for the caller's groups and OWNER
  RIGHTS now decide before the parent's POSIX ACL. Sashiko's review of
  v1 found that a POSIX ACL entry for the caller stopped the parent from
  granting DELETE through them.
- Patch 2: a comment on the DACL walk now says which MAXIMUM_ALLOWED
  requests skip group ACEs.

v1: https://lore.kernel.org/linux-cifs/20261107062233.1579198-1-icb@fastmail.org/

Each patch was tested with KASAN on ksmbd-for-next 6f42553e493d.
smbtorture gives the same results as on the base.

This series was developed with LLM assistance.

Ian Bridges (2):
  ksmbd: evaluate the DACL in order for explicit access requests
  ksmbd: let the parent's DACL grant DELETE and FILE_READ_ATTRIBUTES

 fs/smb/server/mgmt/user_config.h   |   5 ++
 fs/smb/server/smb2pdu.c            |  11 ++-
 fs/smb/server/smbacl.c             | 127 +++++++++++++++++++++++++----
 fs/smb/server/smbacl.h             |   2 +-
 fs/smb/server/tests/smbacl_kunit.c |   4 +-
 5 files changed, 126 insertions(+), 23 deletions(-)


base-commit: 6f42553e493d3f25059a215748ae241e525c865f
-- 
2.47.3


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

* [PATCH v2 1/2] ksmbd: evaluate the DACL in order for explicit access requests
  2026-11-07 23:19 [PATCH v2 0/2] ksmbd: fix explicit DACL checks and parent-derived rights Ian Bridges
@ 2026-11-07 23:19 ` Ian Bridges
  2026-11-07 23:19 ` [PATCH v2 2/2] ksmbd: let the parent's DACL grant DELETE and FILE_READ_ATTRIBUTES Ian Bridges
  1 sibling, 0 replies; 3+ messages in thread
From: Ian Bridges @ 2026-11-07 23:19 UTC (permalink / raw)
  To: linkinjeon
  Cc: senozhatsky, tom, chenxiaosong, linux-cifs, linux-kernel,
	Ian Bridges, stable

For requests without MAXIMUM_ALLOWED, smb_check_perm_dacl() selects one
ACE: the first for the caller or the last Everyone or Authenticated
Users ACE. When that ACE is a deny ACE, the function allows the
complement of the ACE's mask, so the deny grants every right it does
not name. Deny ACEs before the selected ACE are ignored. The selected
ACE can also be an inherit-only ACE, an ACE of an unsupported type, or
an NFS mode ACE. Guest and anonymous sessions also match Authenticated
Users, although they are not authenticated.

These bugs give real SMB access on acl_xattr shares. A deny of
FILE_READ_DATA can grant FILE_WRITE_DATA, and a deny of FILE_WRITE_DATA
can grant FILE_READ_DATA. A deny ACE for Everyone does the same for
every caller, including guests. Guest and anonymous sessions can also
read files granted only to Authenticated Users.

Use the ordered DACL walk for explicit requests. Match, by exact SID,
the caller, Everyone, Authenticated Users for authenticated sessions,
OWNER RIGHTS for the owner and the caller's groups. Ignore the NFS
mode SID.

Keep the single-ACE evaluation when a non-inherit-only object deny ACE
is present, as ksmbd does not parse those ACEs. Guest and anonymous
sessions do not match Authenticated Users there either.
MAXIMUM_ALLOWED remains unchanged.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Fixes: cc2f133e80eb ("ksmbd: fix maximum allowed access checks")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    This patch changes explicit requests only. MAXIMUM_ALLOWED and
    maximal-access replies still treat guest and anonymous sessions as
    Authenticated Users.
    
    The NFS mode ACE carries the POSIX mode but names no caller, so the
    DACL check ignores it. inode_permission() still enforces the mode.
    
    Ordered evaluation now allows a right that an allow ACE grants
    before a later deny ACE names it. A reproducer is available on
    request.

 fs/smb/server/mgmt/user_config.h   |  5 ++++
 fs/smb/server/smb2pdu.c            | 11 +++++--
 fs/smb/server/smbacl.c             | 47 +++++++++++++++++++++++-------
 fs/smb/server/smbacl.h             |  2 +-
 fs/smb/server/tests/smbacl_kunit.c |  4 +--
 5 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/fs/smb/server/mgmt/user_config.h b/fs/smb/server/mgmt/user_config.h
index cc460b4ff7d3..e116b2ee9917 100644
--- a/fs/smb/server/mgmt/user_config.h
+++ b/fs/smb/server/mgmt/user_config.h
@@ -67,4 +67,9 @@ struct ksmbd_user *ksmbd_alloc_user(struct ksmbd_login_response *resp,
 void ksmbd_free_user(struct ksmbd_user *user);
 bool ksmbd_anonymous_user(struct ksmbd_user *user);
 bool ksmbd_compare_user(struct ksmbd_user *u1, struct ksmbd_user *u2);
+
+static inline bool ksmbd_user_authenticated(struct ksmbd_user *user)
+{
+	return user && !user_guest(user) && !ksmbd_anonymous_user(user);
+}
 #endif /* __USER_CONFIG_MANAGEMENT_H__ */
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 5bbc33d5a3a8..72981756ee23 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -4338,7 +4338,8 @@ static int smb2_handle_app_instance_id(struct ksmbd_work *work,
 	/* Authorize the referenced open even if its name changes meanwhile. */
 	old_path = &old_fp->filp->f_path;
 	rc = smb_check_perm_dacl(conn, old_path, &maximal_access, 0,
-				 work->sess->user->uid, false);
+				 work->sess->user->uid,
+				 ksmbd_user_authenticated(work->sess->user), false);
 	if (rc) {
 		/* Access denial prevents takeover, not the CREATE itself. */
 		ksmbd_put_durable_fd(old_fp);
@@ -4860,14 +4861,17 @@ int smb2_open(struct ksmbd_work *work)
 	if (file_present && !(req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
 		rc = smb_check_perm_dacl(conn, &path, &daccess,
 					 req->DesiredAccess,
-					 sess->user->uid, false);
+					 sess->user->uid,
+					 ksmbd_user_authenticated(sess->user), false);
 		if (rc)
 			goto err_out;
 
 		if (maximal_access_ctxt) {
 			maximal_access = FILE_MAXIMAL_ACCESS_LE;
 			rc = smb_check_perm_dacl(conn, &path, &maximal_access,
-						 0, sess->user->uid, false);
+						 0, sess->user->uid,
+						 ksmbd_user_authenticated(sess->user),
+						 false);
 			if (rc)
 				goto err_out;
 
@@ -6347,6 +6351,7 @@ static int process_query_dir_entries(struct smb2_query_dir_private *priv)
 			rc = smb_check_perm_dacl(priv->work->conn, &path,
 						 &daccess, daccess,
 						 priv->work->sess->user->uid,
+						 ksmbd_user_authenticated(priv->work->sess->user),
 						 true);
 			if (rc) {
 				dput(dent);
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 32feafe9f73b..f773ab8efb3f 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1478,6 +1478,12 @@ static bool smb_sid_in_group(const struct smb_sid *sid)
 	return in_group_p(make_kgid(&init_user_ns, gid));
 }
 
+/* Whether two SIDs are the same SID */
+static bool smb_sid_equal(const struct smb_sid *a, const struct smb_sid *b)
+{
+	return a->num_subauth == b->num_subauth && !compare_sids(a, b);
+}
+
 /*
  * compare_sids() compares only the sub-authorities both SIDs have, so it
  * also matches a shorter or longer SID, such as S-1-3 or S-1-3-4-777
@@ -1490,7 +1496,7 @@ static bool smb_sid_is_owner_rights(const struct smb_sid *sid)
 
 int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 			__le32 *pdaccess, __le32 raw_daccess, int uid,
-			bool strict)
+			bool authenticated, bool strict)
 {
 	const struct mnt_idmap *idmap = mnt_idmap(path->mnt);
 	struct smb_ntsd *pntsd = NULL;
@@ -1513,6 +1519,11 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 	unsigned short ace_size;
 	bool is_owner, owner_rights = false;
 	bool maximal = *pdaccess & FILE_MAXIMAL_ACCESS_LE;
+	/*
+	 * MAXIMUM_ALLOWED requests match every session against
+	 * Authenticated Users.
+	 */
+	bool authusers = authenticated || maximal;
 	vfsuid_t vfsuid;
 
 	ksmbd_debug(SMB, "check permission using windows acl\n");
@@ -1564,9 +1575,10 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 
 	/*
 	 * Evaluate the DACL in order (MS-DTYP 2.5.3.2). access_bits collects
-	 * the MAXIMUM_ALLOWED result. grp_granted collects what ACEs for the
-	 * caller's groups, or OWNER RIGHTS for the owner, grant. It is used
-	 * when the SID lookup and POSIX ACL fallback below do not decide.
+	 * the result for the ACEs that apply to the caller, without its groups
+	 * for MAXIMUM_ALLOWED. grp_granted collects what ACEs for the caller's
+	 * groups, or OWNER RIGHTS for the owner, grant. It is used when the
+	 * SID lookup and POSIX ACL fallback below do not decide.
 	 */
 	ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
 	aces_size = pdacl_size - sizeof(struct smb_acl);
@@ -1606,10 +1618,18 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		    (compare_sids(&sid, &ace->sid) &&
 		     compare_sids(&sid_unix_NFS_mode, &ace->sid) &&
 		     compare_sids(&sid_everyone, &ace->sid) &&
-		     compare_sids(&sid_authusers, &ace->sid) &&
+		     (!authusers ||
+		      compare_sids(&sid_authusers, &ace->sid)) &&
 		     compare_sids(&sid_owner_rights, &ace->sid) &&
 		     !member))
 			goto next_ace;
+		/* An explicit request counts only SIDs that match exactly */
+		if (!maximal && !member &&
+		    !smb_sid_equal(&sid, &ace->sid) &&
+		    !smb_sid_equal(&sid_everyone, &ace->sid) &&
+		    !(authusers && smb_sid_equal(&sid_authusers, &ace->sid)) &&
+		    !smb_sid_is_owner_rights(&ace->sid))
+			goto next_ace;
 
 		bits = le32_to_cpu(ace->access_req);
 		switch (ace->type) {
@@ -1627,8 +1647,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		case ACCESS_DENIED_CALLBACK_ACE_TYPE:
 			if (counted)
 				grp_denied |= ~grp_allowed & bits;
-			/* access_bits takes no group denials either */
-			if (!member)
+			/* MAXIMUM_ALLOWED takes no group denials either */
+			if (!(maximal && member))
 				denied |= ~access_bits & bits;
 			break;
 		}
@@ -1648,8 +1668,14 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		    (access_bits & GENERIC_READ_FLAGS) == GENERIC_READ_FLAGS)
 			access_bits |= FILE_EXECUTE;
 		granted = requested | access_bits;
+	} else if (!obj_deny) {
+		/* An explicit request takes the ordered result */
+		access_bits &= ~denied;
 	} else {
-		/* An explicit request uses only grp_granted and obj_deny */
+		/*
+		 * ksmbd does not parse object deny ACEs, so with one present an
+		 * explicit request keeps the answer of the ACE found below
+		 */
 		access_bits = 0;
 	}
 
@@ -1680,7 +1706,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		    !(ace->flags & INHERIT_ONLY_ACE))
 			or_ace = true;
 		if (!compare_sids(&sid_everyone, &ace->sid) ||
-		    !compare_sids(&sid_authusers, &ace->sid))
+		    (authusers &&
+		     !compare_sids(&sid_authusers, &ace->sid)))
 			others_ace = ace;
 
 		ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size));
@@ -1752,7 +1779,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		}
 	}
 
-	if (!maximal) {
+	if (!maximal && obj_deny) {
 		switch (ace->type) {
 		case ACCESS_ALLOWED_ACE_TYPE:
 			access_bits = le32_to_cpu(ace->access_req);
diff --git a/fs/smb/server/smbacl.h b/fs/smb/server/smbacl.h
index 3342cebab333..bbd35948a379 100644
--- a/fs/smb/server/smbacl.h
+++ b/fs/smb/server/smbacl.h
@@ -97,7 +97,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, const struct path *path,
 		     unsigned int uid, unsigned int gid);
 int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 			__le32 *pdaccess, __le32 raw_daccess, int uid,
-			bool strict);
+			bool authenticated, bool strict);
 int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
 		 const struct path *path, struct smb_ntsd *pntsd, int ntsd_len,
 		 bool type_check, bool get_write, bool posix_open,
diff --git a/fs/smb/server/tests/smbacl_kunit.c b/fs/smb/server/tests/smbacl_kunit.c
index 33496b4d31a3..bf28439a7696 100644
--- a/fs/smb/server/tests/smbacl_kunit.c
+++ b/fs/smb/server/tests/smbacl_kunit.c
@@ -229,7 +229,7 @@ static void ksmbd_smb_check_perm_dacl_boundary_test(struct kunit *test)
 		goto out;
 
 	rc = smb_check_perm_dacl(NULL, &file->f_path, &daccess,
-				 cpu_to_le32(FILE_READ_DATA), 0, false);
+				 cpu_to_le32(FILE_READ_DATA), 0, true, false);
 
 	/*
 	 * The post-boundary ACE (ACE #2, beyond pdacl->size) grants
@@ -271,7 +271,7 @@ ksmbd_smb_check_perm_dacl_maximal_boundary_test(struct kunit *test)
 		goto out;
 
 	rc = smb_check_perm_dacl(NULL, &file->f_path, &daccess,
-				 FILE_MAXIMAL_ACCESS_LE, 0, false);
+				 FILE_MAXIMAL_ACCESS_LE, 0, true, false);
 	KUNIT_EXPECT_EQ(test, 0, rc);
 	if (rc)
 		goto out;
-- 
2.47.3


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

* [PATCH v2 2/2] ksmbd: let the parent's DACL grant DELETE and FILE_READ_ATTRIBUTES
  2026-11-07 23:19 [PATCH v2 0/2] ksmbd: fix explicit DACL checks and parent-derived rights Ian Bridges
  2026-11-07 23:19 ` [PATCH v2 1/2] ksmbd: evaluate the DACL in order for explicit access requests Ian Bridges
@ 2026-11-07 23:19 ` Ian Bridges
  1 sibling, 0 replies; 3+ messages in thread
From: Ian Bridges @ 2026-11-07 23:19 UTC (permalink / raw)
  To: linkinjeon
  Cc: senozhatsky, tom, chenxiaosong, linux-cifs, linux-kernel,
	Ian Bridges, stable

A directory's FILE_DELETE_CHILD lets a caller delete the files in it,
and its FILE_LIST_DIRECTORY lets a caller list them with their
attributes. ksmbd checks only the file's DACL, so an open for DELETE
or FILE_READ_ATTRIBUTES can be refused even when the parent grants the
corresponding right. For example, this happens with an empty file DACL
under a parent that grants Everyone full access. Without DELETE on the
handle, the caller cannot rename the file or set its delete
disposition. A caller can also see a file's attributes in a directory
listing but be unable to open the file for FILE_READ_ATTRIBUTES.

When the file's DACL does not grant a requested DELETE or
FILE_READ_ATTRIBUTES, check the parent's DACL for FILE_DELETE_CHILD or
FILE_LIST_DIRECTORY respectively. Use exact SID matching, include group
allows and denies, exclude guest and anonymous sessions from
Authenticated Users, and fail closed on non-inherit-only object deny
ACEs. Do not look above the mount root. A missing or unreadable parent
DACL grants nothing.

In the parent check, ACEs for the caller's groups and OWNER RIGHTS
decide before the parent's POSIX ACL. Otherwise a POSIX ACL entry for
the caller decides whenever no ACE names the caller, Everyone or
Authenticated Users, and the rights that entry maps to never include
FILE_DELETE_CHILD.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    Plain MAXIMUM_ALLOWED requests and maximal-access replies get
    neither right from the parent. Deriving DELETE for them can make
    smb2_open() refuse opens that succeed today when POSIX denies the
    caller write access to the parent. FILE_READ_ATTRIBUTES is left
    out there too, so that both rights come from the parent only when
    the request names them.
    
    Because smb2_open() fails an open whose maximal-access reply is
    refused, a DELETE open requesting that reply can still fail when
    the file DACL grants nothing, as on the base. A DELETE open also
    still requires POSIX read access to the file.
    
    Other checks keep the POSIX ACL's precedence over ACEs for the
    caller's groups and OWNER RIGHTS, as on the base.

 fs/smb/server/smbacl.c | 100 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 17 deletions(-)

diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index f773ab8efb3f..b9a7fed33a66 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1494,6 +1494,11 @@ static bool smb_sid_is_owner_rights(const struct smb_sid *sid)
 	       !compare_sids(sid, &sid_owner_rights);
 }
 
+static unsigned int smb_parent_derived_rights(struct ksmbd_conn *conn,
+					      const struct path *path, int uid,
+					      bool authenticated,
+					      unsigned int want);
+
 int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 			__le32 *pdaccess, __le32 raw_daccess, int uid,
 			bool authenticated, bool strict)
@@ -1514,16 +1519,19 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 	unsigned int grp_allowed = 0, grp_denied = 0, bits;
 	struct smb_ace *others_ace = NULL;
 	bool or_ace = false, obj_deny = false, member, counted;
+	bool grp_ace = 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;
 	/*
-	 * MAXIMUM_ALLOWED requests match every session against
-	 * Authenticated Users.
+	 * MAXIMUM_ALLOWED requests that are not strict match every session
+	 * against Authenticated Users and do not count ACEs for the
+	 * caller's groups. Strict ones match SIDs as explicit requests do.
 	 */
-	bool authusers = authenticated || maximal;
+	bool open_max = maximal && !strict;
+	bool authusers = authenticated || open_max;
 	vfsuid_t vfsuid;
 
 	ksmbd_debug(SMB, "check permission using windows acl\n");
@@ -1557,13 +1565,16 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		/*
 		 * An empty (present, zero-ACE) DACL grants no access to
 		 * anyone except the object owner's implicit READ_CONTROL
-		 * and WRITE_DAC (MS-DTYP 2.4.5). Deny every other caller,
-		 * deny the owner any access beyond those two bits, and do
-		 * not let trailing bytes after a zero-ACE DACL become an
-		 * implicit grant.
+		 * and WRITE_DAC (MS-DTYP 2.5.3.2), besides what the parent
+		 * grants. Deny every other right, and do not let trailing
+		 * bytes after a zero-ACE DACL become an implicit grant.
 		 */
 		if (is_owner)
 			allowed = READ_CONTROL | WRITE_DAC;
+		if (!strict)
+			allowed |= smb_parent_derived_rights(conn, path, uid,
+							     authenticated,
+							     requested & ~allowed);
 		if (maximal)
 			requested |= allowed;
 		if (!allowed || requested & ~allowed)
@@ -1576,9 +1587,10 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 	/*
 	 * Evaluate the DACL in order (MS-DTYP 2.5.3.2). access_bits collects
 	 * the result for the ACEs that apply to the caller, without its groups
-	 * for MAXIMUM_ALLOWED. grp_granted collects what ACEs for the caller's
-	 * groups, or OWNER RIGHTS for the owner, grant. It is used when the
-	 * SID lookup and POSIX ACL fallback below do not decide.
+	 * for a MAXIMUM_ALLOWED request that is not strict. grp_granted
+	 * collects what ACEs for the caller's groups, or OWNER RIGHTS for the
+	 * owner, grant. It is used when the SID lookup and POSIX ACL fallback
+	 * below do not decide.
 	 */
 	ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
 	aces_size = pdacl_size - sizeof(struct smb_acl);
@@ -1623,8 +1635,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		     compare_sids(&sid_owner_rights, &ace->sid) &&
 		     !member))
 			goto next_ace;
-		/* An explicit request counts only SIDs that match exactly */
-		if (!maximal && !member &&
+		/* Other requests count only SIDs that match exactly */
+		if (!open_max && !member &&
 		    !smb_sid_equal(&sid, &ace->sid) &&
 		    !smb_sid_equal(&sid_everyone, &ace->sid) &&
 		    !(authusers && smb_sid_equal(&sid_authusers, &ace->sid)) &&
@@ -1634,10 +1646,11 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 		bits = le32_to_cpu(ace->access_req);
 		switch (ace->type) {
 		case ACCESS_ALLOWED_ACE_TYPE:
+			grp_ace |= counted;
 			if (counted)
 				grp_allowed |= bits;
-			/* MAXIMUM_ALLOWED takes no group grants */
-			if (maximal && member)
+			/* An open's MAXIMUM_ALLOWED takes no group grants */
+			if (open_max && member)
 				break;
 			if (counted)
 				grp_granted |= bits;
@@ -1645,10 +1658,11 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 			break;
 		case ACCESS_DENIED_ACE_TYPE:
 		case ACCESS_DENIED_CALLBACK_ACE_TYPE:
+			grp_ace |= counted;
 			if (counted)
 				grp_denied |= ~grp_allowed & bits;
-			/* MAXIMUM_ALLOWED takes no group denials either */
-			if (!(maximal && member))
+			/* nor group denials */
+			if (!(open_max && member))
 				denied |= ~access_bits & bits;
 			break;
 		}
@@ -1663,6 +1677,9 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 			access_bits |= READ_CONTROL | WRITE_DAC |
 				FILE_READ_ATTRIBUTES | DELETE;
 		access_bits &= ~denied;
+		/* An object deny ACE could deny the parent's rights */
+		if (!open_max && obj_deny)
+			access_bits = 0;
 		if ((raw_daccess & FILE_GENERIC_EXECUTE_LE) &&
 		    S_ISREG(d_inode(path->dentry)->i_mode) &&
 		    (access_bits & GENERIC_READ_FLAGS) == GENERIC_READ_FLAGS)
@@ -1715,7 +1732,13 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 
 	if (IS_ENABLED(CONFIG_FS_POSIX_ACL)) {
 		posix_acls = get_inode_acl(d_inode(path->dentry), ACL_TYPE_ACCESS);
-		if (!IS_ERR_OR_NULL(posix_acls) && !found && !others_ace) {
+		/*
+		 * For a strict MAXIMUM_ALLOWED request, ACEs for the caller's
+		 * groups or OWNER RIGHTS decide before the POSIX ACL, unless
+		 * the DACL has an object deny ACE, which ksmbd does not parse.
+		 */
+		if (!IS_ERR_OR_NULL(posix_acls) && !found && !others_ace &&
+		    !(strict && maximal && grp_ace && !obj_deny)) {
 			unsigned int id = -1;
 
 			pa_entry = posix_acls->a_entries;
@@ -1767,6 +1790,10 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 			 */
 			if (!obj_deny)
 				allowed |= grp_granted;
+			if (!strict)
+				allowed |= smb_parent_derived_rights(conn, path, uid,
+								     authenticated,
+								     requested & ~allowed);
 			if (maximal)
 				requested |= allowed;
 			if (!allowed || requested & ~allowed) {
@@ -1813,6 +1840,45 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 }
 EXPORT_SYMBOL_IF_KUNIT(smb_check_perm_dacl);
 
+/*
+ * Return the rights in @want that @path gets from its parent directory:
+ * DELETE if the parent's DACL grants FILE_DELETE_CHILD, and
+ * FILE_READ_ATTRIBUTES if it grants FILE_LIST_DIRECTORY
+ * (MS-FSA 2.1.5.1.2.1). A parent with no DACL, or with one that ksmbd
+ * cannot read, gives nothing. Callers pass only rights that the request
+ * names.
+ */
+static unsigned int smb_parent_derived_rights(struct ksmbd_conn *conn,
+					      const struct path *path, int uid,
+					      bool authenticated,
+					      unsigned int want)
+{
+	struct path parent = { .mnt = path->mnt };
+	__le32 granted = FILE_MAXIMAL_ACCESS_LE;
+	unsigned int rights = 0;
+	int rc;
+
+	if (!(want & (DELETE | FILE_READ_ATTRIBUTES)))
+		return 0;
+
+	/* The root of the mount, or a disconnected dentry, has no parent */
+	if (path->dentry == path->mnt->mnt_root || IS_ROOT(path->dentry))
+		return 0;
+
+	parent.dentry = dget_parent(path->dentry);
+	/* Strict, so the parent's own parent is not consulted */
+	rc = smb_check_perm_dacl(conn, &parent, &granted, 0, uid,
+				 authenticated, true);
+	dput(parent.dentry);
+	if (rc || granted == FILE_MAXIMAL_ACCESS_LE)
+		return 0;
+	if (granted & FILE_DELETE_CHILD_LE)
+		rights |= DELETE;
+	if (granted & FILE_LIST_DIRECTORY_LE)
+		rights |= FILE_READ_ATTRIBUTES;
+	return rights & want;
+}
+
 int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
 		 const struct path *path, struct smb_ntsd *pntsd, int ntsd_len,
 		 bool type_check, bool get_write, bool posix_open,
-- 
2.47.3


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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-07 23:19 [PATCH v2 0/2] ksmbd: fix explicit DACL checks and parent-derived rights Ian Bridges
2026-11-07 23:19 ` [PATCH v2 1/2] ksmbd: evaluate the DACL in order for explicit access requests Ian Bridges
2026-11-07 23:19 ` [PATCH v2 2/2] ksmbd: let the parent's DACL grant DELETE and FILE_READ_ATTRIBUTES Ian Bridges

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®