- From a450dbbed037fc6fbe1781c773b40ae0cb276c2f Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Wed, 30 Jul 2025 17:23:01 +0200
- Subject: [PATCH 1/3] daemon: ACL code should stop doing |strlen()| on const
- string literals
- ACL code should stop doing |strlen()| on const string literals,
- this eats unneccesary CPU time.
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- daemon/acl.c | 116 ++++++++++++++++++++++++++++++-------------
- daemon/nfs41_const.h | 22 ++++++++
- 2 files changed, 104 insertions(+), 34 deletions(-)
- diff --git a/daemon/acl.c b/daemon/acl.c
- index 767233b..99e1add 100644
- --- a/daemon/acl.c
- +++ b/daemon/acl.c
- @@ -91,30 +91,38 @@ static void free_sids(PSID *sids, int count)
- free(sids);
- }
- -static int check_4_special_identifiers(char *who, PSID *sid, DWORD *sid_len,
- - BOOLEAN *flag)
- +static int check_4_special_identifiers(const char *restrict who,
- + PSID *sid,
- + DWORD *sid_len,
- + BOOLEAN *flag)
- {
- int status = ERROR_SUCCESS;
- WELL_KNOWN_SID_TYPE type = 0;
- *flag = TRUE;
- - if (!strncmp(who, ACE4_OWNER, strlen(ACE4_OWNER)-1))
- +
- + /*
- + * Compare |who| against known constant strings, excluding the '@'
- + * symbol. Note that |ACE4_NOBODY| and |ACE4_WIN_NULL_SID| do not
- + * have a '@
- + */
- + if (!strncmp(who, ACE4_OWNER, ACE4_OWNER_LEN-1))
- type = WinCreatorOwnerSid;
- #ifdef NFS41_DRIVER_WS2022_HACKS
- - else if (!strncmp(who, "CREATOR OWNER@", strlen("CREATOR OWNER@")-1))
- + else if (!strncmp(who, ACE4_WIN_CREATOR_OWNER, ACE4_WIN_CREATOR_OWNER_LEN-1))
- type = WinCreatorOwnerSid;
- #endif /* NFS41_DRIVER_WS2022_HACKS */
- - else if (!strncmp(who, ACE4_GROUP, strlen(ACE4_GROUP)-1))
- + else if (!strncmp(who, ACE4_GROUP, ACE4_GROUP_LEN-1))
- type = WinCreatorGroupSid;
- - else if (!strncmp(who, ACE4_EVERYONE, strlen(ACE4_EVERYONE)-1))
- + else if (!strncmp(who, ACE4_EVERYONE, ACE4_EVERYONE_LEN-1))
- type = WinWorldSid;
- #ifdef NFS41_DRIVER_WS2022_HACKS
- - else if (!strncmp(who, "Everyone@", strlen("Everyone@")-1))
- + else if (!strncmp(who, ACE4_WIN_EVERYONE, ACE4_WIN_EVERYONE_LEN-1))
- type = WinWorldSid;
- #endif /* NFS41_DRIVER_WS2022_HACKS */
- - else if (!strncmp(who, ACE4_NOBODY, strlen(ACE4_NOBODY)))
- + else if (!strncmp(who, ACE4_NOBODY, ACE4_NOBODY_LEN))
- type = WinNullSid;
- #ifdef NFS41_DRIVER_WS2022_HACKS
- - else if (!strncmp(who, "NULL SID", strlen("NULL SID")))
- + else if (!strncmp(who, ACE4_WIN_NULL_SID, ACE4_WIN_NULL_SID_LEN))
- type = WinNullSid;
- #endif /* NFS41_DRIVER_WS2022_HACKS */
- else
- @@ -523,55 +531,95 @@ out:
- static int is_well_known_sid(PSID sid, char *who, SID_NAME_USE *snu_out)
- {
- - int status, i;
- - for (i = 0; i < 78; i++) {
- - status = IsWellKnownSid(sid, (WELL_KNOWN_SID_TYPE)i);
- - if (!status) continue;
- - else {
- - DPRINTF(ACLLVL3, ("WELL_KNOWN_SID_TYPE %d\n", i));
- - switch((WELL_KNOWN_SID_TYPE)i) {
- + const WELL_KNOWN_SID_TYPE test_types[] = {
- + WinCreatorOwnerSid,
- + WinCreatorGroupSid,
- + WinBuiltinUsersSid,
- + WinNullSid,
- + WinAnonymousSid,
- + WinWorldSid,
- + WinAuthenticatedUserSid,
- + WinDialupSid,
- + WinNetworkSid,
- + WinBatchSid,
- + WinInteractiveSid,
- + WinNetworkServiceSid,
- + WinLocalServiceSid,
- + WinServiceSid
- + };
- + const size_t test_types_count = ARRAYSIZE(test_types);
- +
- + BOOL ismatch;
- + size_t i;
- +
- +#ifdef _DEBUG
- + static bool once = true;
- +
- + if (once) {
- + once = false;
- + EASSERT(test_types_count == 14);
- + /* Safeguards if someone tampers with the #defines for this */
- + EASSERT(strlen(ACE4_OWNER) == ACE4_OWNER_LEN);
- + EASSERT(strlen(ACE4_GROUP) == ACE4_GROUP_LEN);
- + EASSERT(strlen(ACE4_NOBODY) == ACE4_NOBODY_LEN);
- + EASSERT(strlen(ACE4_ANONYMOUS) == ACE4_ANONYMOUS_LEN);
- + EASSERT(strlen(ACE4_EVERYONE) == ACE4_EVERYONE_LEN);
- + }
- +#endif /* _DEBUG */
- +
- + for (i = 0; i < test_types_count ; i++) {
- + WELL_KNOWN_SID_TYPE tt = test_types[i];
- +
- + ismatch = IsWellKnownSid(sid, tt);
- + if (!ismatch) {
- + continue;
- + }
- +
- + DPRINTF(ACLLVL3, ("WELL_KNOWN_SID_TYPE=%d\n", (int)tt));
- + switch(tt) {
- case WinCreatorOwnerSid:
- - memcpy(who, ACE4_OWNER, strlen(ACE4_OWNER)+1);
- + (void)memcpy(who, ACE4_OWNER, ACE4_OWNER_LEN+1);
- *snu_out = SidTypeUser;
- return TRUE;
- case WinCreatorGroupSid:
- case WinBuiltinUsersSid:
- - memcpy(who, ACE4_GROUP, strlen(ACE4_GROUP)+1);
- + (void)memcpy(who, ACE4_GROUP, ACE4_GROUP_LEN+1);
- *snu_out = SidTypeGroup;
- return TRUE;
- case WinNullSid:
- - memcpy(who, ACE4_NOBODY, strlen(ACE4_NOBODY)+1);
- + (void)memcpy(who, ACE4_NOBODY, ACE4_NOBODY_LEN+1);
- *snu_out = SidTypeUser;
- return TRUE;
- case WinAnonymousSid:
- - memcpy(who, ACE4_ANONYMOUS, strlen(ACE4_ANONYMOUS)+1);
- + (void)memcpy(who, ACE4_ANONYMOUS, ACE4_ANONYMOUS_LEN+1);
- return TRUE;
- case WinWorldSid:
- - memcpy(who, ACE4_EVERYONE, strlen(ACE4_EVERYONE)+1);
- + (void)memcpy(who, ACE4_EVERYONE, ACE4_EVERYONE_LEN+1);
- *snu_out = SidTypeGroup;
- return TRUE;
- case WinAuthenticatedUserSid:
- - memcpy(who, ACE4_AUTHENTICATED, strlen(ACE4_AUTHENTICATED)+1);
- + (void)memcpy(who, ACE4_AUTHENTICATED, ACE4_AUTHENTICATED_LEN+1);
- return TRUE;
- case WinDialupSid:
- - memcpy(who, ACE4_DIALUP, strlen(ACE4_DIALUP)+1);
- + (void)memcpy(who, ACE4_DIALUP, ACE4_DIALUP_LEN+1);
- return TRUE;
- case WinNetworkSid:
- - memcpy(who, ACE4_NETWORK, strlen(ACE4_NETWORK)+1);
- + (void)memcpy(who, ACE4_NETWORK, ACE4_NETWORK_LEN+1);
- return TRUE;
- case WinBatchSid:
- - memcpy(who, ACE4_BATCH, strlen(ACE4_BATCH)+1);
- + (void)memcpy(who, ACE4_BATCH, ACE4_BATCH_LEN+1);
- return TRUE;
- case WinInteractiveSid:
- - memcpy(who, ACE4_INTERACTIVE, strlen(ACE4_INTERACTIVE)+1);
- + (void)memcpy(who, ACE4_INTERACTIVE, ACE4_INTERACTIVE_LEN+1);
- return TRUE;
- case WinNetworkServiceSid:
- case WinLocalServiceSid:
- case WinServiceSid:
- - memcpy(who, ACE4_SERVICE, strlen(ACE4_SERVICE)+1);
- + (void)memcpy(who, ACE4_SERVICE, ACE4_SERVICE_LEN+1);
- return TRUE;
- - default: return FALSE;
- - }
- + default:
- + eprintf("is_well_known_sid: unknown tt=%d\n", (int)tt);
- + return FALSE;
- }
- }
- return FALSE;
- @@ -939,7 +987,7 @@ int map_sid2nfs4ace_who(PSID sid, PSID owner_sid, PSID group_sid,
- if (owner_sid) {
- if (EqualSid(sid, owner_sid)) {
- DPRINTF(ACLLVL2, ("this is owner's sid\n"));
- - memcpy(who_out, ACE4_OWNER, strlen(ACE4_OWNER)+1);
- + (void)memcpy(who_out, ACE4_OWNER, ACE4_OWNER_LEN+1);
- sid_type = SidTypeUser;
- status = ERROR_SUCCESS;
- goto out;
- @@ -948,7 +996,7 @@ int map_sid2nfs4ace_who(PSID sid, PSID owner_sid, PSID group_sid,
- if (group_sid) {
- if (EqualSid(sid, group_sid)) {
- DPRINTF(ACLLVL2, ("this is group's sid\n"));
- - memcpy(who_out, ACE4_GROUP, strlen(ACE4_GROUP)+1);
- + memcpy(who_out, ACE4_GROUP, ACE4_GROUP_LEN+1);
- sid_type = SidTypeGroup;
- status = ERROR_SUCCESS;
- goto out;
- @@ -956,8 +1004,8 @@ int map_sid2nfs4ace_who(PSID sid, PSID owner_sid, PSID group_sid,
- }
- status = is_well_known_sid(sid, who_out, &sid_type);
- if (status) {
- - if (!strncmp(who_out, ACE4_NOBODY, strlen(ACE4_NOBODY))) {
- - who_size = (DWORD)strlen(ACE4_NOBODY);
- + if (!strncmp(who_out, ACE4_NOBODY, ACE4_NOBODY_LEN)) {
- + who_size = (DWORD)ACE4_NOBODY_LEN;
- goto add_domain;
- }
- @@ -1160,7 +1208,7 @@ static int map_dacl_2_nfs4acl(PACL acl, PSID sid, PSID gsid, nfsacl41 *nfs4_acl,
- goto out;
- }
- nfs4_acl->flag = 0;
- - memcpy(nfs4_acl->aces->who, ACE4_EVERYONE, strlen(ACE4_EVERYONE)+1);
- + (void)memcpy(nfs4_acl->aces->who, ACE4_EVERYONE, ACE4_EVERYONE_LEN+1);
- nfs4_acl->aces->acetype = ACE4_ACCESS_ALLOWED_ACE_TYPE;
- if (file_type == NF4DIR) {
- diff --git a/daemon/nfs41_const.h b/daemon/nfs41_const.h
- index 84b49b1..efb573a 100644
- --- a/daemon/nfs41_const.h
- +++ b/daemon/nfs41_const.h
- @@ -468,16 +468,38 @@ enum nfs_ftype4 {
- /* ACLS well-defined WHOs */
- #define ACE4_OWNER "OWNER@"
- +#define ACE4_OWNER_LEN (sizeof(ACE4_OWNER)-1)
- #define ACE4_GROUP "GROUP@"
- +#define ACE4_GROUP_LEN (sizeof(ACE4_GROUP)-1)
- #define ACE4_EVERYONE "EVERYONE@"
- +#define ACE4_EVERYONE_LEN (sizeof(ACE4_EVERYONE)-1)
- #define ACE4_INTERACTIVE "INTERACTIVE@"
- +#define ACE4_INTERACTIVE_LEN (sizeof(ACE4_INTERACTIVE)-1)
- #define ACE4_NETWORK "NETWORK@"
- +#define ACE4_NETWORK_LEN (sizeof(ACE4_NETWORK)-1)
- #define ACE4_DIALUP "DIALUP@"
- +#define ACE4_DIALUP_LEN (sizeof(ACE4_DIALUP)-1)
- #define ACE4_BATCH "BATCH@"
- +#define ACE4_BATCH_LEN (sizeof(ACE4_BATCH)-1)
- #define ACE4_ANONYMOUS "ANONYMOUS@"
- +#define ACE4_ANONYMOUS_LEN (sizeof(ACE4_ANONYMOUS)-1)
- #define ACE4_AUTHENTICATED "AUTHENTICATED@"
- +#define ACE4_AUTHENTICATED_LEN (sizeof(ACE4_AUTHENTICATED)-1)
- #define ACE4_SERVICE "SERVICE@"
- +#define ACE4_SERVICE_LEN (sizeof(ACE4_SERVICE)-1)
- #define ACE4_NOBODY "nobody"
- +#define ACE4_NOBODY_LEN (sizeof(ACE4_NOBODY)-1)
- +
- +#ifdef NFS41_DRIVER_WS2022_HACKS
- +/* Names used by Microsoft Windows 2019/2022 NFSv4.1 server */
- +#define ACE4_WIN_CREATOR_OWNER "CREATOR OWNER@"
- +#define ACE4_WIN_CREATOR_OWNER_LEN (sizeof(ACE4_WIN_CREATOR_OWNER)-1)
- +#define ACE4_WIN_EVERYONE "Everyone@"
- +#define ACE4_WIN_EVERYONE_LEN (sizeof(ACE4_WIN_EVERYONE)-1)
- +#define ACE4_WIN_NULL_SID "NULL_SID"
- +#define ACE4_WIN_NULL_SID_LEN (sizeof(ACE4_WIN_NULL_SID)-1)
- +#endif /* NFS41_DRIVER_WS2022_HACKS */
- +
- /* ACLE nfsacl41 aclflag4 constants */
- #define ACL4_AUTO_INHERIT 0x00000001
- --
- 2.45.1
- From ca0c172c3a60de5ac4275d179683224d6cced201 Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Wed, 30 Jul 2025 17:58:04 +0200
- Subject: [PATCH 2/3] daemon: Add better debug output for ACE failures
- Add better debug output for ACE failures.
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- daemon/acl.c | 22 ++++++++++++++--------
- 1 file changed, 14 insertions(+), 8 deletions(-)
- diff --git a/daemon/acl.c b/daemon/acl.c
- index 99e1add..56d297a 100644
- --- a/daemon/acl.c
- +++ b/daemon/acl.c
- @@ -272,23 +272,29 @@ static int convert_nfs4acl_2_dacl(nfs41_daemon_globals *nfs41dg,
- win_aceflags, mask, sids[win_i]);
- if (!status) {
- eprintf("convert_nfs4acl_2_dacl: "
- - "AddAccessAllowedAceEx(dacl=0x%p,win_aceflags=0x%x,mask=0x%x) failed "
- - "with status=%d\n",
- - dacl, (int)win_aceflags, (int)mask, status);
- + "AddAccessAllowedAceEx"
- + "(dacl=0x%p,win_aceflags=0x%x,mask=0x%x,who='%s') "
- + "failed with lasterr=%d\n",
- + dacl, (int)win_aceflags, (int)mask,
- + curr_nfsace->who, (int)GetLastError());
- + status = ERROR_INTERNAL_ERROR;
- goto out_free_dacl;
- }
- - else status = ERROR_SUCCESS;
- + status = ERROR_SUCCESS;
- } else if (curr_nfsace->acetype == ACE4_ACCESS_DENIED_ACE_TYPE) {
- status = AddAccessDeniedAceEx(dacl, ACL_REVISION,
- win_aceflags, mask, sids[win_i]);
- if (!status) {
- eprintf("convert_nfs4acl_2_dacl: "
- - "AddAccessDeniedAceEx(dacl=0x%p,win_aceflags=0x%x,mask=0x%x) failed "
- - "with status=%d\n",
- - dacl, (int)win_aceflags, (int)mask, status);
- + "AddAccessDeniedAceEx"
- + "(dacl=0x%p,win_aceflags=0x%x,mask=0x%x,who='%s') "
- + "failed with lasterr=%d\n",
- + dacl, (int)win_aceflags, (int)mask,
- + curr_nfsace->who, (int)GetLastError());
- + status = ERROR_INTERNAL_ERROR;
- goto out_free_dacl;
- }
- - else status = ERROR_SUCCESS;
- + status = ERROR_SUCCESS;
- } else {
- eprintf("convert_nfs4acl_2_dacl: unknown acetype %d\n",
- curr_nfsace->acetype);
- --
- 2.45.1
- From 32e74db9518defa4770408ae569566814c75d80f Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Wed, 30 Jul 2025 18:20:44 +0200
- Subject: [PATCH 3/3] daemon: Maximum Win32 path component length should be
- 255, not 256
- Maximum Win32 path component length should be 255 characters, not
- 256 characters.
- Win32 only supports up to 255, anything higher will cause subtle
- (because we were only 1 character over the limit) API breakage.
- Reported-by: Lionel Cons <Lionelcons1972@gmail.com>
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- daemon/name_cache.c | 6 +++---
- daemon/nfs41_const.h | 2 +-
- daemon/readdir.c | 6 +++---
- daemon/util.c | 4 ++--
- 4 files changed, 9 insertions(+), 9 deletions(-)
- diff --git a/daemon/name_cache.c b/daemon/name_cache.c
- index 950c188..14295cc 100644
- --- a/daemon/name_cache.c
- +++ b/daemon/name_cache.c
- @@ -504,7 +504,7 @@ static void copy_attrs(
- /* name cache */
- RB_HEAD(name_tree, name_cache_entry);
- struct name_cache_entry {
- - char component[NFS41_MAX_COMPONENT_LEN];
- + char component[NFS41_MAX_COMPONENT_LEN+1];
- nfs41_fh fh;
- RB_ENTRY(name_cache_entry) rbnode;
- struct name_tree rbchildren;
- @@ -552,7 +552,7 @@ static __inline void name_cache_entry_rename(
- OUT struct name_cache_entry *entry,
- IN const nfs41_component *component)
- {
- - StringCchCopyNA(entry->component, NFS41_MAX_COMPONENT_LEN,
- + StringCchCopyNA(entry->component, NFS41_MAX_COMPONENT_LEN+1,
- component->name, component->len);
- entry->component_len = component->len;
- }
- @@ -752,7 +752,7 @@ static struct name_cache_entry* name_cache_search(
- DPRINTF(NCLVL2, ("--> name_cache_search('%.*s' under '%s')\n",
- component->len, component->name, parent->component));
- - StringCchCopyNA(tmp.component, NFS41_MAX_COMPONENT_LEN,
- + StringCchCopyNA(tmp.component, NFS41_MAX_COMPONENT_LEN+1,
- component->name, component->len);
- tmp.component_len = component->len;
- diff --git a/daemon/nfs41_const.h b/daemon/nfs41_const.h
- index efb573a..74ad31d 100644
- --- a/daemon/nfs41_const.h
- +++ b/daemon/nfs41_const.h
- @@ -84,7 +84,7 @@
- * NFS41_MAX_COMPONENT_LEN - MaximumComponentNameLength
- * reported for FileFsAttributeInformation
- */
- -#define NFS41_MAX_COMPONENT_LEN 256
- +#define NFS41_MAX_COMPONENT_LEN 255
- /*
- * NFS41_MAX_PATH_LEN - Maximum path length
- * Notes:
- diff --git a/daemon/readdir.c b/daemon/readdir.c
- index ae19d43..f809c9d 100644
- --- a/daemon/readdir.c
- +++ b/daemon/readdir.c
- @@ -754,7 +754,7 @@ static int handle_readdir(void *deamon_context, nfs41_upcall *upcall)
- bool_t eof;
- /* make sure we allocate enough space for one nfs41_readdir_entry */
- const uint32_t max_buf_len = max(args->buf_len,
- - sizeof(nfs41_readdir_entry) + NFS41_MAX_COMPONENT_LEN);
- + sizeof(nfs41_readdir_entry) + NFS41_MAX_COMPONENT_LEN+1);
- DPRINTF(1, ("--> handle_nfs41_dirquery(filter='%s',initial=%d,restart=%d,single=%d)\n",
- args->filter, (int)args->initial, (int)args->restart, (int)args->single));
- @@ -836,9 +836,9 @@ fetch_entries:
- nfs41_readdir_entry *entry = (nfs41_readdir_entry*)entry_buf;
- entry->cookie = 0;
- entry->name_len = (uint32_t)strlen(args->filter) + 1;
- - if (entry->name_len >= NFS41_MAX_COMPONENT_LEN) {
- + if (entry->name_len >= (NFS41_MAX_COMPONENT_LEN+1)) {
- DPRINTF(1,
- - ("entry->name_len(=%d) >= NFS41_MAX_COMPONENT_LEN\n",
- + ("entry->name_len(=%d) >= (NFS41_MAX_COMPONENT_LEN+1)\n",
- (int)entry->name_len));
- status = ERROR_FILENAME_EXCED_RANGE;
- goto out_free_cookie;
- diff --git a/daemon/util.c b/daemon/util.c
- index bf99ac1..aac239d 100644
- --- a/daemon/util.c
- +++ b/daemon/util.c
- @@ -350,7 +350,7 @@ int create_silly_rename(
- #define SILLY_RENAME_PREPOSTFIX_LEN (6L)
- const unsigned short extra_len =
- SILLY_RENAME_PREPOSTFIX_LEN + MD5_HASH_LEN;
- - char name[NFS41_MAX_COMPONENT_LEN+1];
- + char name[NFS41_MAX_COMPONENT_LEN];
- unsigned char fhmd5[MD5_HASH_LEN+1];
- char *tmp;
- int status = NO_ERROR, i;
- @@ -395,7 +395,7 @@ int create_silly_rename(
- }
- last_component(path->path, path->path + path->len, silly);
- - (void)StringCchCopyNA(name, NFS41_MAX_COMPONENT_LEN+1,
- + (void)StringCchCopyNA(name, NFS41_MAX_COMPONENT_LEN,
- silly->name, silly->len);
- tmp = (char*)silly->name;
- --
- 2.45.1
msnfs41client: Patches for ACLs+maximum path component length+misc, 2025-07-30
Posted by Anonymous on Wed 30th Jul 2025 19:08
raw | new post
Submit a correction or amendment below (click here to make a fresh posting)
After submitting an amendment, you'll be able to view the differences between the old and new posts easily.