- From f6d1e0bad63000b7527b70a7438fe845e40aae5b Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Tue, 16 Apr 2024 14:49:10 +0200
- Subject: [PATCH 1/4] libtirpc,daemon: Do not call WinSock API from |DllMain()|
- It is not legal to call the WinSock API, including |WSAStartup()|,
- from (libtirpc's) |DllMain()|.
- See https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsastartup
- AppVerifier+cdb generated this stack trace:
- ---- snip ----
- vrfcore!VerifierStopMessageEx+0x7f7
- vfnet!StopIfCalledWithinDllMain+0xfa
- vfnet!VfHookWSAStartup+0x2d
- libtirpc!winsock_init(void)+0x28 [C:\cygwin64\home\roland_mainz\work\msnfs41_uidmapping\ms-nfs41-client\libtirpc\src\wintirpc.c @ 75]
- libtirpc!DllMain(struct HINSTANCE__ * hinstDLL = 0x00007ffc`75c80000, unsigned long fdwReason = 1, void * lpvReserved = 0x000000ad`2239f640)+0x50 [C:\cygwin64\home\roland_mainz\work\msnfs41_uidmapping\ms-nfs41-client\libtirpc\src\wintirpc.c @ 109]
- libtirpc!dllmain_dispatch(struct HINSTANCE__ * instance = 0x00007ffc`75c80000, unsigned long reason = 1, void * reserved = 0x000000ad`2239f640)+0x98 [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\dll_dllmain.cpp @ 281]
- libtirpc!_DllMainCRTStartup(struct HINSTANCE__ * instance = 0x00007ffc`75c80000, unsigned long reason = 1, void * reserved = 0x000000ad`2239f640)+0x31 [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\dll_dllmain.cpp @ 335]
- verifier!AVrfpStandardDllEntryPointRoutine+0xe9
- vrfcore!VfCoreStandardDllEntryPointRoutine+0x18a
- vfbasics!AVrfpStandardDllEntryPointRoutine+0xf3
- ntdll!LdrpCallInitRoutine+0x61
- ntdll!LdrpInitializeNode+0x1d3
- ntdll!LdrpInitializeGraphRecurse+0x42
- ntdll!LdrpInitializeGraphRecurse+0xc8
- ntdll!LdrpInitializeProcess+0x1f62
- ntdll!LdrpInitialize+0x15f
- ntdll!LdrpInitialize+0x3b
- ntdll!LdrInitializeThunk+0xe
- ---- snip ----
- We now do the |WSAStartup()| via |__rpc_nconf2fd()|, but without
- support for cleanup. In the future we should do refcounting, and
- do a cleanup if the refcount reaches 0.
- Reported-by: Dan Shelton <dan.f.shelton@gmail.com>
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- daemon/nfs41_daemon.c | 18 +++++++++++-
- libtirpc/src/rpc_generic.c | 4 +++
- libtirpc/src/wintirpc.c | 57 ++++++++------------------------------
- libtirpc/tirpc/wintirpc.h | 3 ++
- 4 files changed, 36 insertions(+), 46 deletions(-)
- diff --git a/daemon/nfs41_daemon.c b/daemon/nfs41_daemon.c
- index 5b9c48d..5a6bbc2 100644
- --- a/daemon/nfs41_daemon.c
- +++ b/daemon/nfs41_daemon.c
- @@ -520,6 +520,21 @@ void nfsd_crt_debug_init(void)
- #endif /* _DEBUG */
- }
- +static
- +bool winsock_init(void)
- +{
- + int err;
- + WSADATA WSAData;
- +
- + err = WSAStartup(MAKEWORD(2, 2), &WSAData);
- + if (err != 0) {
- + eprintf("winsock_init: WSAStartup() failed!\n");
- + WSACleanup();
- + return false;
- + }
- + return true;
- +}
- +
- static
- void init_version_string(void)
- {
- @@ -625,8 +640,9 @@ VOID ServiceStart(DWORD argc, LPTSTR *argv)
- exit(1);
- set_debug_level(cmd_args.debug_level);
- open_log_files();
- - init_version_string();
- nfsd_crt_debug_init();
- + (void)winsock_init();
- + init_version_string();
- #ifdef NFS41_DRIVER_SID_CACHE
- sidcache_init();
- #else
- diff --git a/libtirpc/src/rpc_generic.c b/libtirpc/src/rpc_generic.c
- index e5c8274..345c4ba 100644
- --- a/libtirpc/src/rpc_generic.c
- +++ b/libtirpc/src/rpc_generic.c
- @@ -563,6 +563,10 @@ __rpc_nconf2fd(const struct netconfig *nconf)
- struct __rpc_sockinfo si;
- int fd;
- +#ifdef _WIN32
- + if (!wintirpc_winsock_init())
- + return 0;
- +#endif
- if (!__rpc_nconf2sockinfo(nconf, &si))
- return 0;
- diff --git a/libtirpc/src/wintirpc.c b/libtirpc/src/wintirpc.c
- index 152caf4..1eba532 100644
- --- a/libtirpc/src/wintirpc.c
- +++ b/libtirpc/src/wintirpc.c
- @@ -34,52 +34,21 @@ static DWORD dwTlsIndex;
- extern void multithread_init(void);
- -VOID
- -tirpc_report(LPTSTR lpszMsg)
- -{
- - WCHAR chMsg[256];
- - HANDLE hEventSource;
- - LPCWSTR lpszStrings[2];
- -
- - // Use event logging to log the error.
- - //
- - hEventSource = RegisterEventSource(NULL,
- - TEXT("tirpc.dll"));
- -
- - swprintf_s(chMsg, sizeof(chMsg), L"tirpc report: %d", GetLastError());
- - lpszStrings[0] = (LPCWSTR)chMsg;
- - lpszStrings[1] = lpszMsg;
- -
- - if (hEventSource != NULL) {
- - ReportEvent(hEventSource, // handle of event source
- - EVENTLOG_WARNING_TYPE, // event type
- - 0, // event category
- - 0, // event ID
- - NULL, // current user's SID
- - 2, // strings in lpszStrings
- - 0, // no bytes of raw data
- - lpszStrings, // array of error strings
- - NULL); // no raw data
- -
- - (VOID) DeregisterEventSource(hEventSource);
- - }
- -}
- -
- void tirpc_criticalsection_init(void) {
- multithread_init();
- }
- -BOOL winsock_init(void)
- +bool wintirpc_winsock_init(void)
- {
- int err;
- - err = WSAStartup(MAKEWORD( 3, 3 ), &WSAData); // XXX THIS SHOULD BE FAILING!!!!!!!!!!!!!!!!!
- + err = WSAStartup(MAKEWORD(2, 2), &WSAData);
- if (err != 0) {
- init = 0;
- - tirpc_report(L"WSAStartup failed!\n");
- + (void)fprintf(stderr, "winsock_init: WSAStartup failed!\n");
- WSACleanup();
- - return FALSE;
- + return false;
- }
- - return TRUE;
- + return true;
- }
- BOOL winsock_fini(void)
- @@ -104,13 +73,9 @@ BOOL WINAPI DllMain/*tirpc_main*/(HINSTANCE hinstDLL, // DLL module handle
- // The DLL is loading due to process
- // initialization or a call to LoadLibrary.
- case DLL_PROCESS_ATTACH:
- -
- - // Initialize socket library
- - if (winsock_init() == FALSE)
- - return FALSE;
- -
- - // Initialize CriticalSections
- - tirpc_criticalsection_init();
- + // Do NOT init WinSock here, it is not legal and AppVerifer will complain
- + // Initialize CriticalSections
- + tirpc_criticalsection_init();
- // Allocate a TLS index.
- if ((dwTlsIndex = TlsAlloc()) == TLS_OUT_OF_INDEXES)
- @@ -149,8 +114,10 @@ BOOL WINAPI DllMain/*tirpc_main*/(HINSTANCE hinstDLL, // DLL module handle
- // Release the TLS index.
- TlsFree(dwTlsIndex);
- - // Clean up winsock stuff
- - winsock_fini();
- + // Clean up winsock stuff
- + // FIXME: This is not legal in DllMain, we should use
- + // recounting instead
- + winsock_fini();
- break;
- diff --git a/libtirpc/tirpc/wintirpc.h b/libtirpc/tirpc/wintirpc.h
- index 4d70be4..94ecf18 100644
- --- a/libtirpc/tirpc/wintirpc.h
- +++ b/libtirpc/tirpc/wintirpc.h
- @@ -39,9 +39,11 @@
- /* use visual studio's debug heap */
- # define _CRTDBG_MAP_ALLOC
- # include <stdlib.h>
- +# include <stdbool.h>
- # include <crtdbg.h>
- #else
- # include <stdlib.h>
- +# include <stdbool.h>
- #endif
- /* Common Windows includes */
- @@ -136,6 +138,7 @@ struct sockaddr_un {
- typedef SSIZE_T wintirpc_ssize_t;
- /* Prototypes */
- +bool wintirpc_winsock_init(void);
- int wintirpc_socket(int af,int type, int protocol);
- int wintirpc_closesocket(int in_fd);
- int wintirpc_close(int in_fd);
- --
- 2.43.0
- From 1f7d9946d49a3aaffab024e965d4ed54ce6a9ff7 Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Wed, 17 Apr 2024 13:44:45 +0200
- Subject: [PATCH 2/4] nfs41_np: Network provider should use |towupper()|, not
- |toupper()|
- Network provider should use |towupper()|, not |toupper()|
- Reported-by: Martin Wege <martin.l.wege@gmail.com>
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- dll/nfs41_np.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
- diff --git a/dll/nfs41_np.c b/dll/nfs41_np.c
- index 350cb73..db57e4f 100644
- --- a/dll/nfs41_np.c
- +++ b/dll/nfs41_np.c
- @@ -469,7 +469,7 @@ NPAddConnection3(
- goto out;
- }
- - LocalName[0] = (WCHAR) toupper(lpNetResource->lpLocalName[0]);
- + LocalName[0] = towupper(lpNetResource->lpLocalName[0]);
- LocalName[1] = L':';
- LocalName[2] = L'\0';
- StringCchCopyW( ConnectionName, NFS41_SYS_MAX_PATH_LEN, NFS41_DEVICE_NAME );
- --
- 2.43.0
- From 90cdb3adcd1aa323d3489b55a0817eb17af522a8 Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Wed, 17 Apr 2024 14:38:46 +0200
- Subject: [PATCH 3/4] cygwin: nfsd_debug cannot run as user "SYSTEM", fails
- with exit code 0xC0000022
- nfsd_debug.exe cannot run as user "SYSTEM", fails with exit code
- 0xC0000022 due to file permission errors.
- It turns out this is a DLL load error - if a DLL cannot be *read* the
- system triggers an exit code of 0xC0000022.
- DLLs can only be used if they are readable (chmod a+r), the execute
- flag (chmod a+x) only matters for Cygwin, so the bintarball Makefile
- target must do a chmod a+rx instead of chmod a+x, as we install the
- files usually as a non-SYSTEM plain user.
- Reported-by: Martin Wege <martin.l.wege@gmail.com>
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- cygwin/Makefile | 4 ++--
- 1 file changed, 2 insertions(+), 2 deletions(-)
- diff --git a/cygwin/Makefile b/cygwin/Makefile
- index a2fcd24..ea16bfa 100644
- --- a/cygwin/Makefile
- +++ b/cygwin/Makefile
- @@ -111,8 +111,8 @@ installdest: $(VS_BUILD_DIR)/nfsd.exe \
- cp "$$i" $(DESTDIR)/cygdrive/c/cygwin64/sbin/. ; \
- done
- @ printf "# Set file flags\n"
- - (cd $(DESTDIR)/cygdrive/c/cygwin64/sbin/ && chmod a+x *.exe *.dll)
- - (cd $(DESTDIR)/cygdrive/c/cygwin64/lib/msnfs41client/ && chmod a+x *.dll)
- + (cd $(DESTDIR)/cygdrive/c/cygwin64/sbin/ && chmod a+rx *.exe *.dll)
- + (cd $(DESTDIR)/cygdrive/c/cygwin64/lib/msnfs41client/ && chmod a+rx *.dll)
- @printf "\n#\n# TEST sbin dir is %s\n#\n" "$(DESTDIR)/cygdrive/c/cygwin64/sbin/"
- @printf '\n'
- @printf "\n#\n# Now use\n# $$ cd '%s' && ./msnfs41client install #\n# to install the kernel driver as Admin\n#\n" \
- --
- 2.43.0
- From 0955e463cbadad8c456c49d3a3c4b27e993be79c Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Wed, 17 Apr 2024 15:21:18 +0200
- Subject: [PATCH 4/4] daemon: "cygwin_idmapper.ksh: line 141: getent: not
- found" if nfsd runs as "SYSTEM"
- idmapping can fail with a "cygwin_idmapper.ksh: line 141: getent: not found"
- error message if nfsd/nfsd_debug runs as user "SYSTEM".
- This happens because Cygwin does not include "/bin:/sbin" in the PATH
- variable for user "SYSTEM".
- We fix this by explicitly setting PATH='/bin:/usr/bin' (default
- from $ getconf PATH #) in cygwin_idmapper.ksh.
- Reported-by: Martin Wege <martin.l.wege@gmail.com>
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- cygwin_idmapper.ksh | 2 ++
- 1 file changed, 2 insertions(+)
- diff --git a/cygwin_idmapper.ksh b/cygwin_idmapper.ksh
- index 55251b9..588e40b 100644
- --- a/cygwin_idmapper.ksh
- +++ b/cygwin_idmapper.ksh
- @@ -3,6 +3,8 @@
- set -o nounset
- typeset IFS=''
- +export PATH='/bin:/usr/bin'
- +
- #
- # global variables for this script
- # (stored in compound variable so we
- --
- 2.43.0
msnfs41client: AppVerifier/libtirpc/DLL, RunAsUserSYSTEM+misc patches, 2024-04-17 ...
Posted by Anonymous on Wed 17th Apr 2024 15:45
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.