SSV-26770 , SSV-26896 CodeQL must fix error fixes

include/os/windows/spl/sys/kmem.h

@@ -61,10 +61,30 @@ extern uint64_t physmem;
 	 */
 
 #define	MALLOC(A, C, S, T, F) \
-	(A) = (C)ExAllocatePoolWithTag(NonPagedPoolNx, (S), '!SFZ')
+	(A) = (C)ExAllocatePoolUninitialized(NonPagedPoolNx, (S), '!SFZ')
 #define	FREE(A, T) \
 	ExFreePoolWithTag((A), '!SFZ')
 
+/*
+ * Centralizes the "allocate uninitialized, then zero on success"
+ * pattern used throughout the Windows port, in one place, so the
+ * allocation size can never drift between the alloc call and the
+ * zero call (two historical call sites had exactly that bug - see
+ * zfs_windows_zvol.c's zvol_start() and zfs_vnops_windows.c's
+ * pnp_query_id(), before this was centralized). A real function,
+ * not a macro: a macro that referenced its Size argument twice would
+ * silently reintroduce the same double-evaluation bug for any future
+ * caller passing a computed expression.
+ */
+static __inline PVOID
+spl_ExAllocatePoolZero(POOL_TYPE PoolType, SIZE_T Size, ULONG Tag)
+{
+	PVOID ptr = ExAllocatePoolUninitialized(PoolType, Size, Tag);
+	if (ptr != NULL)
+		RtlZeroMemory(ptr, Size);
+	return (ptr);
+}
+
 // Work around symbol collisions in XNU
 #define	kmem_alloc(size, kmflags) zfs_kmem_alloc((size), (kmflags))
 #define	kmem_zalloc(size, kmflags) zfs_kmem_zalloc((size), (kmflags))

include/os/windows/spl/sys/types.h

@@ -94,12 +94,106 @@ typedef uintptr_t pc_t;
 #include <ntstrsafe.h>
 #include <stdlib.h>
 #include <ntddk.h>
+#include <stdarg.h>
 
-
-#define	snprintf _snprintf
+/*
+ * Kernel-mode _snprintf() returns -1 on truncation (not the would-be
+ * length) and does not NUL-terminate the buffer on truncation, unlike
+ * standard snprintf(). Portable ZFS/SPL code assumes real snprintf()
+ * semantics, so give it those semantics here rather than the raw
+ * deprecated function.
+ *
+ * There is no _vscprintf() in ntoskrnl.lib, and ntstrsafe.h's
+ * String RtlStringCchPrintfEx family cannot measure a formatted
+ * string's length without a real, non-zero destination buffer (a
+ * cchDest of 0 short-circuits before formatting even happens) - so
+ * "how long would this be" can only be discovered by actually
+ * formatting into a real, possibly-grown, scratch buffer.
+ *
+ * spl_vsnprintf() is implemented out-of-line in
+ * module/os/windows/spl/spl-kmem.c, NOT as a static inline here,
+ * because that implementation needs kmem_alloc()/kmem_free() -
+ * sys/kmem.h itself #includes sys/types.h, so an inline definition
+ * here could never see kmem_alloc()'s declaration without an
+ * unsupportable circular include.
+ */
+extern int spl_vsnprintf(char *buf, size_t size, const char *fmt,
+    va_list args);
+
+static __inline int
+spl_snprintf(char *buf, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	int ret;
+
+	va_start(args, fmt);
+	ret = spl_vsnprintf(buf, size, fmt, args);
+	va_end(args);
+	return (ret);
+}
+
+#define	snprintf spl_snprintf
 #define	vprintf(...) vKdPrintEx((DPFLTR_IHVDRIVER_ID, DPFLTR_INFO_LEVEL, \
 	__VA_ARGS__))
-#define	vsnprintf _vsnprintf
+/*
+ * No #define vsnprintf here (unlike snprintf above): CodeQL's
+ * extended-deprecated-apis check flags macro invocations by the
+ * macro's own name against Microsoft's banned-API list, regardless of
+ * what the macro expands to - "vsnprintf" (no underscore) is on that
+ * list, "snprintf" is not. A macro named vsnprintf can never pass the
+ * check no matter its target, so every caller below calls
+ * spl_vsnprintf directly instead of going through a same-named macro.
+ */
+
+/*
+ * Kernel-mode strncpy() does not NUL-terminate if strlen(src) >= n,
+ * and zero-fills the whole remainder of the buffer if strlen(src) < n
+ * - neither behavior is depended on by any of this codebase's callers.
+ * strlcpy() is the semantically-closest safe replacement (always
+ * terminates, never overflows) but has no kernel-linkable
+ * implementation here, so provide one - mirrors lib/libspl/strlcpy.c's
+ * existing user-mode algorithm exactly. Callers pass n == the size of
+ * the destination buffer (or the intended-substring-length + 1),
+ * unlike strncpy's n == copy-length - not a drop-in same-args swap.
+ */
+static __inline size_t
+spl_strlcpy(char *dst, const char *src, size_t dstsize)
+{
+	size_t srclen = strlen(src);
+	size_t copied = (srclen < dstsize) ? srclen : dstsize - 1;
+
+	if (dstsize != 0) {
+		memcpy(dst, src, copied);
+		dst[copied] = '\0';
+	}
+	return (srclen);
+}
+
+/*
+ * strcat() has no size parameter at all - unbounded by construction.
+ * strlcat() is the closest safe replacement (always terminates, never
+ * overflows, return value is the total length it tried to create) but
+ * - same as strlcpy() - has no kernel-linkable implementation here.
+ * Mirrors lib/libspl/strlcat.c's existing user-mode algorithm exactly.
+ */
+static __inline size_t
+spl_strlcat(char *dst, const char *src, size_t dstsize)
+{
+	char *df = dst;
+	size_t left = dstsize;
+	size_t l1, l2 = strlen(src), copied;
+
+	while (left-- != 0 && *df != '\0')
+		df++;
+	l1 = df - dst;
+	if (dstsize == l1)
+		return (l1 + l2);
+
+	copied = (l1 + l2 >= dstsize) ? dstsize - l1 - 1 : l2;
+	memcpy(dst + l1, src, copied);
+	dst[l1 + copied] = '\0';
+	return (l1 + l2);
+}
 
 #ifndef ULLONG_MAX
 #define	ULLONG_MAX			(~0ULL)

lib/libspl/include/os/windows/sys/types.h

@@ -228,4 +228,47 @@ typedef uint64_t zoff_t;
 #include <wosix.h>
 #endif
 
+#include <string.h>
+#include <stdio.h>
+
+/*
+ * Mirrors include/os/windows/spl/sys/types.h's kernel-mode shims of the
+ * same name. Several shared module/zfs, module/icp, and module/lua
+ * source files (built both into the ZFSin kernel driver and into
+ * user-mode libzpool/libicp/zlib here) call these directly by name, not
+ * through a portable macro. In user mode, real strlcpy/strlcat
+ * (lib/libspl) and real, C99-conformant UCRT vsnprintf are already
+ * available, so these are simple passthroughs - no downlevel-
+ * unavailability workaround is needed here, unlike the kernel version.
+ */
+static __inline size_t
+spl_strlcpy(char *dst, const char *src, size_t dstsize)
+{
+	return (strlcpy(dst, src, dstsize));
+}
+
+static __inline size_t
+spl_strlcat(char *dst, const char *src, size_t dstsize)
+{
+	return (strlcat(dst, src, dstsize));
+}
+
+static __inline int
+spl_vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
+{
+	return (vsnprintf(buf, size, fmt, args));
+}
+
+static __inline int
+spl_snprintf(char *buf, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	int ret;
+
+	va_start(args, fmt);
+	ret = spl_vsnprintf(buf, size, fmt, args);
+	va_end(args);
+	return (ret);
+}
+
 #endif

lib/libzfs/CMakeLists.txt

@@ -20,20 +20,33 @@ add_library(libzfs
 	os/windows/libzfs_util_os.c
 )
 
-#variable_watch(CRYPTO_STATIC)
-# set(CRYPTO_STATIC "notset")
-set(CMAKE_FIND_DEBUG_MODE TRUE)
-find_library(CRYPTO_STATIC_TEST
+# find_package(OpenSSL)'s own LIB_EAY_DEBUG/LIB_EAY_RELEASE search (see
+# contrib/windows/cmake/FindOpenSSL.cmake) correctly locates the CRT-
+# matched static libs, but nothing in this tree actually links against
+# OPENSSL_CRYPTO_LIBRARY - hence this direct find_library, picking the
+# variant that matches CMAKE_BUILD_TYPE (a fresh search hardcoded to the
+# MTd/debug name here would silently link the debug-CRT crypto lib into
+# a Release build too). Cache variable name changed from the old
+# CRYPTO_STATIC_TEST so this re-searches on the next configure rather
+# than reusing a stale cached path from before this logic existed.
+if(CMAKE_BUILD_TYPE STREQUAL "Debug")
+	set(_libzfs_openssl_crypto_name libcrypto64MTd)
+else()
+	set(_libzfs_openssl_crypto_name libcrypto64MT)
+endif()
+
+find_library(LIBZFS_OPENSSL_CRYPTO
     NAMES
-      libcrypto64MTd
+      ${_libzfs_openssl_crypto_name}
     NAMES_PER_DIR
 	HINTS
 	  "C:/Program Files/OpenSSL-Win64/lib/VC/static"
     PATH_SUFFIXES
       lib
 	REQUIRED
   )
+unset(_libzfs_openssl_crypto_name)
 
 target_include_directories(libzfs PRIVATE "${CMAKE_SOURCE_DIR}/lib/libzfs")
 target_link_libraries(libzfs PUBLIC libpthread zlib libzutil libshare libzfs_core libnvpair libuutil)
-target_link_libraries(libzfs PRIVATE Crypt32.lib ${CRYPTO_STATIC_TEST})
+target_link_libraries(libzfs PRIVATE Crypt32.lib ${LIBZFS_OPENSSL_CRYPTO})

lib/os/windows/zlib-1.2.3/gzio.c

@@ -132,7 +132,7 @@ gz_open(
 	if (s->path == NULL) {
 		return (destroy(s), (gzFile)Z_NULL);
 	}
-	strcpy(s->path, path); /* do this early for debugging */
+	spl_strlcpy(s->path, path, strlen(path) + 1); /* do this early for debugging */
 
 	s->mode = '\0';
 	do {
@@ -234,7 +234,7 @@ gzdopen(
 
 	if (fd < 0)
 		return ((gzFile)Z_NULL);
-	sprintf(name, "<fd:%d>", fd); /* for debugging */
+	spl_snprintf(name, sizeof (name), "<fd:%d>", fd); /* for debugging */
 
 	return (gz_open(name, mode, fd));
 }
@@ -666,7 +666,7 @@ gzprintf(gzFile file, const char *format, /* args */ ...)
 	va_end(va);
 	len = strlen(buf);
 #else
-	len = vsnprintf(buf, sizeof (buf), format, va);
+	len = spl_vsnprintf(buf, sizeof (buf), format, va);
 	va_end(va);
 #endif
 #endif
@@ -1094,9 +1094,9 @@ gzerror(
 	s->msg = (char *)ALLOC(strlen(s->path) + strlen(m) + 3);
 	if (s->msg == Z_NULL)
 		return ((const char *)ERR_MSG(Z_MEM_ERROR));
-	strcpy(s->msg, s->path);
-	strcat(s->msg, ": ");
-	strcat(s->msg, m);
+	spl_strlcpy(s->msg, s->path, strlen(s->path) + strlen(m) + 3);
+	spl_strlcat(s->msg, ": ", strlen(s->path) + strlen(m) + 3);
+	spl_strlcat(s->msg, m, strlen(s->path) + strlen(m) + 3);
 	return ((const char *)s->msg);
 }
 

module/icp/core/kcf_mech_tabs.c

@@ -177,72 +177,72 @@ kcf_init_mech_tabs(void)
 	/* Then the pre-defined mechanism entries */
 
 	/* Two digests */
-	(void) strncpy(kcf_digest_mechs_tab[0].me_name, SUN_CKM_MD5,
+	(void) spl_strlcpy(kcf_digest_mechs_tab[0].me_name, SUN_CKM_MD5,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_digest_mechs_tab[0].me_threshold = kcf_md5_threshold;
 
-	(void) strncpy(kcf_digest_mechs_tab[1].me_name, SUN_CKM_SHA1,
+	(void) spl_strlcpy(kcf_digest_mechs_tab[1].me_name, SUN_CKM_SHA1,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_digest_mechs_tab[1].me_threshold = kcf_sha1_threshold;
 
 	/* The symmetric ciphers in various modes */
-	(void) strncpy(kcf_cipher_mechs_tab[0].me_name, SUN_CKM_DES_CBC,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[0].me_name, SUN_CKM_DES_CBC,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[0].me_threshold = kcf_des_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[1].me_name, SUN_CKM_DES3_CBC,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[1].me_name, SUN_CKM_DES3_CBC,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[1].me_threshold = kcf_des3_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[2].me_name, SUN_CKM_DES_ECB,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[2].me_name, SUN_CKM_DES_ECB,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[2].me_threshold = kcf_des_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[3].me_name, SUN_CKM_DES3_ECB,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[3].me_name, SUN_CKM_DES3_ECB,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[3].me_threshold = kcf_des3_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[4].me_name, SUN_CKM_BLOWFISH_CBC,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[4].me_name, SUN_CKM_BLOWFISH_CBC,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[4].me_threshold = kcf_bf_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[5].me_name, SUN_CKM_BLOWFISH_ECB,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[5].me_name, SUN_CKM_BLOWFISH_ECB,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[5].me_threshold = kcf_bf_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[6].me_name, SUN_CKM_AES_CBC,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[6].me_name, SUN_CKM_AES_CBC,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[6].me_threshold = kcf_aes_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[7].me_name, SUN_CKM_AES_ECB,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[7].me_name, SUN_CKM_AES_ECB,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[7].me_threshold = kcf_aes_threshold;
 
-	(void) strncpy(kcf_cipher_mechs_tab[8].me_name, SUN_CKM_RC4,
+	(void) spl_strlcpy(kcf_cipher_mechs_tab[8].me_name, SUN_CKM_RC4,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_cipher_mechs_tab[8].me_threshold = kcf_rc4_threshold;
 
 
 	/* 4 HMACs */
-	(void) strncpy(kcf_mac_mechs_tab[0].me_name, SUN_CKM_MD5_HMAC,
+	(void) spl_strlcpy(kcf_mac_mechs_tab[0].me_name, SUN_CKM_MD5_HMAC,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_mac_mechs_tab[0].me_threshold = kcf_md5_threshold;
 
-	(void) strncpy(kcf_mac_mechs_tab[1].me_name, SUN_CKM_MD5_HMAC_GENERAL,
+	(void) spl_strlcpy(kcf_mac_mechs_tab[1].me_name, SUN_CKM_MD5_HMAC_GENERAL,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_mac_mechs_tab[1].me_threshold = kcf_md5_threshold;
 
-	(void) strncpy(kcf_mac_mechs_tab[2].me_name, SUN_CKM_SHA1_HMAC,
+	(void) spl_strlcpy(kcf_mac_mechs_tab[2].me_name, SUN_CKM_SHA1_HMAC,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_mac_mechs_tab[2].me_threshold = kcf_sha1_threshold;
 
-	(void) strncpy(kcf_mac_mechs_tab[3].me_name, SUN_CKM_SHA1_HMAC_GENERAL,
+	(void) spl_strlcpy(kcf_mac_mechs_tab[3].me_name, SUN_CKM_SHA1_HMAC_GENERAL,
 	    CRYPTO_MAX_MECH_NAME);
 	kcf_mac_mechs_tab[3].me_threshold = kcf_sha1_threshold;
 
 
 	/* 1 random number generation pseudo mechanism */
-	(void) strncpy(kcf_misc_mechs_tab[0].me_name, SUN_RANDOM,
+	(void) spl_strlcpy(kcf_misc_mechs_tab[0].me_name, SUN_RANDOM,
 	    CRYPTO_MAX_MECH_NAME);
 
 	kcf_mech_hash = mod_hash_create_strhash_nodtr("kcf mech2id hash",

module/icp/spi/kcf_spi.c

@@ -606,7 +606,7 @@ init_prov_mechs(crypto_provider_info_t *info, kcf_provider_desc_t *desc)
 			rand_mi = &desc->pd_mechanisms[mcount - 1];
 
 			bzero(rand_mi, sizeof (crypto_mech_info_t));
-			(void) strncpy(rand_mi->cm_mech_name, SUN_RANDOM,
+			(void) spl_strlcpy(rand_mi->cm_mech_name, SUN_RANDOM,
 			    CRYPTO_MAX_MECH_NAME);
 			rand_mi->cm_func_group_mask = CRYPTO_FG_RANDOM;
 		} else {

module/lua/lcompat.c

@@ -12,7 +12,7 @@ lcompat_sprintf(char *buf, size_t size, const char *fmt, ...)
 	va_list args;
 
 	va_start(args, fmt);
-	res = vsnprintf(buf, size, fmt, args);
+	res = spl_vsnprintf(buf, size, fmt, args);
 	va_end(args);
 
 	return (res);

module/lua/lstrlib.c

@@ -37,7 +37,7 @@ static size_t str_sprintf(char *buf, const char *fmt, ...) {
   size_t len;
 
   va_start(args, fmt);
-  len = vsnprintf(buf, INT_MAX, fmt, args);
+  len = spl_vsnprintf(buf, INT_MAX, fmt, args);
   va_end(args);
 
   return len;