- From 9d5e3868db7f79f4145e5aff7089b0c3182c0436 Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Thu, 9 Nov 2023 11:02:49 +0100
- Subject: [PATCH 1/3] sys/nfs41_driver: Retry |KeWaitForSingleObject()| for
- upcall too
- sys/nfs41_driver: Retry |KeWaitForSingleObject| in |nfs41_upcall()|
- too, as we already did in 'sys/nfs41_driver: Fix "IOCTL_NFS41_WRITE
- failed with 31 xid=... opcode=..."'.
- The issue has not been seen in |nfs41_upcall()| in the wild yet, but
- better to safeguard it here...
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- sys/nfs41_driver.c | 22 ++++++++++++++++++----
- 1 file changed, 18 insertions(+), 4 deletions(-)
- diff --git a/sys/nfs41_driver.c b/sys/nfs41_driver.c
- index 0679a22..71d4032 100644
- --- a/sys/nfs41_driver.c
- +++ b/sys/nfs41_driver.c
- @@ -1564,14 +1564,28 @@ process_upcall:
- RxContext->InformationToReturn = len;
- }
- else {
- +retry_wait:
- status = KeWaitForSingleObject(&upcallEvent, Executive, UserMode, TRUE,
- (PLARGE_INTEGER) NULL);
- print_wait_status(0, "[upcall]", status, NULL, NULL, 0);
- switch (status) {
- - case STATUS_SUCCESS: goto process_upcall;
- - case STATUS_USER_APC:
- - case STATUS_ALERTED:
- - default: goto out;
- + case STATUS_USER_APC:
- + case STATUS_ALERTED:
- + DbgP("nfs41_upcall: KeWaitForSingleObject() "
- + "returned status(=%ld), "
- + "retry waiting for '%s' entry=%p xid=%lld\n",
- + (long)status,
- + opcode2string(entry->opcode), entry, entry->xid);
- + goto retry_wait;
- + case STATUS_SUCCESS:
- + goto process_upcall;
- + default:
- + DbgP("nfs41_upcall: KeWaitForSingleObject() "
- + "returned UNEXPECTED status(=%ld), "
- + "for '%s' entry=%p xid=%lld\n",
- + (long)status,
- + opcode2string(entry->opcode), entry, entry->xid);
- + goto out;
- }
- }
- out:
- --
- 2.42.1
- From 0f0d7b9c90499edf1cce6820a6bd5b2d1badfcee Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Thu, 9 Nov 2023 12:24:43 +0100
- Subject: [PATCH 2/3] libtirpc: Document |_open()| limit+ensure FD_SETSIZE has
- same value
- libtirpc:
- 1. Document the maximum fd number limit of |_open()|
- (currently |8192| on Windows 10)
- 2. Add sourcecode safeguards to ensure that |FD_SETSIZE| has the
- same value (currently |1024|) everywhere we use libtirpc.
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- build.vc19/nfsd/nfsd.vcxproj | 8 ++++----
- libtirpc/src/sources | 2 +-
- libtirpc/src/wintirpc.c | 31 ++++++++++++++++++++++++-------
- libtirpc/tirpc/wintirpc.h | 24 ++++++++++++++++++++++++
- 4 files changed, 53 insertions(+), 12 deletions(-)
- diff --git a/build.vc19/nfsd/nfsd.vcxproj b/build.vc19/nfsd/nfsd.vcxproj
- index b259108..1687a0c 100644
- --- a/build.vc19/nfsd/nfsd.vcxproj
- +++ b/build.vc19/nfsd/nfsd.vcxproj
- @@ -87,7 +87,7 @@
- </PrecompiledHeader>
- <WarningLevel>Level3</WarningLevel>
- <Optimization>Disabled</Optimization>
- - <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;NO_CB_4_KRB5P;STANDALONE_NFSD;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- + <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;FD_SETSIZE=1024;INET6;NO_CB_4_KRB5P;STANDALONE_NFSD;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- <AdditionalIncludeDirectories>..\..\libtirpc\tirpc;..\..\sys;..\..\dll;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
- </ClCompile>
- <Link>
- @@ -102,7 +102,7 @@
- </PrecompiledHeader>
- <WarningLevel>Level3</WarningLevel>
- <Optimization>Disabled</Optimization>
- - <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;NO_CB_4_KRB5P;STANDALONE_NFSD;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- + <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;FD_SETSIZE=1024;INET6;NO_CB_4_KRB5P;STANDALONE_NFSD;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- <AdditionalIncludeDirectories>..\..\libtirpc\tirpc;..\..\sys;..\..\dll;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
- </ClCompile>
- <Link>
- @@ -119,7 +119,7 @@
- <Optimization>MaxSpeed</Optimization>
- <FunctionLevelLinking>true</FunctionLevelLinking>
- <IntrinsicFunctions>true</IntrinsicFunctions>
- - <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;NO_CB_4_KRB5P;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- + <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;FD_SETSIZE=1024;INET6;NO_CB_4_KRB5P;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- <AdditionalIncludeDirectories>..\..\libtirpc\tirpc;..\..\sys;..\..\dll;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
- </ClCompile>
- <Link>
- @@ -138,7 +138,7 @@
- <Optimization>MaxSpeed</Optimization>
- <FunctionLevelLinking>true</FunctionLevelLinking>
- <IntrinsicFunctions>true</IntrinsicFunctions>
- - <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;NO_CB_4_KRB5P;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- + <PreprocessorDefinitions>WIN32_LEAN_AND_MEAN;FD_SETSIZE=1024;INET6;NO_CB_4_KRB5P;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;NDEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
- <AdditionalIncludeDirectories>..\..\libtirpc\tirpc;..\..\sys;..\..\dll;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
- </ClCompile>
- <Link>
- diff --git a/libtirpc/src/sources b/libtirpc/src/sources
- index 87249b5..26f5d59 100644
- --- a/libtirpc/src/sources
- +++ b/libtirpc/src/sources
- @@ -81,7 +81,7 @@ DLLBASE=0x1010000
- #USE_NTDLL=1
- #USE_MSVCRT=1
- USE_LIBCMT=1
- -NET_C_DEFINES=-DUNICODE -DFD_SETSIZE=128 -DINET6 -DNO_CB_4_KRB5P -DPORTMAP
- +NET_C_DEFINES=-DUNICODE -DFD_SETSIZE=1024 -DINET6 -DNO_CB_4_KRB5P -DPORTMAP
- INCLUDES=..\sys; \
- ..\tirpc; \
- diff --git a/libtirpc/src/wintirpc.c b/libtirpc/src/wintirpc.c
- index e75b023..4db1bb8 100644
- --- a/libtirpc/src/wintirpc.c
- +++ b/libtirpc/src/wintirpc.c
- @@ -166,16 +166,14 @@ struct map_osfhandle_fd
- int m_fd;
- };
- -#define MAP_OSFHANDLE_SIZE (1024)
- -
- static
- -struct map_osfhandle_fd handle_fd_map[MAP_OSFHANDLE_SIZE];
- +struct map_osfhandle_fd handle_fd_map[WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE];
- void wintirpc_register_osfhandle_fd(SOCKET handle, int fd)
- {
- assert(handle != 0);
- assert(handle != SOCKET_ERROR);
- - assert(fd < MAP_OSFHANDLE_SIZE);
- + assert(fd < WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE);
- handle_fd_map[fd].m_fd = fd;
- handle_fd_map[fd].m_s = handle;
- @@ -188,7 +186,7 @@ void wintirpc_unregister_osfhandle(SOCKET handle)
- assert(handle != 0);
- assert(handle != SOCKET_ERROR);
- - for (i=0 ; i < MAP_OSFHANDLE_SIZE ; i++) {
- + for (i=0 ; i < WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE ; i++) {
- if (handle_fd_map[i].m_s == handle) {
- handle_fd_map[i].m_s = SOCKET_ERROR;
- handle_fd_map[i].m_fd = -1;
- @@ -205,7 +203,7 @@ int wintirpc_handle2fd(SOCKET handle)
- assert(handle != 0);
- assert(handle != SOCKET_ERROR);
- - for (i=0 ; i < MAP_OSFHANDLE_SIZE ; i++) {
- + for (i=0 ; i < WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE ; i++) {
- if ((handle_fd_map[i].m_s == handle) &&
- (handle_fd_map[i].m_fd != -1)) {
- return handle_fd_map[i].m_fd;
- @@ -238,9 +236,19 @@ int wintirpc_socket(int af, int type, int protocol)
- return -1;
- }
- + if (fd >= WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE) {
- + (void)_close(fd);
- + (void)fprintf(stderr, "wintirpc_socket: fd overflow %d >= %d\n",
- + fd,
- + WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE);
- + errno = ENOMEM;
- + return -1;
- + }
- +
- wintirpc_register_osfhandle_fd(s, fd);
- - (void)fprintf(stderr, "wintirpc_socket: %s/%d: sock fd=%d\n", __FILE__, (int)__LINE__, fd);
- + (void)fprintf(stderr, "wintirpc_socket: %s/%d: sock fd=%d\n",
- + __FILE__, (int)__LINE__, fd);
- return fd;
- }
- @@ -281,6 +289,15 @@ int wintirpc_accept(int in_s_fd, struct sockaddr *addr, int *addrlen)
- return -1;
- }
- + if (out_s_fd >= WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE) {
- + (void)_close(out_s_fd);
- + (void)fprintf(stderr, "wintirpc_accept: out_s_fd overflow %d >= %d\n",
- + out_s_fd,
- + WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE);
- + errno = ENOMEM;
- + return -1;
- + }
- +
- wintirpc_register_osfhandle_fd(out_s, out_s_fd);
- return out_s_fd;
- diff --git a/libtirpc/tirpc/wintirpc.h b/libtirpc/tirpc/wintirpc.h
- index e7472aa..66a0f48 100644
- --- a/libtirpc/tirpc/wintirpc.h
- +++ b/libtirpc/tirpc/wintirpc.h
- @@ -74,6 +74,30 @@
- #define __END_DECLS
- #define __THROW
- +/*
- + * Maximum integer value |_open()| and |_open_osfhandle()| can return
- + *
- + * Right now the Windows headers do not provide this as public value.
- + *
- + * https://github.com/ojdkbuild/tools_toolchain_sdk10_1607/blob/master/Source/10.0.14393.0/ucrt/inc/corecrt_internal_lowio.h#L94
- + * defines it like this:
- + * -- snip --
- + * #define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
- + * #define IOINFO_ARRAYS 128
- + * #define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS)
- + * -- snip --
- + * ... which makes |_NHANDLE_|==2^6*128==8192
- + */
- +#define WINTIRPC_MAX_OSFHANDLE_FD_NHANDLE_VALUE (8192)
- +
- +/* Safeguard */
- +#ifndef FD_SETSIZE
- +#error FD_SETSIZE not set
- +#endif
- +#if (FD_SETSIZE < 1024)
- +#error FD_SETSIZE should be at least 1024
- +#endif
- +
- /*
- * Functions imported from BSD
- */
- --
- 2.42.1
- From 9b359bca46e82257dbf610377a19da6f4bba64ae Mon Sep 17 00:00:00 2001
- From: Roland Mainz <roland.mainz@nrubsig.org>
- Date: Thu, 9 Nov 2023 14:40:40 +0100
- Subject: [PATCH 3/3] sys/nfs41_driver: Fix random crashes/hangs when <CTRL-C>
- nfsd_debug.exe
- 1. Fix random kernel crashes in the debug version of the kernel
- module when nfsd_debug.exe gets a <CTRL-C>, caused by dereferencing
- |NULL| |entry| in |DbgP()|.
- Added new debug macro to catch all possible cases, to ensure we are
- not getting that issue again.
- 2. Disable retry-wait code in |nfs41_upcall()| for now, until
- we can figure out a way to prevent it from hanging nfsd_debug.exe
- on exit.
- Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
- ---
- sys/nfs41_debug.h | 2 ++
- sys/nfs41_driver.c | 66 ++++++++++++++++++++++++++++++----------------
- 2 files changed, 46 insertions(+), 22 deletions(-)
- diff --git a/sys/nfs41_debug.h b/sys/nfs41_debug.h
- index 6297a1a..7e95ff5 100644
- --- a/sys/nfs41_debug.h
- +++ b/sys/nfs41_debug.h
- @@ -3,6 +3,7 @@
- *
- * Olga Kornievskaia <aglo@umich.edu>
- * Casey Bodley <cbodley@umich.edu>
- + * Roland Mainz <roland.mainz@nrubsig.org>
- *
- * This library is free software; you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License as published by
- @@ -49,6 +50,7 @@ void print_ea_info(int on, PFILE_FULL_EA_INFORMATION info);
- void print_get_ea(int on, PFILE_GET_EA_INFORMATION info);
- void print_caching_level(int on, ULONG flag, PUNICODE_STRING s);
- const char *opcode2string(int opcode);
- +#define ENTRY_OPCODE2STRING(entry) ((entry)?opcode2string((entry)->opcode):"<entry==NULL>")
- void print_open_error(int on, int status);
- void print_wait_status(int on, const char *str, NTSTATUS status,
- const char *opcode, PVOID entry, LONGLONG xid);
- diff --git a/sys/nfs41_driver.c b/sys/nfs41_driver.c
- index 71d4032..123e6dd 100644
- --- a/sys/nfs41_driver.c
- +++ b/sys/nfs41_driver.c
- @@ -3,6 +3,7 @@
- *
- * Olga Kornievskaia <aglo@umich.edu>
- * Casey Bodley <cbodley@umich.edu>
- + * Roland Mainz <roland.mainz@nrubsig.org>
- *
- * This library is free software; you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License as published by
- @@ -583,7 +584,7 @@ NTSTATUS marshal_nfs41_header(
- if (MmIsAddressValid(entry->filename))
- DbgP("[upcall header] xid=%lld opcode=%s filename=%wZ version=%d "
- "session=0x%x open_state=0x%x\n", entry->xid,
- - opcode2string(entry->opcode), entry->filename,
- + ENTRY_OPCODE2STRING(entry), entry->filename,
- entry->version, entry->session, entry->open_state);
- else
- status = STATUS_INTERNAL_ERROR;
- @@ -1496,7 +1497,8 @@ retry_wait:
- }
- if (status != STATUS_SUCCESS) {
- print_wait_status(1, "[downcall]", status,
- - opcode2string(entry->opcode), entry, entry->xid);
- + ENTRY_OPCODE2STRING(entry), entry,
- + (entry?entry->xid:-1LL));
- if (status == STATUS_TIMEOUT)
- status = STATUS_NETWORK_UNREACHABLE;
- }
- @@ -1504,29 +1506,38 @@ retry_wait:
- status = KeWaitForSingleObject(&entry->cond, Executive, KernelMode, FALSE, NULL);
- #endif
- - print_wait_status(0, "[downcall]", status, opcode2string(entry->opcode),
- - entry, entry->xid);
- + print_wait_status(0, "[downcall]", status,
- + ENTRY_OPCODE2STRING(entry), entry,
- + (entry?entry->xid:-1LL));
- } else
- goto out;
- switch(status) {
- + case STATUS_SUCCESS:
- + break;
- case STATUS_USER_APC:
- case STATUS_ALERTED:
- DbgP("nfs41_UpcallWaitForReply: KeWaitForSingleObject() "
- "returned status(=%ld), "
- "retry waiting for '%s' entry=%p xid=%lld\n",
- (long)status,
- - opcode2string(entry->opcode), entry, entry->xid);
- - goto retry_wait;
- - case STATUS_SUCCESS: break;
- + ENTRY_OPCODE2STRING(entry),
- + entry,
- + (entry?entry->xid:-1LL));
- + if (entry) {
- + goto retry_wait;
- + }
- + /* fall-through */
- default:
- ExAcquireFastMutex(&entry->lock);
- if (entry->state == NFS41_DONE_PROCESSING) {
- ExReleaseFastMutex(&entry->lock);
- break;
- }
- - DbgP("[upcall] abandoning %s entry=%p xid=%lld\n",
- - opcode2string(entry->opcode), entry, entry->xid);
- + DbgP("[upcall] abandoning '%s' entry=%p xid=%lld\n",
- + ENTRY_OPCODE2STRING(entry),
- + entry,
- + (entry?entry->xid:-1LL));
- entry->state = NFS41_NOT_WAITING;
- ExReleaseFastMutex(&entry->lock);
- goto out;
- @@ -1540,13 +1551,14 @@ NTSTATUS nfs41_upcall(
- IN PRX_CONTEXT RxContext)
- {
- NTSTATUS status = STATUS_SUCCESS;
- - nfs41_updowncall_entry *entry = NULL;
- ULONG len = 0;
- - PLIST_ENTRY pEntry;
- + PLIST_ENTRY pEntry = NULL;
- process_upcall:
- nfs41_RemoveFirst(upcallLock, upcall, pEntry);
- if (pEntry) {
- + nfs41_updowncall_entry *entry;
- +
- entry = (nfs41_updowncall_entry *)CONTAINING_RECORD(pEntry,
- nfs41_updowncall_entry, next);
- ExAcquireFastMutex(&entry->lock);
- @@ -1564,27 +1576,37 @@ process_upcall:
- RxContext->InformationToReturn = len;
- }
- else {
- +/*
- + * gisburn: |NFSV41_UPCALL_RETRY_WAIT| disabled for now because it
- + * causes nfsd_debug.exe to hang on <CTRL-C>
- + */
- +#ifdef NFSV41_UPCALL_RETRY_WAIT
- retry_wait:
- +#endif /* NFSV41_UPCALL_RETRY_WAIT */
- status = KeWaitForSingleObject(&upcallEvent, Executive, UserMode, TRUE,
- (PLARGE_INTEGER) NULL);
- print_wait_status(0, "[upcall]", status, NULL, NULL, 0);
- switch (status) {
- + case STATUS_SUCCESS:
- + goto process_upcall;
- case STATUS_USER_APC:
- case STATUS_ALERTED:
- DbgP("nfs41_upcall: KeWaitForSingleObject() "
- - "returned status(=%ld), "
- - "retry waiting for '%s' entry=%p xid=%lld\n",
- - (long)status,
- - opcode2string(entry->opcode), entry, entry->xid);
- + "returned status(=%ld)"
- +#ifdef NFSV41_UPCALL_RETRY_WAIT
- + ", retry waiting"
- +#endif /* NFSV41_UPCALL_RETRY_WAIT */
- + "\n",
- + (long)status);
- +#ifdef NFSV41_UPCALL_RETRY_WAIT
- goto retry_wait;
- - case STATUS_SUCCESS:
- - goto process_upcall;
- +#else
- + /* fall-through */
- +#endif /* NFSV41_UPCALL_RETRY_WAIT */
- default:
- DbgP("nfs41_upcall: KeWaitForSingleObject() "
- - "returned UNEXPECTED status(=%ld), "
- - "for '%s' entry=%p xid=%lld\n",
- - (long)status,
- - opcode2string(entry->opcode), entry, entry->xid);
- + "returned UNEXPECTED status(=%ld)\n",
- + (long)status);
- goto out;
- }
- }
- @@ -1608,7 +1630,7 @@ void unmarshal_nfs41_header(
- *buf += sizeof(tmp->errno);
- #ifdef DEBUG_MARSHAL_HEADER
- DbgP("[downcall header] xid=%lld opcode=%s status=%d errno=%d\n", tmp->xid,
- - opcode2string(tmp->opcode), tmp->status, tmp->errno);
- + ENTRY_OPCODE2STRING(tmp), tmp->status, tmp->errno);
- #endif
- }
- --
- 2.42.1
msnfs41client: Fixes for stability, crashes in debug kernel module and libtirc, 2023-11-09
Posted by Anonymous on Thu 9th Nov 2023 15:12
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.