ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size

smb_check_perm_dacl() validates that the DACL fits inside the NT
security descriptor, but then bounds its two ACE walks by the
remaining NTSD length (acl_size) rather than the DACL's declared
size (pdacl_size).

When pdacl->size is smaller than the trailing NTSD buffer, bytes
after the declared DACL boundary - still inside the stored security
descriptor - are parsed as ACEs during access checks.  A crafted
DACL can place an access-granting ACE beyond pdacl->size, and the
current code accepts it during SMB2_CREATE access validation, while
parse_dacl() and smb_inherit_dacl() stop at pdacl_size.

Bound both ACE walks by pdacl_size to match the DACL boundary
semantics used elsewhere in the server.

Validation:
- semantic KUnit harness shows the post-boundary ACE is selected
  before the fix and rejected (EACCES) after it
- linux master (7.2-rc6), x86_64

Fixes: 8f0541186e9a ("ksmbd: fix heap-based overflow in set_ntacl_dacl()")
Signed-off-by: Hang Nan <2122295973@qq.com>
---
 fs/smb/server/smbacl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index c13f07a09ab8..429ded811f51 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1484,7 +1484,7 @@
 			DELETE;
 
 		ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
-		aces_size = acl_size - sizeof(struct smb_acl);
+		aces_size = pdacl_size - sizeof(struct smb_acl);
 		for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
 			if (aces_size < offsetof(struct smb_ace, sid) +
 			    CIFS_SID_BASE_SIZE)
@@ -1505,7 +1505,7 @@
 	id_to_sid(uid, sid_type, &sid);
 
 	ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
-	aces_size = acl_size - sizeof(struct smb_acl);
+	aces_size = pdacl_size - sizeof(struct smb_acl);
 	for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
 		if (aces_size < offsetof(struct smb_ace, sid) +
 		    CIFS_SID_BASE_SIZE)

-- 
2.47.0

Hi Hang,

Thanks for your patch. Please rebase it on ksmbd-for-next-next branch.
: https://github.com/smfrench/smb3-kernel/commits/ksmbd-for-next-next/

Could you share the semantic KUnit test harness?

在 2026/8/7 0:04, Hang Nan 写道:
> Validation:
> - semantic KUnit harness shows the post-boundary ACE is selected
>    before the fix and rejected (EACCES) after it
> - linux master (7.2-rc6), x86_64

-- 
ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en

There are not any KUnit tests in fs/smb/server. It would be great if you 
could submit the first KUnit test.

On 8/7/26 04:35, ChenXiaoSong wrote:
> Hi Hang,
> 
> Thanks for your patch. Please rebase it on ksmbd-for-next-next branch.
> : https://github.com/smfrench/smb3-kernel/commits/ksmbd-for-next-next/
> 
> Could you share the semantic KUnit test harness?
> 
> 在 2026/8/7 0:04, Hang Nan 写道:
>> Validation:
>> - semantic KUnit harness shows the post-boundary ACE is selected
>>    before the fix and rejected (EACCES) after it
>> - linux master (7.2-rc6), x86_64
> 

-- 
ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en

Hi ChenXiaoSong,

Thanks for the review. All three points are addressed:

1. The patch is rebased onto the current ksmbd-for-next-next
   (base e9d76059ff03 "smb: server: Clear sensitive stack and heap
   data in auth.c", 2026-08-11).  Rebased v2: patch 1/2.

2. The semantic KUnit harness is now the first KUnit test for
   fs/smb/server (patch 2/2), as you suggested.  It contains two
   tests in fs/smb/server/smbacl_kunit_test.c:

   - ksmbd_dacl_walk_must_stop_at_declared_size: the pure semantic
     harness used for the validation quoted in your mail.  It models
     the ACE walk and pins the invariant that the walk stops at
     struct smb_acl::size -- the post-boundary ACE is selected with
     the old (enclosing descriptor length) boundary and rejected with
     the declared-size boundary.

   - ksmbd_smb_check_perm_dacl_boundary: drives the real
     smb_check_perm_dacl() with a crafted descriptor stored through
     ksmbd's own NTACL xattr path on a tmpfs file, and asserts the
     post-boundary ACE is denied with -EACCES.  With the fix reverted
     this test fails (rc == 0, access granted), so it guards the
     boundary fix itself rather than only a model of it.

3. Validation (KUnit, UML, x86_64, KASAN, CONFIG_SMB_SERVER_KUNIT_TEST=y):

   with the fix:    ksmbd-smbacl: pass 2, fail 0
   fix reverted:    ksmbd_smb_check_perm_dacl_boundary_test FAILS
                    (expected -EACCES, got rc == 0)

Happy to split the harness into a separate RFC or adjust anything
else.

Thanks,
Hang

--
Hang Nan <2122295973@qq.com>