[PATCH] ksmbd: keep group denials when a POSIX ACL entry decides access

[PATCH] ksmbd: keep group denials when a POSIX ACL entry decides access

[Ian Bridges]

When no ACE names the caller, Everyone or Authenticated Users,
smb_check_perm_dacl() lets a POSIX ACL entry for the caller decide
access. The entry's rights replace the result of the DACL walk. Deny
ACEs for the caller's groups, and OWNER RIGHTS for the owner, are then
lost.

For example, a deny of FILE_WRITE_DATA for one of the caller's groups
should refuse writes. A POSIX ACL entry that gives the caller write
access still lets the caller write.

Remove the requested rights that those ACEs deny from the entry's
rights. The DACL walk already records them in grp_denied.

Fixes: 42ac1f1917ae ("ksmbd: grant a caller what its groups and OWNER RIGHTS ACEs allow")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261102200353.884509-1-icb%40fastmail.org
Link: https://sashiko.dev/#/patchset/20261103150458.11591-1-icb%40fastmail.org
Link: https://sashiko.dev/#/patchset/20261107062233.1579198-1-icb%40fastmail.org?part=1
Link: https://sashiko.dev/#/patchset/20261107231941.1824289-1-icb%40fastmail.org?part=1
Link: https://sashiko.dev/#/patchset/20261108182011.1888520-1-icb%40fastmail.org?part=1
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    A POSIX ACL entry that decides can still drop rights that ACEs for
    the caller's groups grant. The implicit allowances that every
    non-strict check adds are also unchanged.
    
    For MAXIMUM_ALLOWED, only the rights named in the request are
    checked against these denials. Group deny ACEs still do not limit
    the rights that MAXIMUM_ALLOWED adds, as on the other paths of this
    check.
    
    Tested with KASAN on ksmbd-for-next 4ab80fd4d1f1, and over SMB on a
    kernel whose fs/smb/server tree is identical to it. smbtorture
    results are unchanged.
    
    This patch was developed with LLM assistance.

 fs/smb/server/smbacl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 1b387c262d59..45f70a05e21f 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1764,6 +1764,15 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 					if (!access_bits)
 						access_bits =
 							SET_MINIMUM_RIGHTS;
+					/*
+					 * Only ACEs for the caller's groups,
+					 * or OWNER RIGHTS for the owner, apply
+					 * here. The POSIX ACL entry does not
+					 * grant an explicitly requested right
+					 * that they deny first in DACL order.
+					 */
+					access_bits &= ~(grp_denied &
+							 requested);
 					posix_acl_release(posix_acls);
 					goto check_access_bits;
 				}

base-commit: 4ab80fd4d1f1c4363b336ddb3d8818ea37c4997d
-- 
2.47.3