Home
My Posts

Archive
Login
Signup

Recent Posts

Free subdomains

Want your own xyz.kpaste.net sub-domain for your community? Just type the address into your browser address bar (foo.xyz.kpaste.net is also supported).

Suggestions/Bugs?

Email us at . Try to include as much info as possible.

About

Pastebin is a tool for collaborative debugging or editing.

 

msnfs41client: Patches for |get_token_*name()| SID cache, disable broken 8dot3 shortname generation, performance optimisations, misc, 2024-04-29
Posted by Anonymous on Mon 29th Apr 2024 15:39
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.

Syntax highlighting: None Bash C C++ CSS Diff HTML mIRC Script Perl PHP Python Smarty XML ---------------------------- ABAP Actionscript ADA Apache Log AppleScript APT sources.list ASM ASP AutoIT Bash C C# C++ C++ (with QT) CSS Diff HTML INI (Config Files) Java Javascript Lua Make mIRC Script MySQL NSIS Perl PHP Python Q(uick)BASIC robots Ruby Ruby on Rails SDLBasic Smarty SQL TCL Text VB.NET Visual BASIC Visual Fox Pro Visual Prolog Windows Registry Files XML Xorg.conf

To highlight particular lines, prefix each line with {%HIGHLIGHT}
From ebf00739d84a205a0d5567258cbddac0f6a8e0a5 Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 12:44:54 +0200 Subject: [PATCH 1/7] daemon: |dprintf_out()| should protect itself from recursion |dprintf_out()| should protect itself from recursion, in case we call |DPRINTF()| from functions which are used by |dprintf_out()| (e.g. |get_token_user_name(), |get_token_primarygroup_name()| etc.). Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- daemon/daemon_debug.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/daemon/daemon_debug.c b/daemon/daemon_debug.c index ffc6ee2..183a398 100644 --- a/daemon/daemon_debug.c +++ b/daemon/daemon_debug.c @@ -85,6 +85,8 @@ void dprintf_out(LPCSTR format, ...) HANDLE tok; const char *tok_src; bool free_tok = false; + /* |in_dprintf_out| - protect against recursive calls */ + __declspec(thread) static bool in_dprintf_out = false; if (OpenThreadToken(GetCurrentThread(), TOKEN_QUERY, FALSE, &tok)) { tok_src = "impersonated_user"; @@ -102,11 +104,21 @@ void dprintf_out(LPCSTR format, ...) tok = GetCurrentProcessToken(); } - if (!get_token_user_name(tok, username)) { - (void)strcpy(username, "<unknown>"); + if (in_dprintf_out) { + (void)strcpy(username, "<unknown-recursive>"); + (void)strcpy(groupname, "<unknown-recursive>"); } - if (!get_token_primarygroup_name(tok, groupname)) { - (void)strcpy(groupname, "<unknown>"); + else { + in_dprintf_out = true; + + if (!get_token_user_name(tok, username)) { + (void)strcpy(username, "<unknown>"); + } + if (!get_token_primarygroup_name(tok, groupname)) { + (void)strcpy(groupname, "<unknown>"); + } + + in_dprintf_out = false; } (void)fprintf(dlog_file, "%04x/%s='%s'/%s' ", -- 2.43.0 From 621eecbdac52dd2886cef92d5b54a2d4073cc33d Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 12:52:47 +0200 Subject: [PATCH 2/7] daemon: |get_token_user_name()|/|get_token_primarygroup_name()| always call lsass.exe |get_token_user_name()| and |get_token_primarygroup_name()| should use the SID cache, otherwise we have to do a |LookupAccountSidA()| each time, which will do call into the lsass.exe daemon (Local Security Authority Subsystem Service) - which is bad for performance as we need to do such a lookup for EACH request we receive from the kernel module. Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- daemon/nfs41_daemon.c | 5 ++--- daemon/sid.c | 36 ++++++++++++++++++++++++++++++++---- daemon/sid.h | 10 ++++++++++ daemon/util.c | 25 +++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/daemon/nfs41_daemon.c b/daemon/nfs41_daemon.c index 633f035..bca8f0a 100644 --- a/daemon/nfs41_daemon.c +++ b/daemon/nfs41_daemon.c @@ -701,12 +701,11 @@ VOID ServiceStart(DWORD argc, LPTSTR *argv) exit(1); set_debug_level(cmd_args.debug_level); open_log_files(); + sidcache_init(); nfsd_crt_debug_init(); (void)winsock_init(); init_version_string(); -#ifdef NFS41_DRIVER_SID_CACHE - sidcache_init(); -#else +#ifndef NFS41_DRIVER_SID_CACHE DPRINTF(0, ("SID cache disabled\n")); #endif /* NFS41_DRIVER_SID_CACHE */ diff --git a/daemon/sid.c b/daemon/sid.c index db0df3a..1716fd5 100644 --- a/daemon/sid.c +++ b/daemon/sid.c @@ -204,7 +204,6 @@ sidcache group_sidcache = { 0 }; void sidcache_init(void) { - DPRINTF(1, ("SID cache init\n")); InitializeCriticalSection(&user_sidcache.lock); InitializeCriticalSection(&group_sidcache.lock); } @@ -268,7 +267,7 @@ done: } /* return |malloc()|'ed copy of SID from cache entry */ -PSID *sidcache_getcached(sidcache *cache, const char *win32name) +PSID *sidcache_getcached_byname(sidcache *cache, const char *win32name) { int i; time_t currentTimestamp; @@ -302,6 +301,35 @@ done: LeaveCriticalSection(&cache->lock); return ret_sid; } + +bool sidcache_getcached_bysid(sidcache *cache, PSID sid, char *out_win32name) +{ + int i; + time_t currentTimestamp; + sidcache_entry *e; + bool ret = false; + + EnterCriticalSection(&cache->lock); + currentTimestamp = time(NULL); + + for (i = 0; i < SIDCACHE_SIZE; i++) { + e = &cache->entries[i]; + + if ((e->sid != NULL) && + (EqualSid(sid, e->sid) && + ((currentTimestamp - e->timestamp) < SIDCACHE_TTL))) { + + (void)strcpy(out_win32name, e->win32name); + + ret = true; + goto done; + } + } + +done: + LeaveCriticalSection(&cache->lock); + return ret; +} #endif /* NFS41_DRIVER_SID_CACHE */ @@ -329,7 +357,7 @@ int map_nfs4servername_2_sid(nfs41_daemon_globals *nfs41dg, int query, DWORD *si gid_t gdummy = -1; #ifdef NFS41_DRIVER_SID_CACHE - if (*sid = sidcache_getcached(&user_sidcache, nfsname)) { + if (*sid = sidcache_getcached_byname(&user_sidcache, nfsname)) { *sid_len = GetLengthSid(*sid); DPRINTF(1, ("map_nfs4servername_2_sid: returning cached sid for user '%s'\n", nfsname)); status = 0; @@ -359,7 +387,7 @@ int map_nfs4servername_2_sid(nfs41_daemon_globals *nfs41dg, int query, DWORD *si gid_t gdummy = -1; #ifdef NFS41_DRIVER_SID_CACHE - if (*sid = sidcache_getcached(&group_sidcache, nfsname)) { + if (*sid = sidcache_getcached_byname(&group_sidcache, nfsname)) { *sid_len = GetLengthSid(*sid); DPRINTF(1, ("map_nfs4servername_2_sid: returning cached sid for group '%s'\n", nfsname)); status = 0; diff --git a/daemon/sid.h b/daemon/sid.h index e6d87c5..ab04294 100644 --- a/daemon/sid.h +++ b/daemon/sid.h @@ -26,10 +26,20 @@ #include "nfs41_build_features.h" #include "nfs41_daemon.h" +#include <stdbool.h> + +typedef struct _sidcache sidcache; + +extern sidcache user_sidcache; +extern sidcache group_sidcache; /* prototypes */ int create_unknownsid(WELL_KNOWN_SID_TYPE type, PSID *sid, DWORD *sid_len); void sidcache_init(void); +void sidcache_add(sidcache *cache, const char* win32name, PSID value); +PSID *sidcache_getcached_byname(sidcache *cache, const char *win32name); +bool sidcache_getcached_bysid(sidcache *cache, PSID sid, char *out_win32name); + int map_nfs4servername_2_sid(nfs41_daemon_globals *nfs41dg, int query, DWORD *sid_len, PSID *sid, LPCSTR name); #endif /* !__NFS41_DAEMON_SID_H */ diff --git a/daemon/util.c b/daemon/util.c index ba753d9..fe863c0 100644 --- a/daemon/util.c +++ b/daemon/util.c @@ -29,6 +29,7 @@ #include "daemon_debug.h" #include "util.h" +#include "sid.h" #include "nfs41_ops.h" @@ -754,6 +755,13 @@ bool get_token_user_name(HANDLE tok, char *out_buffer) pusid = ptuser->User.Sid; +#ifdef NFS41_DRIVER_SID_CACHE + if (sidcache_getcached_bysid(&user_sidcache, pusid, out_buffer)) { + DPRINTF(2, ("get_token_user_name: cached '%s'\n", out_buffer)); + return true; + } +#endif /* NFS41_DRIVER_SID_CACHE */ + if (!LookupAccountSidA(NULL, pusid, out_buffer, &namesize, domainbuffer, &domainbuffer_size, &name_use)) { eprintf("get_token_user_name: " @@ -762,6 +770,11 @@ bool get_token_user_name(HANDLE tok, char *out_buffer) return false; } +#ifdef NFS41_DRIVER_SID_CACHE + DPRINTF(2, ("get_token_user_name: NOT cached '%s'\n", out_buffer)); + sidcache_add(&user_sidcache, out_buffer, pusid); +#endif /* NFS41_DRIVER_SID_CACHE */ + return true; } @@ -788,6 +801,13 @@ bool get_token_primarygroup_name(HANDLE tok, char *out_buffer) pgsid = ptpgroup->PrimaryGroup; +#ifdef NFS41_DRIVER_SID_CACHE + if (sidcache_getcached_bysid(&group_sidcache, pgsid, out_buffer)) { + DPRINTF(2, ("get_token_primarygroup_name: cached '%s'\n", out_buffer)); + return true; + } +#endif /* NFS41_DRIVER_SID_CACHE */ + if (!LookupAccountSidA(NULL, pgsid, out_buffer, &namesize, domainbuffer, &domainbuffer_size, &name_use)) { eprintf("get_token_username: " @@ -796,6 +816,11 @@ bool get_token_primarygroup_name(HANDLE tok, char *out_buffer) return false; } +#ifdef NFS41_DRIVER_SID_CACHE + DPRINTF(2, ("get_token_primarygroup_name: NOT cached '%s'\n", out_buffer)); + sidcache_add(&group_sidcache, out_buffer, pgsid); +#endif /* NFS41_DRIVER_SID_CACHE */ + return true; } -- 2.43.0 From 82b7c4f5a761d917cd530c7af9dc374e25999fe2 Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 13:36:39 +0200 Subject: [PATCH 3/7] cygwin: Fix typo in msnfs41client.bash Fix typo in msnfs41client.bash Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- cygwin/devel/msnfs41client.bash | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cygwin/devel/msnfs41client.bash b/cygwin/devel/msnfs41client.bash index 207153f..41a2b4f 100644 --- a/cygwin/devel/msnfs41client.bash +++ b/cygwin/devel/msnfs41client.bash @@ -247,7 +247,7 @@ function nfsclient_rundeamon "/loadConfig:$(cygpath -w "${vsdiagnostics_path}/AgentConfigs/CpuUsageHigh.json")" printf '#\n' printf '# use\n' - printf '# $ "%s" stop %d /output:nfsd_debug%d # to correct profiling data\n#\n' \ + printf '# $ "%s" stop %d /output:nfsd_debug%d # to collect profiling data\n#\n' \ "$(which -a 'VSDiagnostics.exe')" \ "${vsdiagnostics_id}" "$$" else @@ -350,7 +350,7 @@ function nfsclient_system_rundeamon "/loadConfig:$(cygpath -w "${vsdiagnostics_path}/AgentConfigs/CpuUsageHigh.json")" printf '#\n' printf '# use\n' - printf '# $ "%s" stop %d /output:nfsd_debug%d # to correct profiling data\n#\n' \ + printf '# $ "%s" stop %d /output:nfsd_debug%d # to collect profiling data\n#\n' \ "$(which -a 'VSDiagnostics.exe')" \ "${vsdiagnostics_id}" "$$" else -- 2.43.0 From 92aec122526a33575b02ac2733289f4669cf801e Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 13:38:40 +0200 Subject: [PATCH 4/7] daemon: Fix debug message in |get_token_primarygroup_name()| Fix debug message in |get_token_primarygroup_name()| Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- daemon/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/util.c b/daemon/util.c index fe863c0..82d15f4 100644 --- a/daemon/util.c +++ b/daemon/util.c @@ -810,7 +810,7 @@ bool get_token_primarygroup_name(HANDLE tok, char *out_buffer) if (!LookupAccountSidA(NULL, pgsid, out_buffer, &namesize, domainbuffer, &domainbuffer_size, &name_use)) { - eprintf("get_token_username: " + eprintf("get_token_primarygroup_name: " "LookupAccountSidA() failed, status=%d\n", (int)GetLastError()); return false; -- 2.43.0 From 5eb2c520b89d96a5aca6188b4fe8187a99c2a17e Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 14:12:05 +0200 Subject: [PATCH 5/7] tests: Document how to disable the "Microsoft Compatibility Telemetry" daemon Document how to disable the "Microsoft Compatibility Telemetry" daemon, as it can ruin profiling sessions. Reported-by: Martin Wege <martin.l.wege@gmail.com> Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- tests/manual_testing.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/manual_testing.txt b/tests/manual_testing.txt index fe40f2c..0359cb9 100644 --- a/tests/manual_testing.txt +++ b/tests/manual_testing.txt @@ -28,6 +28,13 @@ # disabled, e.g. disable it like this from an "Adminstrator" terminal: # $ powershell -Command 'Set-MpPreference -DisableRealtimeMonitoring 1' # # +# - Microsoft Compatibility Telemetry daemon should be disabled, +# as it can ruin profiling runs. +# The daemon can be disabled from a Cygwin Adminstrator shell like this: +# ---- snip ---- +# regtool -i set '/HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/Windows/CurrentVersion/Policies/DataCollection/AllowTelemetry' 0 +# ---- snip ---- +# # - A timeserver shoud be running on both Windows (NFSv4.1 client and # NFSv4.1 server). # For example on Windows add timeserver 10.49.0.6 like this: -- 2.43.0 From a15501c0f26295936ffbdec8ead74c251b21e965 Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 14:18:29 +0200 Subject: [PATCH 6/7] daemon: Optimize |create_open_state()| Optimize |create_open_state()|: Both |StringCchCopyA()| and |StringCchPrintfA()| are in the VC19 profiler's "hot codepaths" list, and can be replaced by code which doesn't check the buffer size as we do that anyway before doing the copies. Reported-by: Martin Wege <martin.l.wege@gmail.com> Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- daemon/open.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/daemon/open.c b/daemon/open.c index 814354f..bd21d73 100644 --- a/daemon/open.c +++ b/daemon/open.c @@ -42,6 +42,8 @@ static int create_open_state( { int status; nfs41_open_state *state; + size_t path_len; + state = calloc(1, sizeof(nfs41_open_state)); if (state == NULL) { @@ -50,18 +52,28 @@ static int create_open_state( } InitializeSRWLock(&state->path.lock); - if (FAILED(StringCchCopyA(state->path.path, NFS41_MAX_PATH_LEN, path))) { + + path_len = strlen(path); + if (path_len >= NFS41_MAX_PATH_LEN) { status = ERROR_FILENAME_EXCED_RANGE; goto out_free; } - state->path.len = (unsigned short)strlen(state->path.path); + + (void)strcpy(state->path.path, path); + state->path.len = (unsigned short)path_len; + path_fh_init(&state->file, &state->path); path_fh_init(&state->parent, &state->path); last_component(state->path.path, state->file.name.name, &state->parent.name); - StringCchPrintfA((LPSTR)state->owner.owner, NFS4_OPAQUE_LIMIT, "%u", - open_owner_id); + /* + * We use |_ultoa()| as optimised version of this code: + * |StringCchPrintfA((LPSTR)state->owner.owner, NFS4_OPAQUE_LIMIT, "%u", + * open_owner_id);| + */ + (void)_ultoa(open_owner_id, (LPSTR)state->owner.owner, 10); state->owner.owner_len = (uint32_t)strlen((const char*)state->owner.owner); + InitializeSRWLock(&state->lock); state->ref_count = 1; /* will be released in |cleanup_close()| */ list_init(&state->locks.list); -- 2.43.0 From 86a78253fba8b3e5009b7ca1e4de9b364ed75907 Mon Sep 17 00:00:00 2001 From: Roland Mainz <roland.mainz@nrubsig.org> Date: Mon, 29 Apr 2024 16:26:38 +0200 Subject: [PATCH 7/7] daemon: Disable 8dot3 ShortName filename generation Disable 8dot3 ShortName filename generation, the code does currently not work as expected and |GetShortPathNameW()| in this codepath eats ~~3% CPU time when building bash.git. In general Windows is moving away fro generating 8dot3 short names, but we might need this in the future for compatibility to older DOS&&Windows applications. Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com> --- daemon/readdir.c | 7 +++++++ nfs41_build_features.h | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/daemon/readdir.c b/daemon/readdir.c index 544084f..6eae316 100644 --- a/daemon/readdir.c +++ b/daemon/readdir.c @@ -375,6 +375,7 @@ static void readdir_copy_dir_info( &entry->attr_info); } +#ifndef NFS41_DRIVER_DISABLE_8DOT3_SHORTNAME_GENERATION static void readdir_copy_shortname( IN LPCWSTR name, OUT LPWSTR name_out, @@ -387,6 +388,7 @@ static void readdir_copy_shortname( *name_size_out *= sizeof(WCHAR); } } +#endif /* !NFS41_DRIVER_DISABLE_8DOT3_SHORTNAME_GENERATION */ static void readdir_copy_full_dir_info( IN nfs41_readdir_entry *entry, @@ -408,8 +410,13 @@ static void readdir_copy_both_dir_info( IN PFILE_DIR_INFO_UNION info) { readdir_copy_full_dir_info(entry, info); +#ifdef NFS41_DRIVER_DISABLE_8DOT3_SHORTNAME_GENERATION + info->fbdi.ShortName[0] = L'\0'; + info->fbdi.ShortNameLength = 0; +#else readdir_copy_shortname(wname, info->fbdi.ShortName, &info->fbdi.ShortNameLength); +#endif /* NFS41_DRIVER_DISABLE_8DOT3_SHORTNAME_GENERATION */ } static void readdir_copy_filename( diff --git a/nfs41_build_features.h b/nfs41_build_features.h index 7b607ea..d197668 100644 --- a/nfs41_build_features.h +++ b/nfs41_build_features.h @@ -99,4 +99,11 @@ */ #define NFS41_DRIVER_SETGID_NEWGRP_SUPPORT 1 +/* + * Disable 8DOT3 ShortName filename generation. + * The current code is broken anyway,so we disable it until we + * can implement it better.. + */ +#define NFS41_DRIVER_DISABLE_8DOT3_SHORTNAME_GENERATION 1 + #endif /* !_NFS41_DRIVER_BUILDFEATURES_ */ -- 2.43.0

Title:

Privacy: Public Private

How long should your post be retained?

15 mins an hour a day a week a month forever

All content is user-submitted.
The administrators of this site (kpaste.net) are not responsible for their content.
Abuse reports should be emailed to us at