* [PATCH] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size
@ 2026-08-06 16:04 Hang Nan
2026-08-06 20:35 ` ChenXiaoSong
0 siblings, 1 reply; 7+ messages in thread
From: Hang Nan @ 2026-08-06 16:04 UTC (permalink / raw)
To: Namjae Jeon, Steve French; +Cc: linux-cifs, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size
2026-08-06 16:04 [PATCH] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size Hang Nan
@ 2026-08-06 20:35 ` ChenXiaoSong
2026-08-07 0:54 ` ChenXiaoSong
0 siblings, 1 reply; 7+ messages in thread
From: ChenXiaoSong @ 2026-08-06 20:35 UTC (permalink / raw)
To: Hang Nan, Namjae Jeon, Steve French; +Cc: linux-cifs, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size
2026-08-06 20:35 ` ChenXiaoSong
@ 2026-08-07 0:54 ` ChenXiaoSong
2026-08-12 10:39 ` [PATCH v2 0/2] " Hang Nan
[not found] ` <20260812103945.82495-1-2122295973@qq.com>
0 siblings, 2 replies; 7+ messages in thread
From: ChenXiaoSong @ 2026-08-07 0:54 UTC (permalink / raw)
To: Hang Nan, Namjae Jeon, Steve French; +Cc: linux-cifs, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 0/2] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size
2026-08-07 0:54 ` ChenXiaoSong
@ 2026-08-12 10:39 ` Hang Nan
[not found] ` <20260812103945.82495-1-2122295973@qq.com>
1 sibling, 0 replies; 7+ messages in thread
From: Hang Nan @ 2026-08-12 10:39 UTC (permalink / raw)
To: Namjae Jeon, Steve French; +Cc: linux-cifs, linux-kernel, ChenXiaoSong
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size
[not found] ` <20260812103945.82495-1-2122295973@qq.com>
@ 2026-08-12 10:39 ` Hang Nan
2026-08-12 10:39 ` [PATCH v2 2/2] ksmbd: add KUnit tests for the DACL declared-size boundary Hang Nan
1 sibling, 0 replies; 7+ messages in thread
From: Hang Nan @ 2026-08-12 10:39 UTC (permalink / raw)
To: Namjae Jeon, Steve French
Cc: linux-cifs, linux-kernel, ChenXiaoSong, Hang Nan
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 (KUnit, UML, x86_64, KASAN; first KUnit tests for
fs/smb/server, see follow-up patch):
- ksmbd_dacl_walk_must_stop_at_declared_size (semantic harness):
with the current code the ACE placed after pdacl->size is
selected and access is granted; with the fix the same sample
is denied (EACCES).
- ksmbd_smb_check_perm_dacl_boundary (drives the real function):
passes with the fix; fails with the fix reverted (rc == 0).
- ksmbd-for-next-next (rebased, base e9d76059ff03), 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 b5db6dcfbaa4..8ad2e5a5cca8 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1494,7 +1494,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
if (*pdaccess & FILE_MAXIMAL_ACCESS_LE) {
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)
@@ -1551,7 +1551,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);
+ 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.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] ksmbd: add KUnit tests for the DACL declared-size boundary
[not found] ` <20260812103945.82495-1-2122295973@qq.com>
2026-08-12 10:39 ` [PATCH v2 1/2] " Hang Nan
@ 2026-08-12 10:39 ` Hang Nan
2026-08-12 11:28 ` ChenXiaoSong
1 sibling, 1 reply; 7+ messages in thread
From: Hang Nan @ 2026-08-12 10:39 UTC (permalink / raw)
To: Namjae Jeon, Steve French
Cc: linux-cifs, linux-kernel, ChenXiaoSong, Hang Nan
From: nanhang <2122295973@qq.com>
fs/smb/server currently has no KUnit tests; add the first one,
covering smb_check_perm_dacl()'s DACL walk boundary.
smb_check_perm_dacl() walks the DACL ACE list to decide whether the
requested access is granted. The walk must stop at struct
smb_acl::size (the declared DACL size): a crafted DACL can place an
access-granting ACE beyond pdacl->size, and the pre-fix code selected
it because the walk used the enclosing security descriptor length
instead.
- ksmbd_dacl_walk_must_stop_at_declared_size: a pure semantic harness
modelling the walk; it shows 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 rejected with -EACCES.
Validated with KUnit (UML, x86_64, KASAN): with the fix applied both
tests pass; with the fix reverted, ksmbd_smb_check_perm_dacl_boundary
fails as expected (the post-boundary ACE is selected and access is
granted).
Signed-off-by: Hang Nan <2122295973@qq.com>
---
fs/smb/server/Kconfig | 13 ++
fs/smb/server/Makefile | 1 +
fs/smb/server/smbacl_kunit_test.c | 254 ++++++++++++++++++++++++++++++
3 files changed, 268 insertions(+)
create mode 100644 fs/smb/server/smbacl_kunit_test.c
diff --git a/fs/smb/server/Kconfig b/fs/smb/server/Kconfig
index 08d8b7a965a6..05d16052b9a7 100644
--- a/fs/smb/server/Kconfig
+++ b/fs/smb/server/Kconfig
@@ -72,3 +72,16 @@ config SMB_SERVER_KERBEROS5
bool "Support for Kerberos 5"
depends on SMB_SERVER
default y
+
+config SMB_SERVER_KUNIT_TEST
+ tristate "KUnit tests for SMB3 server helpers" if !KUNIT_ALL_TESTS
+ depends on SMB_SERVER && KUNIT && SHMEM
+ default KUNIT_ALL_TESTS
+
+ help
+ KUnit tests for ksmbd server helpers such as the DACL access
+ check in smb_check_perm_dacl(). This option is only useful
+ for kernel developers; enable it together with CONFIG_KUNIT.
+
+ For more information on KUnit and unit tests in the kernel,
+ please read Documentation/dev-tools/kunit/index.rst.
diff --git a/fs/smb/server/Makefile b/fs/smb/server/Makefile
index a3e9306055e8..dff9d3cd43bd 100644
--- a/fs/smb/server/Makefile
+++ b/fs/smb/server/Makefile
@@ -19,3 +19,4 @@ $(obj)/ksmbd_spnego_negtokentarg.asn1.o: $(obj)/ksmbd_spnego_negtokentarg.asn1.c
ksmbd-$(CONFIG_SMB_SERVER_SMBDIRECT) += transport_rdma.o
ksmbd-$(CONFIG_PROC_FS) += proc.o
+ksmbd-$(CONFIG_SMB_SERVER_KUNIT_TEST) += smbacl_kunit_test.o
diff --git a/fs/smb/server/smbacl_kunit_test.c b/fs/smb/server/smbacl_kunit_test.c
new file mode 100644
index 000000000000..34129708aef6
--- /dev/null
+++ b/fs/smb/server/smbacl_kunit_test.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for ksmbd security descriptor (DACL) handling.
+ *
+ * Copyright (C) 2026 Hang Nan <2122295973@qq.com>
+ *
+ * The tests pin the DACL declared-size boundary in smb_check_perm_dacl():
+ *
+ * - ksmbd_dacl_walk_must_stop_at_declared_size: a pure semantic harness
+ * that models the ACE walk. Walking to the end of the enclosing
+ * security descriptor (the pre-fix behaviour) selects an ACE that
+ * sits beyond struct smb_acl::size; stopping at the declared DACL
+ * size (the fixed behaviour) rejects it.
+ *
+ * - ksmbd_smb_check_perm_dacl_boundary: drives the real
+ * smb_check_perm_dacl() with a descriptor stored through ksmbd's own
+ * NTACL xattr path on a tmpfs file, and asserts that a post-boundary
+ * ACE is not selected (access denied with -EACCES).
+ */
+
+#include <kunit/test.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/shmem_fs.h>
+#include <linux/slab.h>
+
+#include "smbacl.h"
+#include "smb_common.h"
+#include "vfs.h"
+
+struct ksmbd_acl_walk_result {
+ bool found;
+ bool allowed;
+ const struct smb_ace *selected;
+};
+
+static const struct smb_sid test_nonmatching_sid = {
+ 1, 5, {0, 0, 0, 0, 0, 5},
+ { cpu_to_le32(21), cpu_to_le32(1), cpu_to_le32(2),
+ cpu_to_le32(3), cpu_to_le32(9999) }
+};
+
+/*
+ * S-1-22-1-0: the SID id_to_sid(0, SIDUNIX_USER) resolves to, i.e. what
+ * smb_check_perm_dacl() looks for when called with uid == 0.
+ */
+static const struct smb_sid test_owner_sid = {
+ 1, 2, {0, 0, 0, 0, 0, 22},
+ { cpu_to_le32(1), cpu_to_le32(0) }
+};
+
+static int test_compare_sids(const struct smb_sid *a, const struct smb_sid *b)
+{
+ int i;
+
+ if (a->revision != b->revision || a->num_subauth != b->num_subauth)
+ return 1;
+ for (i = 0; i < NUM_AUTHS; i++) {
+ if (a->authority[i] != b->authority[i])
+ return 1;
+ }
+ for (i = 0; i < a->num_subauth; i++) {
+ if (a->sub_auth[i] != b->sub_auth[i])
+ return 1;
+ }
+ return 0;
+}
+
+static u16 fill_test_ace(struct smb_ace *ace, const struct smb_sid *sid,
+ u32 access_req)
+{
+ u16 size = offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE +
+ sid->num_subauth * sizeof(__le32);
+
+ ace->type = ACCESS_ALLOWED_ACE_TYPE;
+ ace->flags = 0;
+ ace->size = cpu_to_le16(size);
+ ace->access_req = cpu_to_le32(access_req);
+ memcpy(&ace->sid, sid, size - offsetof(struct smb_ace, sid));
+ return size;
+}
+
+static struct ksmbd_acl_walk_result test_walk_dacl(struct smb_acl *pdacl,
+ int walk_boundary,
+ const struct smb_sid *target,
+ u32 requested)
+{
+ struct ksmbd_acl_walk_result result = {};
+ struct smb_ace *ace;
+ int aces_size;
+ int i;
+
+ ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
+ aces_size = walk_boundary - sizeof(struct smb_acl);
+ for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
+ u16 ace_size;
+
+ if (aces_size < offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE)
+ break;
+ ace_size = le16_to_cpu(ace->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 ||
+ ace_size < offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE +
+ sizeof(__le32) * ace->sid.num_subauth)
+ break;
+
+ if (!test_compare_sids(target, &ace->sid)) {
+ result.found = true;
+ result.selected = ace;
+ result.allowed = !(requested & ~le32_to_cpu(ace->access_req));
+ return result;
+ }
+
+ ace = (struct smb_ace *)((char *)ace + ace_size);
+ }
+
+ return result;
+}
+
+static void ksmbd_dacl_walk_must_stop_at_declared_size(struct kunit *test)
+{
+ struct ksmbd_acl_walk_result declared, enclosing;
+ struct smb_acl *acl;
+ struct smb_ace *ace1, *fake;
+ u16 ace1_size, fake_size;
+ u16 pdacl_size;
+ u16 acl_size;
+
+ acl = kunit_kzalloc(test, 128, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, acl);
+
+ acl->revision = cpu_to_le16(2);
+ acl->num_aces = cpu_to_le16(2);
+
+ ace1 = (struct smb_ace *)((char *)acl + sizeof(*acl));
+ ace1_size = fill_test_ace(ace1, &test_nonmatching_sid, 0);
+ fake = (struct smb_ace *)((char *)ace1 + ace1_size);
+ fake_size = fill_test_ace(fake, &test_owner_sid, FILE_READ_DATA);
+
+ pdacl_size = sizeof(*acl) + ace1_size;
+ acl_size = pdacl_size + fake_size;
+ acl->size = cpu_to_le16(pdacl_size);
+
+ declared = test_walk_dacl(acl, pdacl_size, &test_owner_sid,
+ FILE_READ_DATA);
+ enclosing = test_walk_dacl(acl, acl_size, &test_owner_sid,
+ FILE_READ_DATA);
+
+ KUNIT_EXPECT_FALSE(test, declared.found);
+ KUNIT_EXPECT_FALSE(test, declared.allowed);
+
+ /* Demonstrates that the buggy acl_size boundary selects fake ACE #2. */
+ KUNIT_EXPECT_TRUE(test, enclosing.found);
+ KUNIT_EXPECT_TRUE(test, enclosing.allowed);
+}
+
+#define TEST_ACE1_SIZE (offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE + \
+ 5 * sizeof(__le32))
+#define TEST_ACE2_SIZE (offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE + \
+ 2 * sizeof(__le32))
+#define TEST_DACL_SIZE (sizeof(struct smb_acl) + TEST_ACE1_SIZE)
+#define TEST_NTSD_SIZE (sizeof(struct smb_ntsd) + sizeof(struct smb_acl) + \
+ TEST_ACE1_SIZE + TEST_ACE2_SIZE)
+
+/*
+ * Build an NTSD whose DACL declares one ACE (pdacl->size) but actually
+ * contains two: the second ACE sits beyond the declared DACL boundary
+ * yet inside the enclosing security descriptor. It grants FILE_READ_DATA
+ * to S-1-22-1-0 (the caller's SID for uid == 0), so the pre-fix walk
+ * that used the descriptor length would select it and grant access.
+ */
+static struct smb_ntsd *build_boundary_ntsd(struct kunit *test)
+{
+ struct smb_ntsd *pntsd;
+ struct smb_acl *pdacl;
+ struct smb_ace *ace;
+
+ pntsd = kunit_kzalloc(test, TEST_NTSD_SIZE, GFP_KERNEL);
+ if (!pntsd)
+ return NULL;
+
+ pntsd->revision = cpu_to_le16(SD_REVISION);
+ pntsd->type = cpu_to_le16(DACL_PRESENT);
+ pntsd->dacloffset = cpu_to_le32(sizeof(struct smb_ntsd));
+
+ pdacl = (struct smb_acl *)((char *)pntsd + sizeof(struct smb_ntsd));
+ pdacl->revision = cpu_to_le16(2);
+ pdacl->num_aces = cpu_to_le16(2);
+ pdacl->size = cpu_to_le16(TEST_DACL_SIZE);
+
+ ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
+ fill_test_ace(ace, &test_nonmatching_sid, 0);
+
+ ace = (struct smb_ace *)((char *)ace + TEST_ACE1_SIZE);
+ fill_test_ace(ace, &test_owner_sid, FILE_READ_DATA);
+
+ return pntsd;
+}
+
+static void ksmbd_smb_check_perm_dacl_boundary_test(struct kunit *test)
+{
+ struct file *file;
+ struct smb_ntsd *pntsd;
+ __le32 daccess = cpu_to_le32(FILE_READ_DATA);
+ int rc;
+
+ file = shmem_file_setup("ksmbd-kunit-dacl", 0,
+ mk_vma_flags(VMA_NORESERVE_BIT));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, file);
+
+ pntsd = build_boundary_ntsd(test);
+ KUNIT_ASSERT_NOT_NULL(test, pntsd);
+
+ rc = ksmbd_vfs_set_sd_xattr(NULL, mnt_idmap(file->f_path.mnt),
+ &file->f_path, pntsd, TEST_NTSD_SIZE,
+ false);
+ KUNIT_EXPECT_EQ(test, 0, rc);
+ if (rc)
+ goto out;
+
+ rc = smb_check_perm_dacl(NULL, &file->f_path, &daccess,
+ cpu_to_le32(FILE_READ_DATA), 0, false);
+
+ /*
+ * The post-boundary ACE (ACE #2, beyond pdacl->size) grants
+ * FILE_READ_DATA to the caller's SID, but it must not be
+ * selected: the walk stops at the declared DACL size and access
+ * is denied. Before the fix the walk used the enclosing
+ * descriptor length, selected ACE #2 and returned 0.
+ */
+ KUNIT_EXPECT_EQ(test, -EACCES, rc);
+out:
+ fput(file);
+}
+
+static struct kunit_case ksmbd_smbacl_test_cases[] = {
+ KUNIT_CASE(ksmbd_dacl_walk_must_stop_at_declared_size),
+ KUNIT_CASE(ksmbd_smb_check_perm_dacl_boundary_test),
+ {}
+};
+
+static struct kunit_suite ksmbd_smbacl_test_suite = {
+ .name = "ksmbd-smbacl",
+ .test_cases = ksmbd_smbacl_test_cases,
+};
+
+kunit_test_suite(ksmbd_smbacl_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for ksmbd smbacl helpers");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] ksmbd: add KUnit tests for the DACL declared-size boundary
2026-08-12 10:39 ` [PATCH v2 2/2] ksmbd: add KUnit tests for the DACL declared-size boundary Hang Nan
@ 2026-08-12 11:28 ` ChenXiaoSong
0 siblings, 0 replies; 7+ messages in thread
From: ChenXiaoSong @ 2026-08-12 11:28 UTC (permalink / raw)
To: Hang Nan, Namjae Jeon, Steve French; +Cc: linux-cifs, linux-kernel
Thanks for your patches. So far, I have found the following two areas
that could be improved. I will finish the review as soon as possible.
On 8/12/26 18:39, Hang Nan wrote:
> diff --git a/fs/smb/server/Kconfig b/fs/smb/server/Kconfig
> index 08d8b7a965a6..05d16052b9a7 100644
> --- a/fs/smb/server/Kconfig
> +++ b/fs/smb/server/Kconfig
> +config SMB_SERVER_KUNIT_TEST
> + tristate "KUnit tests for SMB3 server helpers" if !KUNIT_ALL_TESTS
> + depends on SMB_SERVER && KUNIT && SHMEM
> + default KUNIT_ALL_TESTS
> +
It would be better to use `SMB_KUNIT_TESTS` directly. We do not need to
add a new config.
> diff --git a/fs/smb/server/Makefile b/fs/smb/server/Makefile
> index a3e9306055e8..dff9d3cd43bd 100644
> --- a/fs/smb/server/Makefile
> +++ b/fs/smb/server/Makefile
> +ksmbd-$(CONFIG_SMB_SERVER_KUNIT_TEST) += smbacl_kunit_test.o
obj-$(CONFIG_SMB_KUNIT_TESTS) += smbacl_kunit_test.o
The kunit tests should be built as a separate .ko instead of being
linked into ksmbd.ko.
--
ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-12 11:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-06 16:04 [PATCH] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size Hang Nan
2026-08-06 20:35 ` ChenXiaoSong
2026-08-07 0:54 ` ChenXiaoSong
2026-08-12 10:39 ` [PATCH v2 0/2] " Hang Nan
[not found] ` <20260812103945.82495-1-2122295973@qq.com>
2026-08-12 10:39 ` [PATCH v2 1/2] " Hang Nan
2026-08-12 10:39 ` [PATCH v2 2/2] ksmbd: add KUnit tests for the DACL declared-size boundary Hang Nan
2026-08-12 11:28 ` ChenXiaoSong
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®