From ee89685577fa862f4762eab618cfcf98d0219acb Mon Sep 17 00:00:00 2001 From: Senthil <79847390+datacore-senthil@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:34:21 +0530 Subject: [PATCH 1/2] Revert "Ssv 26770 Fixed all the Codeql must fix errors related to ZFSin driver (#113)" (#117) This reverts commit 79b7320add0035a836506a7a24f6e07980e04c95. (cherry picked from commit c2cad82a49fe74ff8127dc4aa727293d85b05454) --- .../codeql/Invoke-CodeQLZFSinAnalysis.ps1 | 187 ------------------ include/os/windows/spl/sys/kmem.h | 2 +- include/os/windows/spl/sys/types.h | 90 +-------- lib/libspl/include/os/windows/sys/types.h | 12 -- lib/os/windows/zlib-1.2.3/gzio.c | 12 +- lib/os/windows/zlib-1.2.3/zutil.h | 96 +-------- module/icp/core/kcf_mech_tabs.c | 32 +-- module/icp/spi/kcf_spi.c | 2 +- module/lua/lcompat.c | 2 +- module/lua/lstrlib.c | 2 +- module/os/windows/debug.c | 25 +-- module/os/windows/driver.c | 2 +- module/os/windows/spl/spl-err.c | 2 +- module/os/windows/spl/spl-kmem.c | 12 +- module/os/windows/spl/spl-kstat.c | 9 +- module/os/windows/spl/spl-proc_list.c | 4 +- module/os/windows/spl/spl-seg_kmem.c | 2 +- module/os/windows/spl/spl-taskq.c | 2 +- module/os/windows/spl/spl-windows.c | 4 +- module/os/windows/zfs/zfs_ctldir.c | 4 +- module/os/windows/zfs/zfs_debug.c | 14 +- module/os/windows/zfs/zfs_ioctl_os.c | 4 +- module/os/windows/zfs/zfs_vnops_windows.c | 48 +---- module/os/windows/zfs/zfs_vnops_windows_lib.c | 8 +- .../os/windows/zfs/zfs_vnops_windows_mount.c | 6 +- module/os/windows/zfs/zfs_windows_zvol.c | 11 +- module/os/windows/zfs/zfs_windows_zvol_scsi.c | 10 +- module/zcommon/zfs_fletcher.c | 14 +- module/zfs/dmu_send.c | 4 +- module/zfs/dsl_dir.c | 4 +- module/zfs/dsl_prop.c | 5 +- module/zfs/spa_misc.c | 6 +- module/zfs/vdev.c | 2 +- module/zfs/zcp.c | 14 +- module/zfs/zcp_get.c | 2 +- module/zfs/zfs_ioctl.c | 2 +- module/zfs/zio.c | 2 +- module/zfs/zio_inject.c | 2 +- 38 files changed, 100 insertions(+), 561 deletions(-) delete mode 100644 contrib/windows/codeql/Invoke-CodeQLZFSinAnalysis.ps1 diff --git a/contrib/windows/codeql/Invoke-CodeQLZFSinAnalysis.ps1 b/contrib/windows/codeql/Invoke-CodeQLZFSinAnalysis.ps1 deleted file mode 100644 index e04c0500c25e..000000000000 --- a/contrib/windows/codeql/Invoke-CodeQLZFSinAnalysis.ps1 +++ /dev/null @@ -1,187 +0,0 @@ -# ***************************************************************************** -# Copyright (c) 2026 DataCore Software Corporation. All rights reserved. -# ***************************************************************************** - -<# - .SYNOPSIS - Runs CodeQL static analysis over the ZFSin kernel driver target only - (not the full CMake build) and reports the Must-Fix findings required - for the HLK "Static Tools Logo Test" (WHCP certification). - - .DESCRIPTION - 1. Builds a CodeQL database by tracing a CMake+Ninja build of ONLY the - `ZFSin` target (the driver .sys and everything statically linked - into it: splkern, zlibkern, icpkern, luakern, zfskern, zfskern_os, - zcommonkern, nvpairkern, unicodekern, zstdkern). This deliberately - excludes the user-mode tools (zfs.exe, zpool.exe, zfsinstaller.exe, - etc.) and user-mode libraries (libzfs, libnvpair, libzpool, ...) - - several of those share source files with the driver (e.g. - module/zfs/*.c is also compiled into libzpool) but under different - macros/headers, and mixing both into one database produces - confusing multi-context findings for the same source line. - 2. Analyzes the database with the WHCP `mustfix.qls` suite, writing - SARIF to the repo root. - 3. Prints a summary grouped by rule/API. - - .PREREQUISITES - - CodeQL CLI 2.20.1 unpacked to -CodeQLHome (WHCP matrix version). - - Query packs downloaded: microsoft/windows-drivers@1.8.0 and - microsoft/cpp-queries@0.0.4. - - VS2019 (vcvars64.bat) + WDK 10.0.19041 + OpenSSL-Win64 + the - prebuilt ISA-L static libs under lib\ISA-L\ - all - already required by this repo's normal CMake build. - - .EXAMPLE - .\Invoke-CodeQLZFSinAnalysis.ps1 -#> - -PARAM -( - [Parameter(HelpMessage = "Directory containing the CodeQL CLI (codeql.exe)")] - [string]$CodeQLHome = "C:\codeql-home\codeql", - - [Parameter(HelpMessage = "WHCP query suite to run")] - [ValidateSet("mustfix", "recommended", "mustrun")] - [string]$Suite = "mustfix", - - [Parameter(HelpMessage = "CMake build configuration")] - [ValidateSet("Debug", "Release")] - [string]$Configuration = "Debug", - - [Parameter(HelpMessage = "Path to vcvars64.bat (VS2019)")] - [string]$VcVars64 = "C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Auxiliary\Build\vcvars64.bat", - - [Parameter(HelpMessage = "Directory for the CodeQL database")] - [string]$DatabasePath, - - [Parameter(HelpMessage = "Output SARIF path")] - [string]$SarifPath, - - [Parameter(HelpMessage = "Reuse an existing CodeQL database instead of rebuilding it")] - [switch]$ReuseDatabase -) - -$ErrorActionPreference = "Stop" - -# Repo root is two levels up from contrib\windows\codeql. -$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot "..\..\..")).Path - -if (!$DatabasePath) { $DatabasePath = Join-Path $repoRoot "out\CodeQL\databases\ZFSin" } -if (!$SarifPath) { $SarifPath = Join-Path $repoRoot "ZFSin.codeql.sarif" } - -$buildDir = Join-Path $repoRoot "out\build\codeql-zfsin" - -function Invoke-Tool([string]$Exe, [string[]]$ToolArgs) -{ - $prevEap = $script:ErrorActionPreference - $script:ErrorActionPreference = "Continue" - try - { - & $Exe @ToolArgs 2>&1 | ForEach-Object { Write-Host $_.ToString() } - return $LASTEXITCODE - } - finally - { - $script:ErrorActionPreference = $prevEap - } -} - -$codeqlExe = Join-Path $CodeQLHome "codeql.exe" -if (!(Test-Path $codeqlExe)) -{ - $nested = Join-Path $CodeQLHome "codeql\codeql.exe" - if (Test-Path $nested) { $codeqlExe = $nested } -} -if (!(Test-Path $codeqlExe)) -{ - throw "codeql.exe not found under '$CodeQLHome'." -} - -if (!(Test-Path $VcVars64)) -{ - throw "vcvars64.bat not found at '$VcVars64'. Pass -VcVars64 explicitly." -} - -$suiteSpec = "microsoft/windows-drivers:windows-driver-suites\$Suite.qls" - -Write-Host "Repo root : $repoRoot" -Write-Host "CodeQL : $codeqlExe" -Write-Host "Query suite : $suiteSpec" -Write-Host "Build dir : $buildDir" -Write-Host "Database : $DatabasePath" -Write-Host "SARIF output : $SarifPath" -Write-Host "" - -# Fail early if the WHCP query packs are missing. -$prevEap = $ErrorActionPreference -$ErrorActionPreference = "Continue" -$qlpacks = (& $codeqlExe resolve packs 2>&1 | ForEach-Object { $_.ToString() }) -join "`n" -$ErrorActionPreference = $prevEap -if ($qlpacks -notmatch "microsoft/windows-drivers") -{ - throw "Query pack microsoft/windows-drivers not found. Run: codeql pack download microsoft/windows-drivers@1.8.0 (and microsoft/cpp-queries@0.0.4)." -} - -New-Item -ItemType Directory -Force (Split-Path $DatabasePath -Parent) > $null - -if ($ReuseDatabase -and (Test-Path (Join-Path $DatabasePath "codeql-database.yml"))) -{ - Write-Host "Reusing existing database $DatabasePath" -} -else -{ - # Build script: vcvars64 -> configure (only if needed) -> build ONLY the - # ZFSin target (not the default `all` target, which would also build the - # unrelated user-mode tools/libraries). - $buildScript = Join-Path $env:TEMP "build-zfsin-codeql.cmd" - @" -@echo off -setlocal -call "$VcVars64" -if errorlevel 1 exit /b 1 -set "CMAKE=%ProgramFiles(x86)%\Microsoft Visual Studio\2019\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -set "NINJA=%ProgramFiles(x86)%\Microsoft Visual Studio\2019\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja\ninja.exe" -cd /d "$repoRoot" -if not exist "$buildDir\CMakeCache.txt" ( - "%CMAKE%" -S . -B "$buildDir" -G Ninja -DCMAKE_BUILD_TYPE=$Configuration -DCMAKE_MAKE_PROGRAM="%NINJA%" - if errorlevel 1 exit /b 1 -) -"%CMAKE%" --build "$buildDir" --target ZFSin --clean-first -if errorlevel 1 exit /b 1 -exit /b 0 -"@ | Out-File -FilePath $buildScript -Encoding ASCII - - if (Test-Path $DatabasePath) { Remove-Item -Recurse -Force $DatabasePath } - $code = Invoke-Tool $codeqlExe @("database", "create", $DatabasePath, "--language=cpp", - "--source-root=$repoRoot", "--command=$buildScript", "--overwrite") - if ($code -ne 0) { throw "codeql database create failed (exit $code)" } -} - -$code = Invoke-Tool $codeqlExe @("database", "analyze", $DatabasePath, $suiteSpec, - "--format=sarifv2.1.0", "--output=$SarifPath", "--rerun") -if ($code -ne 0) { throw "codeql database analyze failed (exit $code)" } - -$sarif = Get-Content $SarifPath -Raw | ConvertFrom-Json -$findings = @() -foreach ($run in $sarif.runs) { if ($run.results) { $findings += $run.results } } - -Write-Host "" -Write-Host "================ ZFSin ($Suite) ================" -ForegroundColor Cyan -if ($findings.Count -eq 0) -{ - Write-Host "clean - 0 findings" -ForegroundColor Green -} -else -{ - Write-Host "$($findings.Count) finding(s):" -ForegroundColor Yellow - $findings | Group-Object ruleId | Sort-Object Count -Descending | ForEach-Object { - Write-Host (" {0,4} x {1}" -f $_.Count, $_.Name) -ForegroundColor Yellow - } - if ($Suite -eq "mustfix") - { - Write-Host "" - Write-Host "FAILS certification until fixed" -ForegroundColor Red - } -} - -exit $findings.Count diff --git a/include/os/windows/spl/sys/kmem.h b/include/os/windows/spl/sys/kmem.h index 85725ca5a1e2..f457df156439 100644 --- a/include/os/windows/spl/sys/kmem.h +++ b/include/os/windows/spl/sys/kmem.h @@ -61,7 +61,7 @@ extern uint64_t physmem; */ #define MALLOC(A, C, S, T, F) \ - (A) = (C)ExAllocatePoolUninitialized(NonPagedPoolNx, (S), '!SFZ') + (A) = (C)ExAllocatePoolWithTag(NonPagedPoolNx, (S), '!SFZ') #define FREE(A, T) \ ExFreePoolWithTag((A), '!SFZ') diff --git a/include/os/windows/spl/sys/types.h b/include/os/windows/spl/sys/types.h index 04d65ebe5952..40cb9b22a3a4 100644 --- a/include/os/windows/spl/sys/types.h +++ b/include/os/windows/spl/sys/types.h @@ -96,96 +96,10 @@ typedef uintptr_t pc_t; #include -#include -#ifndef va_copy -/* - * clang-cl provides va_copy as a compiler builtin, but the WDK's own - * kernel-mode CRT stdarg.h (km\crt\stdarg.h, used when this header is - * compiled with plain cl.exe rather than clang-cl) does not define it at - * all. This driver is AMD64/x64-only, where va_list is a plain pointer - * and a direct assignment is a correct, equivalent substitute. - */ -#define va_copy(dest, src) ((dest) = (src)) -#endif -/* - * _snprintf()/_vsnprintf() (the legacy MSVCRT functions snprintf/vsnprintf - * were aliased to below) do not null-terminate the destination buffer when - * the formatted output is truncated - unlike the POSIX snprintf/vsnprintf - * this portable code is written against. Wrap them instead of aliasing - * directly, so truncation is always still safely null-terminated. The - * existing "-1 on truncation" return value is preserved unchanged (every - * caller in this tree only checks `if (n < 0)`), so this is purely additive. - */ -/* - * "Measure the required length without writing" (the buf==NULL/size==0 - * idiom used by kmem_asprintf()/kmem_vasprintf()/zfs_dbgmsg()). Neither - * _vscprintf (declared in the WDK headers but not exported by the - * kernel-mode CRT import lib - confirmed via a link failure) nor - * _vsnprintf_s (its count==0 case triggers the invalid-parameter handler) - * can do this directly in kernel mode. Measure into a generously-sized - * scratch buffer instead: every caller in this tree builds short, bounded - * strings (dataset/snapshot names, log messages), so 1024 bytes is never - * exceeded in practice. If a caller's format+args ever did exceed it, the - * result here is a consistently-truncated (safely null-terminated) length - * - the caller's later real write with the same format+args into a - * same-size-or-larger buffer would truncate identically, not silently - * disagree with what was measured. - */ -static inline int -zfs_vscprintf(const char *fmt, va_list ap) -{ - char scratch[1024]; - va_list ap_copy; - int ret; - - va_copy(ap_copy, ap); - ret = _vsnprintf_s(scratch, sizeof (scratch), (size_t)-1, fmt, ap_copy); - va_end(ap_copy); - - return (ret >= 0 ? ret : (int)sizeof (scratch) - 1); -} - -static inline int -zfs_vsnprintf(char *buf, size_t size, const char *fmt, va_list ap) -{ - int ret; - - if (size == 0) { - va_list ap_copy; - va_copy(ap_copy, ap); - ret = zfs_vscprintf(fmt, ap_copy); - va_end(ap_copy); - return (ret); - } - - /* - * _TRUNCATE ((size_t)-1): _vsnprintf_s always null-terminates buf - * itself on truncation (returning -1), so no separate fallback - * write is needed here - callers such as lstrlib.c's str_sprintf() - * pass INT_MAX as a "the caller already pre-sized the real buffer" - * sentinel, not the true size of buf, so a manual buf[size - 1] - * write here would be a wild out-of-bounds write on truncation. - */ - return (_vsnprintf_s(buf, size, (size_t)-1, fmt, ap)); -} - -static inline int -zfs_snprintf(char *buf, size_t size, const char *fmt, ...) -{ - va_list ap; - int ret; - - va_start(ap, fmt); - ret = zfs_vsnprintf(buf, size, fmt, ap); - va_end(ap); - - return (ret); -} - -#define snprintf zfs_snprintf +#define snprintf _snprintf #define vprintf(...) vKdPrintEx((DPFLTR_IHVDRIVER_ID, DPFLTR_INFO_LEVEL, \ __VA_ARGS__)) -#define vsnprintf zfs_vsnprintf +#define vsnprintf _vsnprintf #ifndef ULLONG_MAX #define ULLONG_MAX (~0ULL) diff --git a/lib/libspl/include/os/windows/sys/types.h b/lib/libspl/include/os/windows/sys/types.h index a8a2a3d7f26f..10090c77652a 100644 --- a/lib/libspl/include/os/windows/sys/types.h +++ b/lib/libspl/include/os/windows/sys/types.h @@ -228,16 +228,4 @@ typedef uint64_t zoff_t; #include #endif -/* - * Several source files under module/ (module/zfs, module/lua, ...) are - * shared between the ZFSin kernel driver and this user-mode build (e.g. - * libzpool) and call zfs_vsnprintf() directly by name - the kernel-side - * include/os/windows/spl/sys/types.h defines that name as a safe wrapper - * around the legacy, non-null-terminating-on-truncation kernel-mode - * _vsnprintf. This user-mode types.h has no such problem (the real UCRT - * vsnprintf() is already POSIX-conformant), so just alias the name to it. - */ -#include -#define zfs_vsnprintf vsnprintf - #endif diff --git a/lib/os/windows/zlib-1.2.3/gzio.c b/lib/os/windows/zlib-1.2.3/gzio.c index 75e44fcc5229..8a8181c80b31 100644 --- a/lib/os/windows/zlib-1.2.3/gzio.c +++ b/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); } - strlcpy(s->path, path, strlen(path)+1); /* do this early for debugging */ + strcpy(s->path, path); /* do this early for debugging */ s->mode = '\0'; do { @@ -234,7 +234,7 @@ gzdopen( if (fd < 0) return ((gzFile)Z_NULL); - zlib_snprintf(name, sizeof (name), "", fd); /* for debugging */ + sprintf(name, "", 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 = zlib_vsnprintf(buf, sizeof (buf), format, va); + len = 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)); - strlcpy(s->msg, s->path, strlen(s->path) + strlen(m) + 3); - strlcat(s->msg, ": ", strlen(s->path) + strlen(m) + 3); - strlcat(s->msg, m, strlen(s->path) + strlen(m) + 3); + strcpy(s->msg, s->path); + strcat(s->msg, ": "); + strcat(s->msg, m); return ((const char *)s->msg); } diff --git a/lib/os/windows/zlib-1.2.3/zutil.h b/lib/os/windows/zlib-1.2.3/zutil.h index f1517926936b..c1562c1a95aa 100644 --- a/lib/os/windows/zlib-1.2.3/zutil.h +++ b/lib/os/windows/zlib-1.2.3/zutil.h @@ -178,100 +178,6 @@ typedef int ptrdiff_t; /* functions */ -#ifdef WIN32 -/* - * Callers in this file call zlib_vsnprintf() directly (not via a "vsnprintf" - * macro alias) so the safe wrapper is always used regardless of whether this - * compiler/CRT already exposes some form of vsnprintf. _vsnprintf_s (unlike - * the legacy _vsnprintf) always null-terminates the destination buffer, even - * when the formatted output is truncated, matching the POSIX vsnprintf - * contract this code is written against. - */ -#include -#ifndef va_copy -/* - * clang-cl provides va_copy as a compiler builtin, but the WDK's own - * kernel-mode CRT stdarg.h (km\crt\stdarg.h, used when this header is - * compiled with plain cl.exe rather than clang-cl) does not define it at - * all. This driver is AMD64/x64-only, where va_list is a plain pointer - * and a direct assignment is a correct, equivalent substitute. - */ -#define va_copy(dest, src) ((dest) = (src)) -#endif - -/* - * "Measure the required length without writing" (buf==NULL/size==0). - * Neither _vscprintf (declared in the WDK headers but not exported by the - * kernel-mode CRT import lib - confirmed via a link failure) nor - * _vsnprintf_s (its count==0 case triggers the invalid-parameter handler) - * can do this directly in kernel mode. Measure into a generously-sized - * scratch buffer instead: this file's only caller formats a short - * "" tag and short error messages, never anything close to 1024 - * bytes. If that ever changed, the result here is a consistently - * truncated (safely null-terminated) length - a later real write with the - * same format+args into a same-size-or-larger buffer would truncate - * identically, not silently disagree with what was measured. - */ -static inline int -zlib_vscprintf(const char *fmt, va_list ap) -{ - char scratch[1024]; - va_list ap_copy; - int ret; - - va_copy(ap_copy, ap); - ret = _vsnprintf_s(scratch, sizeof (scratch), (size_t)-1, fmt, ap_copy); - va_end(ap_copy); - - return (ret >= 0 ? ret : (int)sizeof (scratch) - 1); -} - -static inline int -zlib_vsnprintf(char *buf, size_t size, const char *fmt, va_list ap) -{ - int ret; - - if (size == 0) { - va_list ap_copy; - va_copy(ap_copy, ap); - ret = zlib_vscprintf(fmt, ap_copy); - va_end(ap_copy); - return (ret); - } - - /* - * _TRUNCATE ((size_t)-1): _vsnprintf_s always null-terminates buf - * itself on truncation (returning -1), so no separate fallback - * write is needed here - a manual buf[size - 1] write would be an - * out-of-bounds write for any future caller that passes a sentinel - * size larger than the true buffer, the same pattern already found - * in module/lua/lstrlib.c's use of the analogous zfs_vsnprintf(). - */ - return (_vsnprintf_s(buf, size, (size_t)-1, fmt, ap)); -} - -/* - * Portable (fixed-argument) counterpart to zlib_vsnprintf(), for callers - * that don't already have a va_list - e.g. gzio.c's gzdopen(), which needs - * this to compile both as part of the kernel-mode zlibkern static lib - * (linked into ZFSin) and as part of the plain user-mode zlib library used - * by minigzip.c/test tooling. RtlStringCbPrintfA (ntstrsafe.h) would only - * be usable from the former. - */ -static inline int -zlib_snprintf(char *buf, size_t size, const char *fmt, ...) -{ - va_list ap; - int ret; - - va_start(ap, fmt); - ret = zlib_vsnprintf(buf, size, fmt, ap); - va_end(ap); - - return (ret); -} -#endif - #if defined(STDC99) || (defined(__TURBOC__) && __TURBOC__ >= 0x550) #ifndef HAVE_VSNPRINTF #define HAVE_VSNPRINTF @@ -296,7 +202,7 @@ zlib_snprintf(char *buf, size_t size, const char *fmt, ...) #ifdef WIN32 /* In Win32, vsnprintf is available as the "non-ANSI" _vsnprintf. */ #if !defined(vsnprintf) && !defined(NO_vsnprintf) -#define vsnprintf zlib_vsnprintf +#define vsnprintf _vsnprintf #endif #endif #ifdef __SASC diff --git a/module/icp/core/kcf_mech_tabs.c b/module/icp/core/kcf_mech_tabs.c index a4a5380925e3..2642b317d698 100644 --- a/module/icp/core/kcf_mech_tabs.c +++ b/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) strlcpy(kcf_digest_mechs_tab[0].me_name, SUN_CKM_MD5, + (void) strncpy(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) strlcpy(kcf_digest_mechs_tab[1].me_name, SUN_CKM_SHA1, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[0].me_name, SUN_CKM_DES_CBC, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[1].me_name, SUN_CKM_DES3_CBC, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[2].me_name, SUN_CKM_DES_ECB, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[3].me_name, SUN_CKM_DES3_ECB, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[4].me_name, SUN_CKM_BLOWFISH_CBC, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[5].me_name, SUN_CKM_BLOWFISH_ECB, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[6].me_name, SUN_CKM_AES_CBC, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[7].me_name, SUN_CKM_AES_ECB, + (void) strncpy(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) strlcpy(kcf_cipher_mechs_tab[8].me_name, SUN_CKM_RC4, + (void) strncpy(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) strlcpy(kcf_mac_mechs_tab[0].me_name, SUN_CKM_MD5_HMAC, + (void) strncpy(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) strlcpy(kcf_mac_mechs_tab[1].me_name, SUN_CKM_MD5_HMAC_GENERAL, + (void) strncpy(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) strlcpy(kcf_mac_mechs_tab[2].me_name, SUN_CKM_SHA1_HMAC, + (void) strncpy(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) strlcpy(kcf_mac_mechs_tab[3].me_name, SUN_CKM_SHA1_HMAC_GENERAL, + (void) strncpy(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) strlcpy(kcf_misc_mechs_tab[0].me_name, SUN_RANDOM, + (void) strncpy(kcf_misc_mechs_tab[0].me_name, SUN_RANDOM, CRYPTO_MAX_MECH_NAME); kcf_mech_hash = mod_hash_create_strhash_nodtr("kcf mech2id hash", diff --git a/module/icp/spi/kcf_spi.c b/module/icp/spi/kcf_spi.c index 96b9ea4130d5..34b36b81c0ab 100644 --- a/module/icp/spi/kcf_spi.c +++ b/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) strlcpy(rand_mi->cm_mech_name, SUN_RANDOM, + (void) strncpy(rand_mi->cm_mech_name, SUN_RANDOM, CRYPTO_MAX_MECH_NAME); rand_mi->cm_func_group_mask = CRYPTO_FG_RANDOM; } else { diff --git a/module/lua/lcompat.c b/module/lua/lcompat.c index 099a14298dcf..c0a27182c7d8 100644 --- a/module/lua/lcompat.c +++ b/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 = zfs_vsnprintf(buf, size, fmt, args); + res = vsnprintf(buf, size, fmt, args); va_end(args); return (res); diff --git a/module/lua/lstrlib.c b/module/lua/lstrlib.c index dadb19e786df..12027757bf53 100644 --- a/module/lua/lstrlib.c +++ b/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 = zfs_vsnprintf(buf, INT_MAX, fmt, args); + len = vsnprintf(buf, INT_MAX, fmt, args); va_end(args); return len; diff --git a/module/os/windows/debug.c b/module/os/windows/debug.c index 48394242048f..d97637e44861 100644 --- a/module/os/windows/debug.c +++ b/module/os/windows/debug.c @@ -51,17 +51,8 @@ static unsigned long long startOff = 0; int initDbgCircularBuffer(void) { - cbuf = ExAllocatePoolUninitialized(NonPagedPoolNx, cbuf_size, '!GBD'); - /* - * ASSERT() compiles to a no-op in Release/free builds (DBG not - * defined), so it cannot be relied on to catch an allocation - * failure here - an unconditional RtlZeroMemory(cbuf, ...) right - * after would be a NULL-pointer write that bugchecks the box at - * driver load, with no debugger present to catch the assert. - */ - if (cbuf == NULL) - return (ENOMEM); - RtlZeroMemory(cbuf, cbuf_size); + cbuf = ExAllocatePoolWithTag(NonPagedPoolNx, cbuf_size, '!GBD'); + ASSERT(cbuf); KeInitializeSpinLock(&cbuf_spin); return (0); } @@ -91,13 +82,7 @@ void addbuffer(char *buf) { // unsigned long long writtenBytes = 0; - /* - * cbuf is NULL if initDbgCircularBuffer()'s allocation failed; - * without this check, the first debug message after such a - * failure would dereference NULL below instead of being silently - * dropped. - */ - if (buf && cbuf) { + if (buf) { unsigned long long bufLen = strlen(buf); unsigned long long endLineLen = strlen(endLine); unsigned long long endBufLen = strlen(endBuf); @@ -140,12 +125,12 @@ printBuffer(const char *fmt, ...) va_list args; va_start(args, fmt); char buf[max_line_length]; - RtlStringCbPrintfA(buf, 18, "%p: ", PsGetCurrentThread()); + _snprintf(buf, 18, "%p: ", PsGetCurrentThread()); int tmp = _vsnprintf_s(&buf[17], sizeof (buf), max_line_length, fmt, args); if (tmp >= max_line_length) { - RtlStringCbPrintfA(&buf[17], 17, "buffer too small"); + _snprintf(&buf[17], 17, "buffer too small"); } KeAcquireSpinLock(&cbuf_spin, &level); diff --git a/module/os/windows/driver.c b/module/os/windows/driver.c index 995e27a636d2..5873fae229ef 100644 --- a/module/os/windows/driver.c +++ b/module/os/windows/driver.c @@ -319,7 +319,7 @@ spl_kstat_registry(void *arg, kstat_t *ksp) break; // Something is wrong - or we finished // Allocate space to hold - regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolUninitialized( + regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolWithTag( NonPagedPoolNx, length, 'zfsr'); if (regBuffer == NULL) diff --git a/module/os/windows/spl/spl-err.c b/module/os/windows/spl/spl-err.c index 1767c29b96fe..f54ff5405e27 100644 --- a/module/os/windows/spl/spl-err.c +++ b/module/os/windows/spl/spl-err.c @@ -36,7 +36,7 @@ vcmn_err(int ce, const char *fmt, va_list ap) { char msg[MAXMSGLEN]; - RtlStringCbVPrintfA(msg, MAXMSGLEN - 1, fmt, ap); + _vsnprintf(msg, MAXMSGLEN - 1, fmt, ap); switch (ce) { case CE_IGNORE: diff --git a/module/os/windows/spl/spl-kmem.c b/module/os/windows/spl/spl-kmem.c index 84e604307eb2..54cc47adbae4 100644 --- a/module/os/windows/spl/spl-kmem.c +++ b/module/os/windows/spl/spl-kmem.c @@ -1885,7 +1885,7 @@ kmem_dumppr(char **pp, char *e, const char *format, ...) va_list ap; va_start(ap, format); - n = zfs_vsnprintf(p, e - p, format, ap); + n = vsnprintf(p, e - p, format, ap); va_end(ap); *pp = p + n; } @@ -3560,7 +3560,7 @@ kmem_cache_create( /* * Set cache properties. */ - (void) strlcpy(cp->cache_name, name, KMEM_CACHE_NAMELEN + 1); + (void) strncpy(cp->cache_name, name, KMEM_CACHE_NAMELEN); strident_canon(cp->cache_name, KMEM_CACHE_NAMELEN + 1); cp->cache_bufsize = bufsize; cp->cache_align = align; @@ -6621,13 +6621,13 @@ kmem_asprintf(const char *fmt, ...) char *buf; va_start(adx, fmt); - size = zfs_vsnprintf(NULL, 0, fmt, adx) + 1; + size = _vsnprintf(NULL, 0, fmt, adx) + 1; va_end(adx); buf = kmem_alloc(size, KM_SLEEP); va_start(adx, fmt); - (void) zfs_vsnprintf(buf, size, fmt, adx); + (void) _vsnprintf(buf, size, fmt, adx); va_end(adx); return (buf); @@ -6645,11 +6645,11 @@ kmem_vasprintf(const char *fmt, va_list ap) int size; int r = -1; - size = zfs_vsnprintf(NULL, 0, fmt, ap); + size = vsnprintf(NULL, 0, fmt, ap); if ((size >= 0) && (size < INT_MAX)) { ptr = (char *)kmem_alloc(size + 1, KM_SLEEP); // +1 for null if (ptr) { - r = zfs_vsnprintf(ptr, size + 1, fmt, ap); // +1 for null + r = vsnprintf(ptr, size + 1, fmt, ap); // +1 for null if ((r < 0) || (r > size)) { kmem_free(ptr, size); r = -1; diff --git a/module/os/windows/spl/spl-kstat.c b/module/os/windows/spl/spl-kstat.c index 7bd498c7431f..bd2a2c045b10 100644 --- a/module/os/windows/spl/spl-kstat.c +++ b/module/os/windows/spl/spl-kstat.c @@ -155,7 +155,7 @@ struct sbuf { /* sbuf_new() and family does exist in XNU, but Apple wont let us call them */ #define M_SBUF 105 /* string buffers */ #define SBMALLOC(size) \ - (struct sbuf *)ExAllocatePoolUninitialized(NonPagedPoolNx, (size), '!SFZ') + (struct sbuf *)ExAllocatePoolWithTag(NonPagedPoolNx, (size), '!SFZ') #define SBFREE(buf) ExFreePoolWithTag((buf), '!SFZ') #define SBUF_SETFLAG(s, f) do { (s)->s_flags |= (f); } while (0) @@ -309,7 +309,7 @@ sbuf_vprintf(struct sbuf *s, const char *fmt, va_list ap) do { va_copy(ap_copy, ap); - len = zfs_vsnprintf(&s->s_buf[s->s_len], SBUF_FREESPACE(s) + 1, + len = vsnprintf(&s->s_buf[s->s_len], SBUF_FREESPACE(s) + 1, fmt, ap_copy); // left-side must be assignable. Win tries to set to 0. // va_end(ap_copy); @@ -785,7 +785,7 @@ void kstat_set_string(char *dst, const char *src) { bzero(dst, KSTAT_STRLEN); - (void) strlcpy(dst, src, KSTAT_STRLEN); + (void) strncpy(dst, src, KSTAT_STRLEN - 1); } void @@ -1034,8 +1034,7 @@ kstat_create_zone(const char *ks_module, int ks_instance, const char *ks_name, if (ks_name == NULL) { char buf[KSTAT_STRLEN]; kstat_set_string(buf, ks_module); - (void) RtlStringCbPrintfA(namebuf, sizeof (namebuf), "%s%d", - buf, ks_instance); + (void) sprintf(namebuf, "%s%d", buf, ks_instance); ks_name = namebuf; } diff --git a/module/os/windows/spl/spl-proc_list.c b/module/os/windows/spl/spl-proc_list.c index e88b8717c2cf..1d750447c77c 100644 --- a/module/os/windows/spl/spl-proc_list.c +++ b/module/os/windows/spl/spl-proc_list.c @@ -35,7 +35,7 @@ seq_printf(struct seq_file *f, const char *fmt, ...) va_list adx; va_start(adx, fmt); - (void) zfs_vsnprintf(f->sf_buf, f->sf_size, fmt, adx); + (void) vsnprintf(f->sf_buf, f->sf_size, fmt, adx); va_end(adx); } @@ -81,7 +81,7 @@ procfs_list_addr(kstat_t *ksp, loff_t n) ksp->ks_private1 = list_next(&pl->pl_list, elt); if (ksp->ks_private1) { - p = ExAllocatePoolUninitialized(NonPagedPoolNx, sizeof (*p), '!SFZ'); + p = ExAllocatePoolWithTag(NonPagedPoolNx, sizeof (*p), '!SFZ'); p->pli_pl = pl; p->pli_elt = ksp->ks_private1; } diff --git a/module/os/windows/spl/spl-seg_kmem.c b/module/os/windows/spl/spl-seg_kmem.c index e4b2f7bdfe4a..958d7a4f2acb 100644 --- a/module/os/windows/spl/spl-seg_kmem.c +++ b/module/os/windows/spl/spl-seg_kmem.c @@ -121,7 +121,7 @@ osif_malloc(uint64_t size) #ifdef _KERNEL void *tr = NULL; - tr = ExAllocatePoolUninitialized(NonPagedPoolNx, size, '!SFZ'); + tr = ExAllocatePoolWithTag(NonPagedPoolNx, size, '!SFZ'); ASSERT(P2PHASE(tr, PAGE_SIZE) == 0); if (tr != NULL) { atomic_inc_64(&stat_osif_malloc_success); diff --git a/module/os/windows/spl/spl-taskq.c b/module/os/windows/spl/spl-taskq.c index 556184be695e..499485d904c2 100644 --- a/module/os/windows/spl/spl-taskq.c +++ b/module/os/windows/spl/spl-taskq.c @@ -2458,7 +2458,7 @@ taskq_create_common(const char *name, int instance, int nthreads, pri_t pri, * Make sure the name is 0-terminated, and conforms to the rules for * C indentifiers */ - (void) strlcpy(tq->tq_name, name, TASKQ_NAMELEN + 1); + (void) strncpy(tq->tq_name, name, TASKQ_NAMELEN + 1); strident_canon(tq->tq_name, TASKQ_NAMELEN + 1); tq->tq_flags = flags | TASKQ_CHANGING; diff --git a/module/os/windows/spl/spl-windows.c b/module/os/windows/spl/spl-windows.c index 3faa96058587..3aabbaa91116 100644 --- a/module/os/windows/spl/spl-windows.c +++ b/module/os/windows/spl/spl-windows.c @@ -713,7 +713,7 @@ spl_GetZfsTotalMemory(PUNICODE_STRING RegistryPath) break; // Something is wrong - or we finished // Allocate space to hold - regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolUninitialized( + regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolWithTag( NonPagedPoolNx, length, 'zfsr'); if (regBuffer == NULL) @@ -801,7 +801,7 @@ spl_getZfsPreallocSize(PUNICODE_STRING RegistryPath) break; // Something is wrong - or we finished // Allocate space to hold - regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolUninitialized( + regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolWithTag( NonPagedPoolNx, length, 'zfsr'); if (regBuffer == NULL) diff --git a/module/os/windows/zfs/zfs_ctldir.c b/module/os/windows/zfs/zfs_ctldir.c index 122487df9c49..b3fc0875a7e1 100644 --- a/module/os/windows/zfs/zfs_ctldir.c +++ b/module/os/windows/zfs/zfs_ctldir.c @@ -995,8 +995,8 @@ zfsctl_snapshot_name(zfsvfs_t *zfsvfs, const char *snap_name, int len, if ((strlen(full_name) + 1 + strlen(snap_name)) >= len) return (SET_ERROR(ENAMETOOLONG)); - (void) strlcat(full_name, "@", len); - (void) strlcat(full_name, snap_name, len); + (void) strcat(full_name, "@"); + (void) strcat(full_name, snap_name); return (0); } diff --git a/module/os/windows/zfs/zfs_debug.c b/module/os/windows/zfs/zfs_debug.c index 59eb3f316333..01584506703d 100644 --- a/module/os/windows/zfs/zfs_debug.c +++ b/module/os/windows/zfs/zfs_debug.c @@ -228,7 +228,7 @@ __dprintf(boolean_t dprint, const char *file, const char *func, } va_start(adx, fmt); - size = zfs_vsnprintf(NULL, 0, fmt, adx); + size = vsnprintf(NULL, 0, fmt, adx); va_end(adx); size += snprintf(NULL, 0, "%s%s:%d:%s(): ", prefix, newfile, line, @@ -246,17 +246,7 @@ __dprintf(boolean_t dprint, const char *file, const char *func, va_start(adx, fmt); i = snprintf(buf, size + 1, "%s%s:%d:%s(): ", prefix, newfile, line, func); - /* - * buf has exactly `size` bytes total; `i` bytes are already used by - * the prefix, leaving `size - i` true remaining bytes at buf + i - * (not size - i + 1 - that overstates the real remaining capacity - * by one byte). This was harmless while zfs_vsnprintf's size==0 - * "measure" path could return an unbounded true length, but - * zfs_vscprintf now caps that measurement at a 1023-character - * scratch buffer, so a fmt+args needing >= 1024 characters would - * make this call write one byte past the end of buf. - */ - roger = zfs_vsnprintf(buf + i, size - i, fmt, adx); + roger = vsnprintf(buf + i, size -i + 1, fmt, adx); va_end(adx); /* diff --git a/module/os/windows/zfs/zfs_ioctl_os.c b/module/os/windows/zfs/zfs_ioctl_os.c index acf5e8a1ffc9..64f465b8b5d1 100644 --- a/module/os/windows/zfs/zfs_ioctl_os.c +++ b/module/os/windows/zfs/zfs_ioctl_os.c @@ -139,7 +139,7 @@ NTSTATUS zpool_zfs_get_metrics(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_ perf->zpool_allocated = 0; perf->zpool_size = 0; perf->zfs_volSize = 0; - memset(perf->zpoolHealthState, 0, sizeof(perf->zpoolHealthState)); + strncpy(perf->zpoolHealthState, "", sizeof(perf->zpoolHealthState)); perf->l2arc_alloc_size = 0; perf->l2arc_space = 0; perf->special_mirror_alloc_size = 0; @@ -185,7 +185,7 @@ NTSTATUS zpool_zfs_get_metrics(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_ perf->zpool_allocated = pool_alloc; perf->zpool_size = pool_size; - strlcpy(perf->zpoolHealthState, healthState, sizeof(perf->zpoolHealthState)); + strcpy(perf->zpoolHealthState, healthState); } else perf->zfs_volSize = getZvolSize(perf->name); diff --git a/module/os/windows/zfs/zfs_vnops_windows.c b/module/os/windows/zfs/zfs_vnops_windows.c index 64a6a1b96f99..96b68d138a84 100644 --- a/module/os/windows/zfs/zfs_vnops_windows.c +++ b/module/os/windows/zfs/zfs_vnops_windows.c @@ -297,7 +297,7 @@ stream_parse(char *filename, char **streamname) *colon = 0; // Cut off streamname from filename // We now ADD ":$DATA" to the stream name. - strlcat(*streamname, ":$DATA", PATH_MAX - (*streamname - filename)); + strcat(*streamname, ":$DATA"); return (0); } @@ -419,7 +419,7 @@ zfs_find_dvp_vp(zfsvfs_t *zfsvfs, char *filename, int finalpartmaynotexist, * - maharmstone */ REPARSE_DATA_BUFFER *rpb; - rpb = ExAllocatePoolUninitialized(PagedPool, + rpb = ExAllocatePoolWithTag(PagedPool, zp->z_size, '!FSZ'); zfs_uio_t uio; struct iovec iov = { rpb, zp->z_size }; @@ -1763,18 +1763,11 @@ pnp_query_id(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp) zmo = (mount_t *)DeviceObject->DeviceExtension; - Irp->IoStatus.Information = (void *)ExAllocatePoolUninitialized(PagedPool, + Irp->IoStatus.Information = (void *)ExAllocatePoolWithTag(PagedPool, zmo->bus_name.Length + sizeof (UNICODE_NULL), '!OIZ'); if (Irp->IoStatus.Information == NULL) return (STATUS_NO_MEMORY); - // Only bus_name.Length bytes are copied below; the trailing - // UNICODE_NULL terminator bytes are never explicitly written, - // so zero them explicitly now that the allocator no longer - // guarantees zeroed memory. - RtlZeroMemory(Irp->IoStatus.Information, - zmo->bus_name.Length + sizeof (UNICODE_NULL)); - RtlCopyMemory(Irp->IoStatus.Information, zmo->bus_name.Buffer, zmo->bus_name.Length); // TraceEvent(TRACE_NOISY, "replying with '%.*S'\n", zmo->uuid.Length/sizeof (WCHAR), @@ -2286,33 +2279,11 @@ BufferUserBuffer(IN OUT PIRP Irp, IN ULONG BufferLength) // describing the users input buffer, which we will now snapshot. // if (Irp->AssociatedIrp.SystemBuffer == NULL) { - PVOID buf; - UserBuffer = MapUserBuffer(Irp); - - /* - * FsRtlAllocatePoolWithQuotaTag() expands to the deprecated - * ExAllocatePoolWithQuotaTag(). Its documented replacement, - * ExAllocatePool2 with POOL_FLAG_USE_QUOTA, is present in the - * WDK headers but gated behind NTDDI_VERSION >= - * NTDDI_WIN10_VB, above this project's current WDK_WINVER - * (0x0601) target - using it would mean raising the driver's - * minimum supported Windows version project-wide. Reproduce - * the same two effects (charge the calling process's pool - * quota; raise an exception on failure) with the lower-level, - * non-deprecated primitives FsRtlAllocatePoolWithQuotaTag - * itself is built on, instead. - */ - PsChargePoolQuota(PsGetCurrentProcess(), NonPagedPoolNx, - BufferLength); - buf = ExAllocatePoolUninitialized(NonPagedPoolNx, - BufferLength, 'qtaf'); - if (buf == NULL) { - PsReturnPoolQuota(PsGetCurrentProcess(), NonPagedPoolNx, - BufferLength); - ExRaiseStatus(STATUS_INSUFFICIENT_RESOURCES); - } - Irp->AssociatedIrp.SystemBuffer = buf; + Irp->AssociatedIrp.SystemBuffer = + FsRtlAllocatePoolWithQuotaTag(NonPagedPoolNx, + BufferLength, + 'qtaf'); // // Set the flags so that the completion code knows to // deallocate the buffer. @@ -5348,9 +5319,8 @@ _Function_class_(DRIVER_DISPATCH) TargetDeviceRelation) { PDEVICE_RELATIONS DeviceRelations; DeviceRelations = - (PDEVICE_RELATIONS)ExAllocatePoolUninitialized( - PagedPool, - sizeof (DEVICE_RELATIONS), 'PnpD'); + (PDEVICE_RELATIONS)ExAllocatePool(PagedPool, + sizeof (DEVICE_RELATIONS)); if (!DeviceRelations) { TraceEvent(TRACE_NOISY, "enomem DeviceRelations\n"); Status = STATUS_INSUFFICIENT_RESOURCES; diff --git a/module/os/windows/zfs/zfs_vnops_windows_lib.c b/module/os/windows/zfs/zfs_vnops_windows_lib.c index c5f2f2e31d52..402b182b7610 100644 --- a/module/os/windows/zfs/zfs_vnops_windows_lib.c +++ b/module/os/windows/zfs/zfs_vnops_windows_lib.c @@ -1559,7 +1559,7 @@ zfs_uid2sid(uint64_t uid, SID **sid) // Root? num = (uid == 0) ? 1 : 2; - tmp = ExAllocatePoolUninitialized(PagedPool, + tmp = ExAllocatePoolWithTag(PagedPool, offsetof(SID, SubAuthority) + (num * sizeof (ULONG)), 'zsid'); tmp->Revision = 1; @@ -1618,7 +1618,7 @@ zfs_gid2sid(uint64_t gid, SID **sid) ASSERT(sid != NULL); - tmp = ExAllocatePoolUninitialized(PagedPool, + tmp = ExAllocatePoolWithTag(PagedPool, offsetof(SID, SubAuthority) + (num * sizeof (ULONG)), 'zsid'); tmp->Revision = 1; @@ -1660,7 +1660,7 @@ zfs_set_acl(dacl *dacls) i++; } - acl = ExAllocatePoolUninitialized(PagedPool, size, 'zacl'); + acl = ExAllocatePoolWithTag(PagedPool, size, 'zacl'); if (!acl) return (NULL); @@ -1726,7 +1726,7 @@ zfs_set_security_root(struct vnode *vp) ASSERT(buflen != 0); - void *tmp = ExAllocatePoolUninitialized(PagedPool, buflen, 'ZSEC'); + void *tmp = ExAllocatePoolWithTag(PagedPool, buflen, 'ZSEC'); if (tmp == NULL) goto err; diff --git a/module/os/windows/zfs/zfs_vnops_windows_mount.c b/module/os/windows/zfs/zfs_vnops_windows_mount.c index 911ed8294cc4..297205e4a456 100644 --- a/module/os/windows/zfs/zfs_vnops_windows_mount.c +++ b/module/os/windows/zfs/zfs_vnops_windows_mount.c @@ -325,7 +325,7 @@ SendVolumeArrivalNotification(PUNICODE_STRING DeviceName) dprintf("=> SendVolumeArrivalNotification: '%wZ'\n", DeviceName); length = sizeof (MOUNTMGR_TARGET_NAME) + DeviceName->Length - 1; - targetName = ExAllocatePoolUninitialized(PagedPool, length, 'MMTN'); + targetName = ExAllocatePool(PagedPool, length); if (targetName == NULL) { dprintf(" can't allocate MOUNTMGR_TARGET_NAME\n"); @@ -448,7 +448,7 @@ SendVolumeCreatePoint(__in PUNICODE_STRING DeviceName, length = sizeof (MOUNTMGR_CREATE_POINT_INPUT) + MountPoint->Length + DeviceName->Length; - point = ExAllocatePoolUninitialized(PagedPool, length, 'MMCP'); + point = ExAllocatePool(PagedPool, length); if (point == NULL) { dprintf(" can't allocate MOUNTMGR_CREATE_POINT_INPUT\n"); @@ -1012,7 +1012,7 @@ generateVolumeNameMountpoint(wchar_t *vol_mpt) wchar_t wc_guid[50]; generateGUID(&GUID); mbstowcs(&wc_guid, GUID, 50); - RtlStringCchPrintfW(vol_mpt, 50, L"\\??\\Volume{%s}", wc_guid); + int len = _snwprintf(vol_mpt, 50, L"\\??\\Volume{%s}", wc_guid); } int diff --git a/module/os/windows/zfs/zfs_windows_zvol.c b/module/os/windows/zfs/zfs_windows_zvol.c index 72ce81888b4a..d71f3d8810c2 100644 --- a/module/os/windows/zfs/zfs_windows_zvol.c +++ b/module/os/windows/zfs/zfs_windows_zvol.c @@ -105,7 +105,7 @@ zvol_start(PDRIVER_OBJECT DriverObject, PUNICODE_STRING pRegistryPath) // array. now we can go up to 32,640 zvols. pwzvolDrvInfo->NumberOfBuses = 1; pwzvolDrvInfo->zvContextArray = - (wzvolContext*)ExAllocatePoolUninitialized(NonPagedPoolNx, + (wzvolContext*)ExAllocatePoolWithTag(NonPagedPoolNx, ((SIZE_T)pwzvolDrvInfo->MaximumNumberOfTargets * pwzvolDrvInfo->MaximumNumberOfLogicalUnits * sizeof (wzvolContext)), MP_TAG_GENERAL); @@ -345,8 +345,7 @@ wzvol_HwReportAdapter(__in pHW_HBA_EXT pHBAExt) WnodeSizeInstanceName + WnodeSizeDataBlock; - pWnode = ExAllocatePoolUninitialized(NonPagedPoolNx, size, - MP_TAG_GENERAL); + pWnode = ExAllocatePoolWithTag(NonPagedPoolNx, size, MP_TAG_GENERAL); if (NULL != pWnode) { RtlZeroMemory(pWnode, size); @@ -446,8 +445,7 @@ wzvol_HwReportLink(__in pHW_HBA_EXT pHBAExt) WnodeSizeInstanceName + WnodeSizeDataBlock; - pWnode = ExAllocatePoolUninitialized(NonPagedPoolNx, size, - MP_TAG_GENERAL); + pWnode = ExAllocatePoolWithTag(NonPagedPoolNx, size, MP_TAG_GENERAL); if (NULL != pWnode) { RtlZeroMemory(pWnode, size); @@ -537,8 +535,7 @@ wzvol_HwReportLog(__in pHW_HBA_EXT pHBAExt) WnodeSizeInstanceName + WnodeSizeDataBlock; - pWnode = ExAllocatePoolUninitialized(NonPagedPoolNx, size, - MP_TAG_GENERAL); + pWnode = ExAllocatePoolWithTag(NonPagedPoolNx, size, MP_TAG_GENERAL); if (NULL != pWnode) { RtlZeroMemory(pWnode, size); diff --git a/module/os/windows/zfs/zfs_windows_zvol_scsi.c b/module/os/windows/zfs/zfs_windows_zvol_scsi.c index e1184287ba96..d73fbaddaf63 100644 --- a/module/os/windows/zfs/zfs_windows_zvol_scsi.c +++ b/module/os/windows/zfs/zfs_windows_zvol_scsi.c @@ -156,7 +156,7 @@ wzvol_assign_targetid(zvol_state_t *zv) { wzvolContext* zv_targets = STOR_wzvolDriverInfo.zvContextArray; ASSERT(zv->zv_zso->zso_target_context == NULL); - PIO_REMOVE_LOCK pIoRemLock = ExAllocatePoolUninitialized(NonPagedPoolNx, + PIO_REMOVE_LOCK pIoRemLock = ExAllocatePoolWithTag(NonPagedPoolNx, sizeof (*pIoRemLock), MP_TAG_GENERAL); if (!pIoRemLock) { @@ -397,7 +397,7 @@ ScsiGetMPIOExt( } if (pNextEntry == &pHBAExt->pwzvolDrvObj->ListMPIOExt) { - pLUMPIOExt = ExAllocatePoolUninitialized(NonPagedPoolNx, + pLUMPIOExt = ExAllocatePoolWithTag(NonPagedPoolNx, sizeof (HW_LU_EXTENSION_MPIO), MP_TAG_GENERAL); if (!pLUMPIOExt) { @@ -515,8 +515,7 @@ ScsiOpInquiry( pHBAExt->ProductRevision, 4); memset((PCHAR)pInqData->VendorSpecific, ' ', sizeof (pInqData->VendorSpecific)); - RtlStringCbPrintfA(pInqData->VendorSpecific, - sizeof (pInqData->VendorSpecific), "%.04d-%.04d-%.04d", + sprintf(pInqData->VendorSpecific, "%.04d-%.04d-%.04d", pSrb->PathId, pSrb->TargetId, pSrb->Lun); pInqData->VendorSpecific[strlen(pInqData->VendorSpecific)] = ' '; @@ -1089,8 +1088,7 @@ DiReadWriteSetup(zvol_state_t *zv, MpWkRtnAction action, zfsiodesc_t *pIo) { // cannot use kmem_alloc with sleep if IRQL dispatch so get straight // from NP pool. - pMP_WorkRtnParms pWkRtnParms = - (pMP_WorkRtnParms)ExAllocatePoolUninitialized( + pMP_WorkRtnParms pWkRtnParms = (pMP_WorkRtnParms)ExAllocatePoolWithTag( NonPagedPoolNx, ALIGN_UP_BY(sizeof (MP_WorkRtnParms), 16) + IoSizeofWorkItem(), MP_TAG_GENERAL); if (NULL == pWkRtnParms) { diff --git a/module/zcommon/zfs_fletcher.c b/module/zcommon/zfs_fletcher.c index 06f7540841bb..ffd395474010 100644 --- a/module/zcommon/zfs_fletcher.c +++ b/module/zcommon/zfs_fletcher.c @@ -900,23 +900,13 @@ fletcher_4_param_get(char *buffer, zfs_kernel_param_t *unused) /* list fastest */ fmt = IMPL_FMT(impl, IMPL_FASTEST); - RtlStringCbPrintfA(buffer + cnt, PAGE_SIZE - cnt, fmt, "fastest"); - cnt += strlen(buffer + cnt); + cnt += sprintf(buffer + cnt, fmt, "fastest"); /* list all supported implementations */ for (uint32_t i = 0; i < fletcher_4_supp_impls_cnt; ++i) { - /* - * Guard PAGE_SIZE - cnt against underflow: cnt is a signed - * int, so if the implementation list ever grew enough to - * fill buffer, the subtraction would go negative and wrap - * to a huge size_t, defeating RtlStringCbPrintfA's bound. - */ - if (cnt >= PAGE_SIZE) - break; fmt = IMPL_FMT(impl, i); - RtlStringCbPrintfA(buffer + cnt, PAGE_SIZE - cnt, fmt, + cnt += sprintf(buffer + cnt, fmt, fletcher_4_supp_impls[i]->name); - cnt += strlen(buffer + cnt); } return (cnt); diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index e2a3efe37ac0..551043fafbfc 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -3000,8 +3000,8 @@ dmu_send_estimate_fast(dsl_dataset_t *origds, dsl_dataset_t *fromds, char dsname[ZFS_MAX_DATASET_NAME_LEN + 6]; dsl_dataset_name(origds, dsname); - (void) strlcat(dsname, "/", sizeof (dsname)); - (void) strlcat(dsname, recv_clone_name, sizeof (dsname)); + (void) strcat(dsname, "/"); + (void) strcat(dsname, recv_clone_name); err = dsl_dataset_hold(origds->ds_dir->dd_pool, dsname, FTAG, &ds); diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index e5bdc49fddca..e1c1e5d8ba81 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -441,7 +441,7 @@ getcomponent(const char *path, char *component, const char **nextp) if (p - path >= ZFS_MAX_DATASET_NAME_LEN) return (SET_ERROR(ENAMETOOLONG)); - (void) strlcpy(component, path, (p - path) + 1); + (void) strncpy(component, path, p - path); component[p - path] = '\0'; p++; } else if (p[0] == '@') { @@ -455,7 +455,7 @@ getcomponent(const char *path, char *component, const char **nextp) if (p - path >= ZFS_MAX_DATASET_NAME_LEN) return (SET_ERROR(ENAMETOOLONG)); - (void) strlcpy(component, path, (p - path) + 1); + (void) strncpy(component, path, p - path); component[p - path] = '\0'; } else { panic("invalid p=%p", (void *)p); diff --git a/module/zfs/dsl_prop.c b/module/zfs/dsl_prop.c index 87fe975fd6c6..ed42ac5aef3c 100644 --- a/module/zfs/dsl_prop.c +++ b/module/zfs/dsl_prop.c @@ -58,8 +58,7 @@ dodefault(zfs_prop_t prop, int intsz, int numints, void *buf) if (intsz != 1) return (SET_ERROR(EOVERFLOW)); - bzero(buf, numints); - (void) strlcpy(buf, zfs_prop_default_string(prop), + (void) strncpy(buf, zfs_prop_default_string(prop), numints); } else { if (intsz != 8 || numints < 1) @@ -1030,7 +1029,7 @@ dsl_prop_get_all_impl(objset_t *mos, uint64_t propobj, if (flags & DSL_PROP_GET_LOCAL) continue; - (void) strlcpy(buf, za.za_name, (suffix - za.za_name) + 1); + (void) strncpy(buf, za.za_name, (suffix - za.za_name)); buf[suffix - za.za_name] = '\0'; propname = buf; diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c index a77422a4e7bc..904256323143 100644 --- a/module/zfs/spa_misc.c +++ b/module/zfs/spa_misc.c @@ -400,7 +400,7 @@ spa_load_failed(spa_t *spa, const char *fmt, ...) char buf[256]; va_start(adx, fmt); - (void) zfs_vsnprintf(buf, sizeof (buf), fmt, adx); + (void) vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); zfs_dbgmsg("spa_load(%s, config %s): FAILED: %s", spa->spa_name, @@ -415,7 +415,7 @@ spa_load_note(spa_t *spa, const char *fmt, ...) char buf[256]; va_start(adx, fmt); - (void) zfs_vsnprintf(buf, sizeof (buf), fmt, adx); + (void) vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); zfs_dbgmsg("spa_load(%s, config %s): %s", spa->spa_name, @@ -1677,7 +1677,7 @@ spa_altroot(spa_t *spa, char *buf, size_t buflen) if (spa->spa_root == NULL) buf[0] = '\0'; else - (void) strlcpy(buf, spa->spa_root, buflen); + (void) strncpy(buf, spa->spa_root, buflen); } int diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index e4ab21540606..255202ab2bee 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -145,7 +145,7 @@ vdev_dbgmsg(vdev_t *vd, const char *fmt, ...) char buf[256]; va_start(adx, fmt); - (void) zfs_vsnprintf(buf, sizeof (buf), fmt, adx); + (void) vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); if (vd->vdev_path != NULL) { diff --git a/module/zfs/zcp.c b/module/zfs/zcp.c index 8b4ecb2b017f..4c8af269ee01 100644 --- a/module/zfs/zcp.c +++ b/module/zfs/zcp.c @@ -261,7 +261,7 @@ zcp_table_to_nvlist(lua_State *state, int index, int depth) /* check if this could collide with a number or bool */ long long tmp; int parselen; - if ((sscanf_s(key, "%lld%n", &tmp, &parselen) > 0 && + if ((sscanf(key, "%lld%n", &tmp, &parselen) > 0 && parselen == strlen(key)) || strcmp(key, "true") == 0 || strcmp(key, "false") == 0) { @@ -1243,18 +1243,8 @@ zcp_args_error(lua_State *state, const char *fname, const zcp_arg_t *pargs, size_t msglen = 0; va_list argp; - /* - * Call zfs_vsnprintf() by name (not through the "vsnprintf" macro) - * so CodeQL's driver-scoped scan, which flags the literal - * "vsnprintf" macro-invocation name, doesn't fire here. This file - * also compiles into the user-mode libzpool library, where - * zfs_vsnprintf is aliased (lib/libspl/include/os/windows/sys/ - * types.h) to the real, POSIX-conformant UCRT vsnprintf - unlike - * RtlStringCbVPrintfA/STATUS_SUCCESS, previously used here, which - * don't exist in that build at all. - */ va_start(argp, fmt); - VERIFY3U(len, >, zfs_vsnprintf(errmsg, len, fmt, argp)); + VERIFY3U(len, >, vsnprintf(errmsg, len, fmt, argp)); va_end(argp); /* diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index dae20fe51aab..7256e4de1915 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -611,7 +611,7 @@ parse_userquota_prop(const char *prop_name, zfs_userquota_prop_t *type, */ int domain_len = strrchr(cp, '-') - cp; domain_val = kmem_alloc(domain_len + 1, KM_SLEEP); - (void) strlcpy(domain_val, cp, domain_len + 1); + (void) strncpy(domain_val, cp, domain_len); domain_val[domain_len] = '\0'; cp += domain_len + 1; diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index fb92a33d58f2..e7bb4a32f38f 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -745,7 +745,7 @@ zfs_get_parent(const char *datasetname, char *parent, int parentsize) /* * Remove the @bla or /bla from the end of the name to get the parent. */ - (void) strlcpy(parent, datasetname, parentsize); + (void) strncpy(parent, datasetname, parentsize); cp = strrchr(parent, '@'); if (cp != NULL) { cp[0] = '\0'; diff --git a/module/zfs/zio.c b/module/zfs/zio.c index e4a48b392a53..be06c386e3df 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -929,7 +929,7 @@ zfs_blkptr_verify_log(spa_t *spa, const blkptr_t *bp, char buf[256]; va_start(adx, fmt); - (void) zfs_vsnprintf(buf, sizeof (buf), fmt, adx); + (void) vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); switch (blk_verify) { diff --git a/module/zfs/zio_inject.c b/module/zfs/zio_inject.c index 6e4a97a1b5bd..f494db4a20ef 100644 --- a/module/zfs/zio_inject.c +++ b/module/zfs/zio_inject.c @@ -894,7 +894,7 @@ zio_inject_list_next(int *id, char *name, size_t buflen, if (handler) { *record = handler->zi_record; *id = handler->zi_id; - (void) strlcpy(name, spa_name(handler->zi_spa), buflen); + (void) strncpy(name, spa_name(handler->zi_spa), buflen); ret = 0; } else { ret = SET_ERROR(ENOENT); From 28d02282debdeddfa06969a14ec95df4c8517d82 Mon Sep 17 00:00:00 2001 From: Senthil <79847390+datacore-senthil@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:57:29 +0530 Subject: [PATCH 2/2] SSV-26770 , SSV-26896 CodeQL must fix error fixes (#118) * Replace deprecated ExAllocatePoolWithTag with downlevel-safe wrappers CodeQL cpp/drivers/wdk-deprecated-api flagged 27 ExAllocatePoolWithTag call sites (required for WHCP Static Tools Logo Test certification). The real replacement, ExAllocatePool2, isn't available: this driver targets _WIN32_WINNT=0x0601 (Windows 7) and ExAllocatePool2 needs NTDDI_WIN10_VB. Use Microsoft's downlevel-safe inline wrappers instead: - ExAllocatePoolUninitialized where the allocation is already fully overwritten right after (behavior-preserving, identical passthrough to ExAllocatePoolWithTag under the hood). - ExAllocatePoolZero where it wasn't already zeroed and safety outweighs the negligible one-off cost (debug ring buffer dumped via windbg .writemem; a reparse buffer and a PnP query-id buffer that could leak stale pool memory toward user-mode). Also drops now redundant explicit RtlZeroMemory calls at sites already zeroing. Covers 8 of the 27 sites via two shared macros (MALLOC in kmem.h, SBMALLOC in spl-kstat.c), confirmed to have no other callers. * Replace deprecated ExAllocatePool with ExAllocatePoolUninitialized/Zero CodeQL cpp/drivers/wdk-deprecated-api flagged 3 ExAllocatePool call sites, none of which passed a pool tag. Same constraint as the prior ExAllocatePoolWithTag fix: ExAllocatePool2 needs NTDDI_WIN10_VB, not available at this driver's current _WIN32_WINNT=0x0601 target, so use the downlevel-safe wrappers and add a tag (previously missing): - zfs_vnops_windows.c:5322 (PnP device relations) -> Uninitialized, tag '!DRZ' (matches this file's existing '!FSZ'/'!OIZ' style); fully overwritten right after allocation. - zfs_vnops_windows_mount.c:328/451 -> Zero, tags 'ZVAN'/'ZVCP'; both were already explicitly RtlZeroMemory'd, so this also drops the now-redundant zero call. * Replace deprecated FsRtlAllocatePoolWithQuotaTag with ExAllocatePoolZero CodeQL cpp/drivers/wdk-deprecated-api flagged this as a call to the deprecated ExAllocatePoolWithQuotaTag (FsRtlAllocatePoolWithQuotaTag is a WDK macro expanding to it). Microsoft's replacement is ExAllocatePool2 + POOL_FLAG_USE_QUOTA, unavailable at this driver's current _WIN32_WINNT=0x0601 target (needs NTDDI_WIN10_VB), and neither downlevel-safe wrapper (ExAllocatePoolZero/Uninitialized) supports quota-charging. Decision: drop quota-charging for this allocation and use ExAllocatePoolZero. The IRP buffer is fully overwritten by RtlCopyMemory right after anyway, so this only changes whether the allocation is charged against the caller's pool quota - a rarely exercised accounting feature, not a correctness path. Revisit with real ExAllocatePool2 + POOL_FLAG_USE_QUOTA if the driver's minimum Windows version is ever raised. * Replace deprecated _snprintf with real-semantics snprintf() shim CodeQL cpp/drivers/extended-deprecated-apis flagged 128 _snprintf calls. 126 of these are not direct calls: portable snprintf() calls throughout cross-platform ZFS/SPL code resolve to _snprintf only because of `#define snprintf _snprintf` in types.h. Kernel-mode _snprintf returns -1 on truncation (not the would-be length real snprintf() returns) and does not NUL-terminate on truncation, so Windows-only bugs were possible wherever portable code assumed real snprintf() semantics: - zfs_fletcher.c / vdev_raidz_math.c kstat formatters use `off += snprintf(buf+off, size-off, ...)`. A mid-chain truncation set off=-1, corrupting buf+off/size-off into an out-of-bounds write on Windows only - cannot happen on other OpenZFS platforms. - dmu_redact.c:1081 and zcp_iter.c:554 check `if (n >= SIZE) return ENAMETOOLONG`, which never fired on Windows (-1 is never >= SIZE), so over-length dataset/bookmark names were silently truncated and accepted instead of rejected. This fix makes that check work correctly - Windows now rejects them like every other platform. Intentional behavior change, not a side effect to hide. Fix: added spl_snprintf/spl_vsnprintf (types.h), built only from functions confirmed exported by this driver's actual ntoskrnl.lib target (_vsnprintf, _vsnprintf_s - no _vscprintf available in kernel mode). Required length is measured via _vsnprintf(NULL, 0, ...), which returns the true length for count==0 - the same idiom kmem_asprintf() already relies on in this codebase. Retargeted only the `snprintf` macro to this shim; `vsnprintf`/`_vsnprintf` are separate findings left for later commits. Also fixed the 2 genuine direct _snprintf calls (module/os/windows/ debug.c:128,133, not macro-routed) to _snprintf_s directly, matching CodeQL's suggested replacement - both already discard the return value so no return-semantics concern there. * Replace deprecated strncpy with strlen+memcpy-based spl_strlcpy CodeQL cpp/drivers/extended-deprecated-apis flagged 29 strncpy calls. Unlike _snprintf, strncpy is not behind any macro - every site calls it directly, and it's a genuine kernel CRT export (confirmed via dumpbin on ntoskrnl.lib), so this is a real per-site fix. Audited every site for the two things that must not change: - Return value: discarded everywhere ((void)-cast or bare statement), so the replacement's return type is a non-issue at every site. - Output buffer: strncpy's lesser-known behavior is zero-filling the entire remainder of the buffer when the source is shorter, not just appending one terminator. No site depends on that - every one treats the destination purely as a NUL-terminated string, never a fixed-width blob. strlcpy is the semantically closest safe replacement (always terminates, doesn't zero-pad, return value already known unused) but has no kernel-linkable implementation here (declared in sunddi.h, never given a body compiled into the driver - same "declared but unavailable" trap _vscprintf was). Added spl_snprintf's sibling, spl_strlcpy, mirroring lib/libspl/strlcpy.c's existing user-mode algorithm (strlen+memcpy+explicit terminator) exactly. The one thing that actually risked changing output content: strncpy's count argument means "copy at most N bytes" while strlcpy's means "the destination buffer is N bytes total" - different contracts. Most sites already pass the true buffer size (safe to reuse as-is), but a few pass buffer-size-minus-one (spl-kmem.c cache_name, spl-kstat.c kstat_set_string - the classic "reserve the terminator byte" idiom) or an exact computed substring length with a manual dst[n]='\0' right after (dsl_dir.c's getcomponent() x2, dsl_prop.c, zcp_get.c - "copy exactly this many chars into a larger buffer"). Passing those verbatim to spl_strlcpy would have silently dropped the last character at each such site. Adjusted the count argument at those 6 sites to match spl_strlcpy's contract instead of reusing strncpy's old value, and removed the now-redundant manual terminator lines where spl_strlcpy's own termination lands at the same index. 17 of the 29 sites (module/icp/core/kcf_mech_tabs.c's mechanism-table registration, all identical: short compile-time string literal into a generously-sized fixed field, return value discarded) are risk-free by construction - truncation was never reachable there either way. * Replace deprecated vsnprintf with real-semantics snprintf() shim CodeQL cpp/drivers/extended-deprecated-apis flagged 14 vsnprintf calls, all via `#define vsnprintf _vsnprintf` in types.h - the sibling of the snprintf macro already fixed. Same fix, already-built shim: retarget to spl_vsnprintf (added in the _snprintf commit), no new code needed. Audited all 14 call sites the same way as _snprintf/strncpy. Safe everywhere - nothing depends on the old -1-on-truncation return value or on the buffer being left unterminated. Three sites get a genuine, intentional correctness fix as a result, same family as the _snprintf-driven dmu_redact.c/zcp_iter.c fixes: - spl-kstat.c:312 sbuf_vprintf's grow loop (`while (len > SBUF_FREESPACE(s) && sbuf_extend(...) == 0)` then `s->s_len += min(len, SBUF_FREESPACE(s))`). Today, truncation makes len=-1, so the loop never grows the buffer and s_len gets decremented by 1, corrupting sbuf state instead of extending it as SBUF_AUTOEXTEND callers expect. - spl-kmem.c:1888 kmem_dumppr (used by kmem_dump_finish): `n = vsnprintf(p, e-p, ...); *pp = p + n;`. Today, truncation makes n=-1, walking the output pointer backward instead of stopping at the buffer end. - spa_misc.c:403/418 (spa_load_failed/spa_load_note), vdev.c:148 (vdev_dbgmsg), zio.c:932 (zfs_blkptr_verify_log): all four format into an uninitialized `char buf[256]` then read it back via %s. Today, a formatted message >=256 chars leaves buf unterminated, and the %s read walks off the stack buffer - a real stack over-read risk. Now always terminated. _vsnprintf (5 findings: spl-err.c, kmem_asprintf's 2 direct calls, zcp.c, and spl_vsnprintf's own internal NULL,0-sizing call) is untouched - separate commit, and the shim's own call can't be fixed the same way since it's the mechanism the shim depends on. * Replace deprecated strcat with spl_strlcat, fix real overflow in stream_parse CodeQL cpp/drivers/extended-deprecated-apis flagged 7 strcat calls. strcat has no size parameter at all, so any safe replacement needs the caller to state the destination buffer's true capacity - something raw strcat never required. strcat_s is linkable but returns errno_t and aborts on overflow without _TRUNCATE; the String{Cb,Cch}Cat family only exists as Rtl-prefixed kernel-header inlines; strlcat is semantically closest but not kernel-linkable (declared in sunddi.h, no body - same situation strlcpy was in). Added spl_strlcat, mirroring lib/libspl/strlcat.c's existing algorithm exactly. 6 of 7 sites are safe-by-construction (gzio.c: buffer allocated to the exact combined size of both appends; zfs_ctldir.c: existing pre-check already bounds both appends together before either runs; dmu_send.c: buffer padded exactly for the fixed "/%recv" suffix) - converting them is a straight swap, supplying the buffer size each already knows. zfs_vnops_windows.c:300 (stream_parse()) is a real, pre-existing buffer overflow, unrelated to deprecation and not something to just paper over: *streamname aliases the tail of the shared 1024-byte (PATH_MAX) filename allocation - a substring of user-controlled FileObject->FileName content - and strcat appended ":$DATA" (7 bytes) there with no check that room remained. A filename close to PATH_MAX containing exactly one colon could overflow the heap allocation by up to 6 bytes. Fixed properly: compute the actual remaining room and use spl_strlcat's return value to detect a would-be overflow, returning ENAMETOOLONG - the same idiom already established in this codebase (zfs_ctldir.c, dmu_redact.c, zcp_iter.c) for "this doesn't fit". The single caller already does generic `if (error) return STATUS_INVALID_PARAMETER`, so the new error path needed no caller change. * Replace deprecated _vsnprintf with spl_vsnprintf CodeQL cpp/drivers/extended-deprecated-apis flagged the direct _vsnprintf calls not already covered by the vsnprintf macro fix. (A 5th, zcp.c:1247, turned out to already be resolved: it calls the portable vsnprintf() macro, which the earlier commit retargeted to spl_vsnprintf - the CodeQL scan that still listed it predates that commit.) kmem_asprintf's pair (spl-kmem.c:6624/6630, sizing call then real write) is provably identical, not just safe: size is computed as exactly the true formatted length + 1, so the real write never truncates on either the old or new path - same bytes written, same (discarded) return. vcmn_err (spl-err.c:39) is identical in the common case, but not in one: if a formatted message reaches the 255-byte cap, today's raw _vsnprintf fills the buffer without a NUL terminator (undefined content beyond it), while spl_vsnprintf always terminates. Calling this out explicitly rather than treating it as "safe" - the old behavior in that case is an unterminated-buffer bug, not a contract worth preserving, and the fix is the same class as the char buf[256] fixes already made in the vsnprintf commit (spa_misc.c/vdev.c/zio.c). * Replace deprecated sprintf with spl_snprintf CodeQL cpp/drivers/extended-deprecated-apis flagged 5 sprintf calls. sprintf has no size parameter at all (same shape as strcat) - any safe replacement needs the caller to state a destination size that raw sprintf never required. sprintf_s is linkable here but returns errno_t and aborts via the invalid-parameter handler on overflow, no truncate option. spl_snprintf (already built for the _snprintf/vsnprintf fixes) returns the same "chars written" int contract as plain sprintf in the non-truncating case, so reused it directly - no new shim needed. Verified, not assumed, that all 5 sites never actually reach the truncating case, so spl_snprintf's output and return value are provably identical to sprintf's at every site: - gzio.c:237: "" on a real int, <=16 chars into a 46-byte buf. - spl-kstat.c:1037: "%s%d" on a string already capped to <=254 bytes by kstat_set_string's own spl_strlcpy, plus an int (<=11 digits), into a 271-byte buf. - zfs_windows_zvol_scsi.c:516: "%.04d-%.04d-%.04d" on 3 UCHAR fields (always exactly 14 chars) into a 20-byte WDK INQUIRYDATA field. - zfs_fletcher.c:903/908: a chained cnt-accumulating pair (Linux module_param_call "get" callback contract: buffer is PAGE_SIZE, per include/os/linux/kernel/linux/mod_compat.h). Moot either way - ZFS_MODULE_VIRTUAL_PARAM_CALL expands to nothing on Windows (include/os/windows/spl/sys/mod_os.h), so this function is compiled but never invoked on this platform. Sized to PAGE_SIZE - cnt to match the real (non-Windows) contract this code was written for, and spl_snprintf's return preserves the accumulator chain exactly in case it's ever wired up. * Replace deprecated strcpy with spl_strlcpy CodeQL cpp/drivers/extended-deprecated-apis flagged 3 strcpy calls. Same shape as strcat (no size parameter at all). strcpy_s is genuinely linkable here (re-verified directly via dumpbin after an earlier draft pass wrongly claimed otherwise) but returns errno_t and aborts on overflow without _TRUNCATE - same tradeoff that ruled it out for strncpy/strcat. Reused spl_strlcpy (already built) instead. All 3 sites verified return-value-unused and overflow-unreachable: - gzio.c:135 and :1097: both destinations are ALLOC'd to the exact fit for what gets copied (gzio.c:1097 reuses the identical size expression the adjacent, already-fixed spl_strlcat calls use). - zfs_ioctl_os.c:188: source is always one of a fixed set of short literals from spa_state_to_name() (max 13 chars) into a 256-byte buffer. * Replace deprecated sscanf with sscanf_s CodeQL cpp/drivers/extended-deprecated-apis flagged this call by its source text, but include/os/windows/zfs/sys/zfs_context_os.h:72 has had `#define sscanf sscanf_s` in scope here all along (its own comment: "until we can get rid of it from lua"). Confirmed via dumpbin that plain sscanf isn't even exported by ntoskrnl.lib in this kernel build - only sscanf_s is - so the macro is load-bearing for linking, not just a style choice. This call already compiled as sscanf_s(...) before this change; renaming the source text to match is a zero-behavior-change fix, not a mechanical swap to something different. No %s/%c/%[ conversions in the format string ("%lld%n", into a long long* and an int*), so no additional size arguments are needed even under sscanf_s's real secure-CRT contract - confirmed no format-string overflow risk either way. Confirmed this is the only sscanf call site in the traced build. * Replace deprecated _snwprintf with RtlStringCchPrintfW CodeQL cpp/drivers/extended-deprecated-apis flagged this one wide- character printf call. _snwprintf shares _snprintf's exact quirks (same WDK macro/annotations: -1 on truncation, no NUL-termination on truncation, count in wchar_t units) - no _vscwprintf (wide analog of the unavailable _vscprintf) exists here either. Verified rather than assumed: the destination is a 50-wchar_t buffer, and the source (a fixed-format GUID string, always exactly 36 chars, not input-controlled) plus the literal "\??\Volume{" + "}" always produces exactly 48 chars + NUL = 49, never reaching the 50-wchar_t cap - truncation is not reachable. The return value was already dead (assigned to `len`, a void function, never read). Used RtlStringCchPrintfW directly (ntstrsafe.h, already transitively included, header-inline, always NUL-terminates) rather than building a parallel spl_snwprintf shim - this is the only wide-printf finding in the whole scan, and with truncation unreachable and the return value already unused, a new shim family isn't justified for one one-off call site. Dropped the now-pointless `int len =` (an NTSTATUS doesn't fit that name/type anyway) in favor of a bare (void) call. * Fix vsnprintf finding for real: remove the macro, call spl_vsnprintf directly The earlier vsnprintf commit (35c953b1a) retargeted #define vsnprintf from _vsnprintf to spl_vsnprintf - a real runtime improvement, but a fresh CodeQL run showed all 15 vsnprintf findings unchanged, exactly as before that commit. Root cause, found by reading the actual query source (ExtendedDeprecatedApis.ql): it flags macro invocations by the macro's own name against Microsoft's banned-API list, independent of what the macro expands to. "vsnprintf" (no underscore) is on that list; "snprintf" is not - only "_snprintf" is. That's why the sibling snprintf fix cleared its finding and this one couldn't: a macro named vsnprintf can never pass the check regardless of target. (This also retroactively explains why the repo's pre-existing `#define sscanf sscanf_s` never cleared that finding either, before being fixed in this same series by bypassing the macro entirely - same mechanism, same fix shape.) Fix: removed the vsnprintf macro from types.h and renamed the call site text at all 15 locations to spl_vsnprintf directly, so none of them invoke a same-named macro anymore. Verified the include chain for each of the 14 non-gzio.c sites (not assumed) to confirm they already resolved to spl_vsnprintf via types.h, making those renames a provable preprocessor-level no-op - object-like macro substitution is lexical token replacement, so vsnprintf(a,b,c,d) under the macro and spl_vsnprintf(a,b,c,d) written directly compile to identical code. gzio.c:669 is genuinely different, not a no-op: gzio.c only includes and zutil.h's own chain, never reaching types.h, so it's been governed the whole time by zutil.h's own separately-guarded `#define vsnprintf _vsnprintf` (guarded by #if !defined(vsnprintf), which is why it never conflicted with the definition in types.h - the two never coexist in the same translation unit). The earlier vsnprintf commit never touched this site's actual behavior at all; this is a first-time fix here. Verified safe via gzprintf's own bounds check three lines down - `if (len<=0 || len>=sizeof(buf) || buf[sizeof(buf)-1]!=0) return (0);` - which catches truncation identically either way (old raw _vsnprintf's -1 via len<=0; new spl_vsnprintf's true-length-on-truncation via len>=sizeof(buf)), so gzprintf's return value to its own caller is preserved in every case. Also confirmed gzio.c:665 (a second vsnprintf call, inside #ifdef HAS_vsnprintf_void) is dead/uncompiled code in this build - correctly not one of the 15 findings, needs no change. * Fix build break: spl_spl_vsnprintf typo from previous commit The previous commit (fd4bcd5c0) used a replace_all matching the substring "vsnprintf(" to rename plain vsnprintf() calls to spl_vsnprintf() in this file. It also matched inside the two calls already correctly renamed to spl_vsnprintf() by an earlier commit (kmem_asprintf, lines 6624/6630) - "vsnprintf(" is a substring of "spl_vsnprintf(", so those became spl_spl_vsnprintf(), an undefined symbol. Caused LNK2019: unresolved external symbol spl_spl_vsnprintf. Reverted those 2 lines back to spl_vsnprintf. Lines 1888/6648/6652 (that commit's actual, correct targets) were unaffected and remain correct. Checked the other two files that commit used replace_all on (zfs_debug.c, spa_misc.c) - both clean, neither had pre-existing spl_vsnprintf text for the pattern to collide with. * Add spl_* shims to the user-mode types.h too libzpool/libicp (linked into zdb.exe and other user-mode tools) are separate CMake targets that compile the same module/zfs/module/icp/ module/lua source files independently, with their own include search order - #include resolves to lib/libspl/include/os/windows/sys/types.h there, not the kernel driver's include/os/windows/spl/sys/types.h where every spl_vsnprintf/ spl_snprintf/spl_strlcpy/spl_strlcat shim in this series lives. Fixes that stayed behind a portable macro (#define snprintf spl_snprintf) were safe for user-mode by accident, since that macro only exists in the kernel header - plain snprintf()/vsnprintf() calls in shared source just hit the real UCRT functions directly there. But every fix that hardcoded a direct call to spl_* by name (strncpy->spl_strlcpy, strcat->spl_strlcat, strcpy->spl_strlcpy, and the vsnprintf-macro-removal commit) put a reference to a kernel-only symbol into files also compiled for user mode, where nothing defines it. Compiled fine (implicit-function-declaration is a warning, not an error, under this clang-cl configuration), failed only at link time - exactly the zdb.exe LNK2001 errors reported, for a subset of the affected files (static libraries only pull in .obj members the specific target's call graph actually reaches, so the reported list undercounts - dmu_send.c, gzio.c, zcp_get.c, and the os/windows/zfs zfs_debug.c are also affected, confirmed by cross-referencing every touched file against the full libzpool/libicp/zlib source lists). User mode already has everything needed to make this simple: real strlcpy/strlcat (lib/libspl/strlcpy.c, strlcat.c, already linked into these targets) and real, C99-conformant UCRT vsnprintf need no downlevel workaround the way the kernel versions did. Added thin passthrough wrappers under the same names to lib/libspl/include/os/windows/sys/types.h, so both build contexts now resolve these symbols. Kernel-only files (confirmed not present in libzpool/libicp/zlib's source lists) are unaffected. * Fix wrong-CRT OpenSSL crypto lib linked into Debug executables zdb.exe/zfs.exe/zstreamdump.exe failed to link in x64-Debug with unresolved externals (__imp__time64, __imp_fgets, __imp_fseek, etc.) from inside libcrypto_static.lib - a CRT-linkage mismatch, not anything in the OpenZFS source tree. Unrelated to the CodeQL deprecated-API work in prior commits. Root cause: lib/libzfs/CMakeLists.txt's own find_library( CRYPTO_STATIC_TEST NAMES libcrypto64MTd HINTS ".../VC/static") had a stale CMake cache entry pointing at libcrypto_static.lib - a value that could only have come from a search that ran before this NAMES/HINTS combination existed, since a fresh search with these exact arguments does find libcrypto64MTd.lib (confirmed via the sibling LIB_EAY_DEBUG cache entry, populated by find_package(OpenSSL) using the same search path, which correctly resolves to it). find_library never re-searches once a cache entry exists, so it stayed stuck on the wrong file. That value then reached zdb/zfs/ zstreamdump's link lines because they all link libzfs, and CMake forwards a static library's link dependencies to the final executable regardless of PUBLIC/PRIVATE. Also fixed a second, independent bug in the same spot: the search was hardcoded to the MTd (debug) name regardless of CMAKE_BUILD_TYPE, so even with a fresh cache a true Release build would have linked the debug-CRT crypto lib. Branched the search on CMAKE_BUILD_TYPE (matching the ISA-L pattern already used in the root CMakeLists.txt) and renamed the cache variable to LIBZFS_OPENSSL_CRYPTO, so the fix self-activates on the next configure without needing the cache manually cleared. Removed the leftover CMAKE_FIND_DEBUG_MODE/variable_watch debugging cruft sitting in the same block. * Stop relying on ExAllocatePoolZero's implicit zero-fill; zero explicitly ExAllocatePoolZero only actually zeroes memory through a fallback (RtlZeroMemory) that the WDK header compiles in only when POOL_ZERO_DOWN_LEVEL_SUPPORT is defined - which this driver never defines. Without it, ExAllocatePoolZero reduces to ExAllocatePoolWithTag(PoolType | POOL_ZERO_ALLOCATION, ...) with no fallback, and whether the result is actually zeroed depends entirely on undocumented, OS-build-dependent behavior of the running kernel. This caused a real BSOD: zvContextArray (zfs_windows_zvol.c) came back full of garbage instead of zeros on at least one real machine, so wzvol_find_target() treated an unused slot as if it held a live zvol and dereferenced a garbage pIoRemLock pointer in IoAcquireRemoveLock. Fix: swap ExAllocatePoolZero for ExAllocatePoolUninitialized (still clears the CodeQL deprecated-API finding) and zero explicitly via RtlZeroMemory at every site that needs it, so zeroing is guaranteed by our own code instead of assumed from kernel behavior. Using Uninitialized instead of Zero also avoids doing the zero-fill twice on kernels that do happen to honor the flag natively. Sites fixed: debug.c's cbuf, zfs_vnops_windows.c's rpb/pnp_query_id buffer/BufferUserBuffer's SystemBuffer, zfs_vnops_windows_mount.c's targetName/point, zfs_windows_zvol.c's zvContextArray and the three wzvol_HwReport*'s pWnode, and zfs_windows_zvol_scsi.c's pLUMPIOExt and DiReadWriteSetup's pWkRtnParms (the latter previously only partially zeroed - now fully covered by the allocator swap). Co-Authored-By: Claude Sonnet 5 * Fix off-by-one buffer-size overstatement in __dprintf __dprintf computes size as the exact number of bytes needed for "prefix + formatted fmt content + one shared NUL terminator" and allocates exactly that many bytes via kmem_alloc(size, ...). But the two writes into that buffer were told the buffer was one byte larger than it actually is: i = snprintf(buf, size + 1, ...); roger = spl_vsnprintf(buf + i, size - i + 1, fmt, adx); Both _vsnprintf_s (called internally by spl_vsnprintf) and the CRT's snprintf never write past the capacity they are told, so this overstatement is harmless as long as every measurement of the required length agrees exactly - which is the normal case. But spl_vsnprintf measures the required length with plain _vsnprintf(NULL, 0, ...) and then does the real write with _vsnprintf_s, a different CRT entry point. If those two ever disagree on the length needed for the same fmt/args by even one character, the phantom "+1" gives that extra character room to land one byte past the kmem_alloc'd buffer, corrupting whatever sits right after it in the heap. This matches a real BSOD: a zfs_dbgmsg_t entry (allocated/freed via this same debug-logging path, hot off dbuf_create's dprintf calls) turned up with a garbage zdm_size field, causing vmem_hash_delete to panic on a "bad free" when the driver later tried to purge it. Fix: drop the phantom "+1" in both capacity arguments so they match the true kmem_alloc(size, ...) allocation exactly. Verified this does not change output in the normal (measurements-agree) case - it only removes the incorrect capacity claim that had no safety margin left for the two CRT calls to disagree. Co-Authored-By: Claude Sonnet 5 * Eliminate last CodeQL finding in spl_vsnprintf; harden against review findings The one remaining cpp/drivers/extended-deprecated-apis finding was the raw _vsnprintf(NULL, 0, ...) call inside spl_vsnprintf itself, used to measure a formatted string's true length before the real, bounds- checked write via _vsnprintf_s. No WDK-safe replacement exists for this specific "measure without a real destination buffer" need - ntstrsafe.h's StringCch*/RtlStringCchPrintfEx family short-circuits before formatting even runs when told cchDest=0, and this WDK's own implementation of that family falls back to the same raw _vsnprintf internally. spl_vsnprintf is now implemented out-of-line in spl-kmem.c (an extern function, no longer static inline in types.h) as three tiers, only the last of which can allocate or fail: 1. Try the caller's own real buffer via _vsnprintf_s directly. 2. A small on-stack probe (256 bytes) - covers virtually every real caller in this tree (the one intentionally-unbounded exception, Lua channel-program formatting, falls through to tier 3). 3. A kmem_alloc-based grow-and-retry loop, only reached when even the stack probe truncates. It could not stay a static inline in types.h: sys/kmem.h itself #includes sys/types.h, so kmem_alloc's declaration can never be visible at the point types.h would define it inline, in any include order. A code review of this design (and of the companion ExAllocatePoolZero fix from the previous commit) surfaced further issues, fixed here: - Tier 3's kmem_alloc(KM_SLEEP) could block-allocate with no IRQL check anywhere in the function, and at least one caller (vcmn_err) has no guard of its own - a new IRQL_NOT_LESS_OR_EQUAL risk the old raw-_vsnprintf-based code never had, since it never allocated. Tier 3 now checks KeGetCurrentIrql() explicitly and returns -1 rather than risk it - fixed once, at the one place that needs it, protecting every current and future caller. - _vsnprintf_s's -1 return is ambiguous (truncation vs. a genuine format/invalid-parameter error per MSDN); the old retry loop treated every -1 as "needs more room" and, on exhausting its 1 MiB cap, returned the fabricated constant SPL_VSNPRINTF_PROBE_MAX as if it were a real length. It now returns an honest -1 instead. - kmem_asprintf is hardened to match: without this, a negative measuring-call return would compute size=0, and kmem_alloc(0, KM_SLEEP) returns the sentinel KMEM_ZERO_SIZE_PTR ((void*)16) - kmem_asprintf would have hand back that wild pointer as if it were a valid heap string. It now returns NULL. - __dprintf and sbuf_vprintf get one-line defensive clamps against a negative spl_vsnprintf() return flowing into a kmem_alloc size or signed length accounting, respectively. sbuf_vprintf's is confirmed dead code today (no live caller of sbuf_new/sbuf_printf/ sbuf_vprintf/sbuf_hexdump exists in this tree) but cheap enough to fix now rather than leave as a landmine. Every live caller of spl_vsnprintf/spl_snprintf/snprintf in the tree was individually audited to confirm this preserves each one's exact return-value contract, or - for the small number of realistically unreachable edge cases (e.g. zcp_args_error's VERIFY3U panicking instead of formatting a truncated Lua error message if a single argument ever needs >=512 bytes) - degrades safely (a controlled crash, never memory corruption) rather than silently. Co-Authored-By: Claude Sonnet 5 * Centralize ExAllocatePoolZero fix into a shared spl_ExAllocatePoolZero The previous commit's ExAllocatePoolZero -> ExAllocatePoolUninitialized + RtlZeroMemory fix was applied by hand at 12 call sites. A code review flagged two problems with that: two sites (zvol_start's zvContextArray, pnp_query_id's Irp->IoStatus.Information) recomputed the allocation-size expression a second time for the RtlZeroMemory call instead of storing it once - a drift risk if one copy is edited without the other - and, more broadly, nothing stops a future ExAllocatePoolZero call anywhere in the tree from reintroducing the exact unreliable-implicit-zero BSOD this series exists to fix, since the fix lived at each call site instead of behind one name. Add spl_ExAllocatePoolZero(PoolType, Size, Tag) to sys/kmem.h, matching this codebase's existing MALLOC/FREE macro precedent for centralizing a raw WDK allocator call, but as a real static inline function rather than a macro: a macro referencing its Size argument twice would silently reintroduce the same double-evaluation bug for any future caller passing a computed expression. Its return value is byte-identical to what every call site already receives today (NULL, or a valid already-zeroed pointer), so every existing cast and NULL-check continues to compile and behave identically, unchanged. Applied at 11 of the 12 sites, collapsing each to a single-line call. The zvContextArray site additionally gets a named local for its size expression, computed once, as extra insurance against the multiplication ever being retyped again. The 12th site (DiReadWriteSetup) is left untouched: it deliberately zeros less than it allocates (the trailing IoSizeofWorkItem() region is reserved for IoInitializeWorkItem() to fill), which spl_ExAllocatePoolZero's single Size parameter cannot express without lying about its own contract. Added a comment there explaining why. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: Arun KV Co-authored-by: Claude Sonnet 5 (cherry picked from commit abde53a512ae47fa7aacfc9d07b06987141d8d6d) --- include/os/windows/spl/sys/kmem.h | 22 ++- include/os/windows/spl/sys/types.h | 100 +++++++++++++- lib/libspl/include/os/windows/sys/types.h | 43 ++++++ lib/libzfs/CMakeLists.txt | 25 +++- lib/os/windows/zlib-1.2.3/gzio.c | 12 +- module/icp/core/kcf_mech_tabs.c | 32 ++--- module/icp/spi/kcf_spi.c | 2 +- module/lua/lcompat.c | 2 +- module/lua/lstrlib.c | 2 +- module/os/windows/debug.c | 7 +- module/os/windows/driver.c | 2 +- module/os/windows/spl/spl-err.c | 2 +- module/os/windows/spl/spl-kmem.c | 128 +++++++++++++++++- module/os/windows/spl/spl-kstat.c | 19 ++- module/os/windows/spl/spl-proc_list.c | 4 +- module/os/windows/spl/spl-seg_kmem.c | 2 +- module/os/windows/spl/spl-taskq.c | 2 +- module/os/windows/spl/spl-windows.c | 4 +- module/os/windows/zfs/zfs_ctldir.c | 4 +- module/os/windows/zfs/zfs_debug.c | 16 ++- module/os/windows/zfs/zfs_ioctl_os.c | 5 +- module/os/windows/zfs/zfs_vnops_windows.c | 14 +- module/os/windows/zfs/zfs_vnops_windows_lib.c | 8 +- .../os/windows/zfs/zfs_vnops_windows_mount.c | 10 +- module/os/windows/zfs/zfs_windows_zvol.c | 26 ++-- module/os/windows/zfs/zfs_windows_zvol_scsi.c | 15 +- module/zcommon/zfs_fletcher.c | 4 +- module/zfs/dmu_send.c | 4 +- module/zfs/dsl_dir.c | 6 +- module/zfs/dsl_prop.c | 6 +- module/zfs/spa_misc.c | 6 +- module/zfs/vdev.c | 2 +- module/zfs/zcp.c | 4 +- module/zfs/zcp_get.c | 3 +- module/zfs/zfs_ioctl.c | 2 +- module/zfs/zio.c | 2 +- module/zfs/zio_inject.c | 2 +- 37 files changed, 423 insertions(+), 126 deletions(-) diff --git a/include/os/windows/spl/sys/kmem.h b/include/os/windows/spl/sys/kmem.h index f457df156439..9993b8419b94 100644 --- a/include/os/windows/spl/sys/kmem.h +++ b/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)) diff --git a/include/os/windows/spl/sys/types.h b/include/os/windows/spl/sys/types.h index 40cb9b22a3a4..db7b1fb545e8 100644 --- a/include/os/windows/spl/sys/types.h +++ b/include/os/windows/spl/sys/types.h @@ -94,12 +94,106 @@ typedef uintptr_t pc_t; #include #include #include +#include - -#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) diff --git a/lib/libspl/include/os/windows/sys/types.h b/lib/libspl/include/os/windows/sys/types.h index 10090c77652a..fb0e153febff 100644 --- a/lib/libspl/include/os/windows/sys/types.h +++ b/lib/libspl/include/os/windows/sys/types.h @@ -228,4 +228,47 @@ typedef uint64_t zoff_t; #include #endif +#include +#include + +/* + * 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 diff --git a/lib/libzfs/CMakeLists.txt b/lib/libzfs/CMakeLists.txt index 488bc9826742..5fc68999915e 100644 --- a/lib/libzfs/CMakeLists.txt +++ b/lib/libzfs/CMakeLists.txt @@ -20,12 +20,24 @@ 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" @@ -33,7 +45,8 @@ find_library(CRYPTO_STATIC_TEST 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}) diff --git a/lib/os/windows/zlib-1.2.3/gzio.c b/lib/os/windows/zlib-1.2.3/gzio.c index 8a8181c80b31..5c7ed4aaf16b 100644 --- a/lib/os/windows/zlib-1.2.3/gzio.c +++ b/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); /* for debugging */ + spl_snprintf(name, sizeof (name), "", 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); } diff --git a/module/icp/core/kcf_mech_tabs.c b/module/icp/core/kcf_mech_tabs.c index 2642b317d698..d9f185eea8b0 100644 --- a/module/icp/core/kcf_mech_tabs.c +++ b/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", diff --git a/module/icp/spi/kcf_spi.c b/module/icp/spi/kcf_spi.c index 34b36b81c0ab..edfb9765002c 100644 --- a/module/icp/spi/kcf_spi.c +++ b/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 { diff --git a/module/lua/lcompat.c b/module/lua/lcompat.c index c0a27182c7d8..a43ca10c331e 100644 --- a/module/lua/lcompat.c +++ b/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); diff --git a/module/lua/lstrlib.c b/module/lua/lstrlib.c index 12027757bf53..13e5941d6f5d 100644 --- a/module/lua/lstrlib.c +++ b/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; diff --git a/module/os/windows/debug.c b/module/os/windows/debug.c index d97637e44861..be8c349be086 100644 --- a/module/os/windows/debug.c +++ b/module/os/windows/debug.c @@ -28,6 +28,7 @@ #define _NO_CRT_STDIO_INLINE #include +#include #include #include #include @@ -51,7 +52,7 @@ static unsigned long long startOff = 0; int initDbgCircularBuffer(void) { - cbuf = ExAllocatePoolWithTag(NonPagedPoolNx, cbuf_size, '!GBD'); + cbuf = spl_ExAllocatePoolZero(NonPagedPoolNx, cbuf_size, '!GBD'); ASSERT(cbuf); KeInitializeSpinLock(&cbuf_spin); return (0); @@ -125,12 +126,12 @@ printBuffer(const char *fmt, ...) va_list args; va_start(args, fmt); char buf[max_line_length]; - _snprintf(buf, 18, "%p: ", PsGetCurrentThread()); + _snprintf_s(buf, sizeof (buf), _TRUNCATE, "%p: ", PsGetCurrentThread()); int tmp = _vsnprintf_s(&buf[17], sizeof (buf), max_line_length, fmt, args); if (tmp >= max_line_length) { - _snprintf(&buf[17], 17, "buffer too small"); + _snprintf_s(&buf[17], 17, _TRUNCATE, "buffer too small"); } KeAcquireSpinLock(&cbuf_spin, &level); diff --git a/module/os/windows/driver.c b/module/os/windows/driver.c index 5873fae229ef..995e27a636d2 100644 --- a/module/os/windows/driver.c +++ b/module/os/windows/driver.c @@ -319,7 +319,7 @@ spl_kstat_registry(void *arg, kstat_t *ksp) break; // Something is wrong - or we finished // Allocate space to hold - regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolWithTag( + regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolUninitialized( NonPagedPoolNx, length, 'zfsr'); if (regBuffer == NULL) diff --git a/module/os/windows/spl/spl-err.c b/module/os/windows/spl/spl-err.c index f54ff5405e27..d4250bfc51b2 100644 --- a/module/os/windows/spl/spl-err.c +++ b/module/os/windows/spl/spl-err.c @@ -36,7 +36,7 @@ vcmn_err(int ce, const char *fmt, va_list ap) { char msg[MAXMSGLEN]; - _vsnprintf(msg, MAXMSGLEN - 1, fmt, ap); + spl_vsnprintf(msg, MAXMSGLEN - 1, fmt, ap); switch (ce) { case CE_IGNORE: diff --git a/module/os/windows/spl/spl-kmem.c b/module/os/windows/spl/spl-kmem.c index 54cc47adbae4..4fad95d648c1 100644 --- a/module/os/windows/spl/spl-kmem.c +++ b/module/os/windows/spl/spl-kmem.c @@ -1885,7 +1885,7 @@ kmem_dumppr(char **pp, char *e, const char *format, ...) va_list ap; va_start(ap, format); - n = vsnprintf(p, e - p, format, ap); + n = spl_vsnprintf(p, e - p, format, ap); va_end(ap); *pp = p + n; } @@ -3560,7 +3560,7 @@ kmem_cache_create( /* * Set cache properties. */ - (void) strncpy(cp->cache_name, name, KMEM_CACHE_NAMELEN); + (void) spl_strlcpy(cp->cache_name, name, KMEM_CACHE_NAMELEN + 1); strident_canon(cp->cache_name, KMEM_CACHE_NAMELEN + 1); cp->cache_bufsize = bufsize; cp->cache_align = align; @@ -6613,6 +6613,118 @@ kmem_asdprintf(const char *fmt, ...) return (ptr); } +#define SPL_VSNPRINTF_PROBE_MIN 256 +/* + * 1 MiB: roughly 256x the largest single formatted string anywhere + * in this tree today (PAGE_SIZE == 4096, in zfs_fletcher.c). No real + * caller is expected to ever reach this; it exists only to bound a + * pathological/malformed format string's retry loop. + */ +#define SPL_VSNPRINTF_PROBE_MAX (1024 * 1024) + +/* + * True-length-preserving, deprecated-API-free replacement for + * _vsnprintf(). See the comment above its prototype in sys/types.h + * for why this can't be ntstrsafe.h-based and why it lives here + * rather than as a header inline. + * + * Contract (do not change without auditing every caller of + * spl_vsnprintf()/spl_snprintf()/snprintf() in the tree - e.g. + * dmu_redact.c, zcp_iter.c, zfs_fletcher.c, kmem_asprintf(), + * kmem_vasprintf()): + * - If the formatted string (plus NUL) fits in [buf, buf+size), + * it is written in full and the exact number of characters + * written (excluding the NUL) is returned. + * - Otherwise (including buf==NULL/size==0), the return value is + * still the exact number of characters the FULL, untruncated + * result would have needed - real snprintf() semantics, not + * _vsnprintf()'s -1 - even though buf itself may be left + * truncated exactly as _vsnprintf_s(..., _TRUNCATE, ...) leaves + * it (or untouched, if buf==NULL/size==0). + */ +int +spl_vsnprintf(char *buf, size_t size, const char *fmt, va_list args) +{ + va_list args_copy; + int ret; + size_t cap; + char stackbuf[SPL_VSNPRINTF_PROBE_MIN]; + + /* + * Tier 1: try the caller's own buffer first. This covers every + * call site that already passes a real, adequately sized + * buffer (the common case) with zero extra allocation - IRQL- + * safe (no allocation), cheaper than the old code, which always + * paid for a wasted measuring call even when the real write + * succeeded. + */ + if (buf != NULL && size > 0) { + args_copy = args; /* x64 MSVC va_list is a plain pointer */ + ret = _vsnprintf_s(buf, size, _TRUNCATE, fmt, args_copy); + if (ret >= 0) + return (ret); /* fit: ret IS the true length */ + } + + /* + * Tier 2: a small on-stack probe. Still IRQL-safe (no + * allocation) - every real caller in this tree writes a buffer + * under a few hundred bytes (the one known exception, + * module/lua/lstrlib.c's Lua channel-program formatting, is + * intentionally unbounded and falls through to Tier 3), so this + * is what makes every measure-only caller (buf==NULL, e.g. + * __dprintf's first call, kmem_asprintf(), kmem_vasprintf()) + * avoid the allocator entirely in the overwhelmingly common + * case. + */ + args_copy = args; + ret = _vsnprintf_s(stackbuf, sizeof (stackbuf), _TRUNCATE, fmt, + args_copy); + if (ret >= 0) + return (ret); + + /* + * Tier 3: only reached when even a 256-byte probe truncates. + * This is the only tier that allocates, so it is the only tier + * that can violate IRQL rules (KM_SLEEP can block) - guard it + * explicitly here, at the one place that actually needs it, + * rather than requiring every current and future caller + * (vcmn_err included) to remember its own guard. + * + * This is a real, if rare, new return value. Every live caller + * in the tree has been individually audited to confirm this is + * safe: callers that discard the return value are unaffected + * (their buffer is already correctly truncated by Tier 1/2's + * _vsnprintf_s call); kmem_vasprintf() already anticipates and + * handles a negative return from its measuring call; + * kmem_asprintf() is hardened alongside this change specifically + * because it previously was not safe against one. + */ + if (KeGetCurrentIrql() >= DISPATCH_LEVEL) + return (-1); + + cap = SPL_VSNPRINTF_PROBE_MIN * 2; + if (size > cap) + cap = size; + if (cap >= SPL_VSNPRINTF_PROBE_MAX) + cap = SPL_VSNPRINTF_PROBE_MAX; + + for (;;) { + /* KM_SLEEP: always succeeds, never returns NULL. */ + char *tmp = kmem_alloc(cap, KM_SLEEP); + args_copy = args; + ret = _vsnprintf_s(tmp, cap, _TRUNCATE, fmt, args_copy); + kmem_free(tmp, cap); + if (ret >= 0) + return (ret); + if (cap >= SPL_VSNPRINTF_PROBE_MAX) + return (-1); /* honest failure, not a fabricated length */ + if (cap > SPL_VSNPRINTF_PROBE_MAX / 2) + cap = SPL_VSNPRINTF_PROBE_MAX; + else + cap *= 2; + } +} + char * kmem_asprintf(const char *fmt, ...) { @@ -6621,13 +6733,17 @@ kmem_asprintf(const char *fmt, ...) char *buf; va_start(adx, fmt); - size = _vsnprintf(NULL, 0, fmt, adx) + 1; + size = spl_vsnprintf(NULL, 0, fmt, adx); va_end(adx); + if (size < 0) + return (NULL); /* honest failure, not KMEM_ZERO_SIZE_PTR */ + size++; + buf = kmem_alloc(size, KM_SLEEP); va_start(adx, fmt); - (void) _vsnprintf(buf, size, fmt, adx); + (void) spl_vsnprintf(buf, size, fmt, adx); va_end(adx); return (buf); @@ -6645,11 +6761,11 @@ kmem_vasprintf(const char *fmt, va_list ap) int size; int r = -1; - size = vsnprintf(NULL, 0, fmt, ap); + size = spl_vsnprintf(NULL, 0, fmt, ap); if ((size >= 0) && (size < INT_MAX)) { ptr = (char *)kmem_alloc(size + 1, KM_SLEEP); // +1 for null if (ptr) { - r = vsnprintf(ptr, size + 1, fmt, ap); // +1 for null + r = spl_vsnprintf(ptr, size + 1, fmt, ap); // +1 for null if ((r < 0) || (r > size)) { kmem_free(ptr, size); r = -1; diff --git a/module/os/windows/spl/spl-kstat.c b/module/os/windows/spl/spl-kstat.c index bd2a2c045b10..61579cf812dd 100644 --- a/module/os/windows/spl/spl-kstat.c +++ b/module/os/windows/spl/spl-kstat.c @@ -155,7 +155,7 @@ struct sbuf { /* sbuf_new() and family does exist in XNU, but Apple wont let us call them */ #define M_SBUF 105 /* string buffers */ #define SBMALLOC(size) \ - (struct sbuf *)ExAllocatePoolWithTag(NonPagedPoolNx, (size), '!SFZ') + (struct sbuf *)ExAllocatePoolUninitialized(NonPagedPoolNx, (size), '!SFZ') #define SBFREE(buf) ExFreePoolWithTag((buf), '!SFZ') #define SBUF_SETFLAG(s, f) do { (s)->s_flags |= (f); } while (0) @@ -309,10 +309,20 @@ sbuf_vprintf(struct sbuf *s, const char *fmt, va_list ap) do { va_copy(ap_copy, ap); - len = vsnprintf(&s->s_buf[s->s_len], SBUF_FREESPACE(s) + 1, + len = spl_vsnprintf(&s->s_buf[s->s_len], SBUF_FREESPACE(s) + 1, fmt, ap_copy); // left-side must be assignable. Win tries to set to 0. // va_end(ap_copy); + /* + * spl_vsnprintf() can return -1 on failure (e.g. format + * needs more than its ~1 MiB growth ceiling). Treat that + * as "nothing written" rather than let a negative len + * flow into the signed s_len accounting below, which + * would silently decrement s_len and corrupt the next + * sbuf_vprintf() call's buffer offset. + */ + if (len < 0) + len = 0; } while (len > SBUF_FREESPACE(s) && sbuf_extend(s, len - SBUF_FREESPACE(s)) == 0); s->s_len += min(len, SBUF_FREESPACE(s)); @@ -785,7 +795,7 @@ void kstat_set_string(char *dst, const char *src) { bzero(dst, KSTAT_STRLEN); - (void) strncpy(dst, src, KSTAT_STRLEN - 1); + (void) spl_strlcpy(dst, src, KSTAT_STRLEN); } void @@ -1034,7 +1044,8 @@ kstat_create_zone(const char *ks_module, int ks_instance, const char *ks_name, if (ks_name == NULL) { char buf[KSTAT_STRLEN]; kstat_set_string(buf, ks_module); - (void) sprintf(namebuf, "%s%d", buf, ks_instance); + (void) spl_snprintf(namebuf, sizeof (namebuf), "%s%d", buf, + ks_instance); ks_name = namebuf; } diff --git a/module/os/windows/spl/spl-proc_list.c b/module/os/windows/spl/spl-proc_list.c index 1d750447c77c..6fea0a75cf34 100644 --- a/module/os/windows/spl/spl-proc_list.c +++ b/module/os/windows/spl/spl-proc_list.c @@ -35,7 +35,7 @@ seq_printf(struct seq_file *f, const char *fmt, ...) va_list adx; va_start(adx, fmt); - (void) vsnprintf(f->sf_buf, f->sf_size, fmt, adx); + (void) spl_vsnprintf(f->sf_buf, f->sf_size, fmt, adx); va_end(adx); } @@ -81,7 +81,7 @@ procfs_list_addr(kstat_t *ksp, loff_t n) ksp->ks_private1 = list_next(&pl->pl_list, elt); if (ksp->ks_private1) { - p = ExAllocatePoolWithTag(NonPagedPoolNx, sizeof (*p), '!SFZ'); + p = ExAllocatePoolUninitialized(NonPagedPoolNx, sizeof (*p), '!SFZ'); p->pli_pl = pl; p->pli_elt = ksp->ks_private1; } diff --git a/module/os/windows/spl/spl-seg_kmem.c b/module/os/windows/spl/spl-seg_kmem.c index 958d7a4f2acb..e4b2f7bdfe4a 100644 --- a/module/os/windows/spl/spl-seg_kmem.c +++ b/module/os/windows/spl/spl-seg_kmem.c @@ -121,7 +121,7 @@ osif_malloc(uint64_t size) #ifdef _KERNEL void *tr = NULL; - tr = ExAllocatePoolWithTag(NonPagedPoolNx, size, '!SFZ'); + tr = ExAllocatePoolUninitialized(NonPagedPoolNx, size, '!SFZ'); ASSERT(P2PHASE(tr, PAGE_SIZE) == 0); if (tr != NULL) { atomic_inc_64(&stat_osif_malloc_success); diff --git a/module/os/windows/spl/spl-taskq.c b/module/os/windows/spl/spl-taskq.c index 499485d904c2..a521ea40982a 100644 --- a/module/os/windows/spl/spl-taskq.c +++ b/module/os/windows/spl/spl-taskq.c @@ -2458,7 +2458,7 @@ taskq_create_common(const char *name, int instance, int nthreads, pri_t pri, * Make sure the name is 0-terminated, and conforms to the rules for * C indentifiers */ - (void) strncpy(tq->tq_name, name, TASKQ_NAMELEN + 1); + (void) spl_strlcpy(tq->tq_name, name, TASKQ_NAMELEN + 1); strident_canon(tq->tq_name, TASKQ_NAMELEN + 1); tq->tq_flags = flags | TASKQ_CHANGING; diff --git a/module/os/windows/spl/spl-windows.c b/module/os/windows/spl/spl-windows.c index 3aabbaa91116..3faa96058587 100644 --- a/module/os/windows/spl/spl-windows.c +++ b/module/os/windows/spl/spl-windows.c @@ -713,7 +713,7 @@ spl_GetZfsTotalMemory(PUNICODE_STRING RegistryPath) break; // Something is wrong - or we finished // Allocate space to hold - regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolWithTag( + regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolUninitialized( NonPagedPoolNx, length, 'zfsr'); if (regBuffer == NULL) @@ -801,7 +801,7 @@ spl_getZfsPreallocSize(PUNICODE_STRING RegistryPath) break; // Something is wrong - or we finished // Allocate space to hold - regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolWithTag( + regBuffer = (PKEY_VALUE_FULL_INFORMATION)ExAllocatePoolUninitialized( NonPagedPoolNx, length, 'zfsr'); if (regBuffer == NULL) diff --git a/module/os/windows/zfs/zfs_ctldir.c b/module/os/windows/zfs/zfs_ctldir.c index b3fc0875a7e1..488c5f30f87d 100644 --- a/module/os/windows/zfs/zfs_ctldir.c +++ b/module/os/windows/zfs/zfs_ctldir.c @@ -995,8 +995,8 @@ zfsctl_snapshot_name(zfsvfs_t *zfsvfs, const char *snap_name, int len, if ((strlen(full_name) + 1 + strlen(snap_name)) >= len) return (SET_ERROR(ENAMETOOLONG)); - (void) strcat(full_name, "@"); - (void) strcat(full_name, snap_name); + (void) spl_strlcat(full_name, "@", len); + (void) spl_strlcat(full_name, snap_name, len); return (0); } diff --git a/module/os/windows/zfs/zfs_debug.c b/module/os/windows/zfs/zfs_debug.c index 01584506703d..36c3fd09e99b 100644 --- a/module/os/windows/zfs/zfs_debug.c +++ b/module/os/windows/zfs/zfs_debug.c @@ -228,7 +228,7 @@ __dprintf(boolean_t dprint, const char *file, const char *func, } va_start(adx, fmt); - size = vsnprintf(NULL, 0, fmt, adx); + size = spl_vsnprintf(NULL, 0, fmt, adx); va_end(adx); size += snprintf(NULL, 0, "%s%s:%d:%s(): ", prefix, newfile, line, @@ -236,6 +236,16 @@ __dprintf(boolean_t dprint, const char *file, const char *func, size++; /* null byte in the "buf" string */ + /* + * size is negative only if both spl_vsnprintf() measuring calls + * above independently failed (e.g. each needed more than the + * ~1 MiB spl_vsnprintf() will grow to) - not realistic for a + * single log line, but kmem_alloc() must never see a negative + * size turn into a huge size_t. + */ + if (size <= 0) + return; + /* * There is one byte of string in sizeof (zfs_dbgmsg_t), used * for the terminating null. @@ -244,9 +254,9 @@ __dprintf(boolean_t dprint, const char *file, const char *func, int roger = 0; va_start(adx, fmt); - i = snprintf(buf, size + 1, "%s%s:%d:%s(): ", + i = snprintf(buf, size, "%s%s:%d:%s(): ", prefix, newfile, line, func); - roger = vsnprintf(buf + i, size -i + 1, fmt, adx); + roger = spl_vsnprintf(buf + i, size - i, fmt, adx); va_end(adx); /* diff --git a/module/os/windows/zfs/zfs_ioctl_os.c b/module/os/windows/zfs/zfs_ioctl_os.c index 64f465b8b5d1..26155c33c8c4 100644 --- a/module/os/windows/zfs/zfs_ioctl_os.c +++ b/module/os/windows/zfs/zfs_ioctl_os.c @@ -139,7 +139,7 @@ NTSTATUS zpool_zfs_get_metrics(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_ perf->zpool_allocated = 0; perf->zpool_size = 0; perf->zfs_volSize = 0; - strncpy(perf->zpoolHealthState, "", sizeof(perf->zpoolHealthState)); + spl_strlcpy(perf->zpoolHealthState, "", sizeof(perf->zpoolHealthState)); perf->l2arc_alloc_size = 0; perf->l2arc_space = 0; perf->special_mirror_alloc_size = 0; @@ -185,7 +185,8 @@ NTSTATUS zpool_zfs_get_metrics(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_ perf->zpool_allocated = pool_alloc; perf->zpool_size = pool_size; - strcpy(perf->zpoolHealthState, healthState); + spl_strlcpy(perf->zpoolHealthState, healthState, + sizeof (perf->zpoolHealthState)); } else perf->zfs_volSize = getZvolSize(perf->name); diff --git a/module/os/windows/zfs/zfs_vnops_windows.c b/module/os/windows/zfs/zfs_vnops_windows.c index 96b68d138a84..4b498fe41a1c 100644 --- a/module/os/windows/zfs/zfs_vnops_windows.c +++ b/module/os/windows/zfs/zfs_vnops_windows.c @@ -297,7 +297,9 @@ stream_parse(char *filename, char **streamname) *colon = 0; // Cut off streamname from filename // We now ADD ":$DATA" to the stream name. - strcat(*streamname, ":$DATA"); + size_t remaining = PATH_MAX - (*streamname - filename); + if (spl_strlcat(*streamname, ":$DATA", remaining) >= remaining) + return (SET_ERROR(ENAMETOOLONG)); return (0); } @@ -419,7 +421,7 @@ zfs_find_dvp_vp(zfsvfs_t *zfsvfs, char *filename, int finalpartmaynotexist, * - maharmstone */ REPARSE_DATA_BUFFER *rpb; - rpb = ExAllocatePoolWithTag(PagedPool, + rpb = spl_ExAllocatePoolZero(PagedPool, zp->z_size, '!FSZ'); zfs_uio_t uio; struct iovec iov = { rpb, zp->z_size }; @@ -1763,7 +1765,7 @@ pnp_query_id(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp) zmo = (mount_t *)DeviceObject->DeviceExtension; - Irp->IoStatus.Information = (void *)ExAllocatePoolWithTag(PagedPool, + Irp->IoStatus.Information = (void *)spl_ExAllocatePoolZero(PagedPool, zmo->bus_name.Length + sizeof (UNICODE_NULL), '!OIZ'); if (Irp->IoStatus.Information == NULL) return (STATUS_NO_MEMORY); @@ -2281,7 +2283,7 @@ BufferUserBuffer(IN OUT PIRP Irp, IN ULONG BufferLength) if (Irp->AssociatedIrp.SystemBuffer == NULL) { UserBuffer = MapUserBuffer(Irp); Irp->AssociatedIrp.SystemBuffer = - FsRtlAllocatePoolWithQuotaTag(NonPagedPoolNx, + spl_ExAllocatePoolZero(NonPagedPoolNx, BufferLength, 'qtaf'); // @@ -5319,8 +5321,8 @@ _Function_class_(DRIVER_DISPATCH) TargetDeviceRelation) { PDEVICE_RELATIONS DeviceRelations; DeviceRelations = - (PDEVICE_RELATIONS)ExAllocatePool(PagedPool, - sizeof (DEVICE_RELATIONS)); + (PDEVICE_RELATIONS)ExAllocatePoolUninitialized(PagedPool, + sizeof (DEVICE_RELATIONS), '!DRZ'); if (!DeviceRelations) { TraceEvent(TRACE_NOISY, "enomem DeviceRelations\n"); Status = STATUS_INSUFFICIENT_RESOURCES; diff --git a/module/os/windows/zfs/zfs_vnops_windows_lib.c b/module/os/windows/zfs/zfs_vnops_windows_lib.c index 402b182b7610..c5f2f2e31d52 100644 --- a/module/os/windows/zfs/zfs_vnops_windows_lib.c +++ b/module/os/windows/zfs/zfs_vnops_windows_lib.c @@ -1559,7 +1559,7 @@ zfs_uid2sid(uint64_t uid, SID **sid) // Root? num = (uid == 0) ? 1 : 2; - tmp = ExAllocatePoolWithTag(PagedPool, + tmp = ExAllocatePoolUninitialized(PagedPool, offsetof(SID, SubAuthority) + (num * sizeof (ULONG)), 'zsid'); tmp->Revision = 1; @@ -1618,7 +1618,7 @@ zfs_gid2sid(uint64_t gid, SID **sid) ASSERT(sid != NULL); - tmp = ExAllocatePoolWithTag(PagedPool, + tmp = ExAllocatePoolUninitialized(PagedPool, offsetof(SID, SubAuthority) + (num * sizeof (ULONG)), 'zsid'); tmp->Revision = 1; @@ -1660,7 +1660,7 @@ zfs_set_acl(dacl *dacls) i++; } - acl = ExAllocatePoolWithTag(PagedPool, size, 'zacl'); + acl = ExAllocatePoolUninitialized(PagedPool, size, 'zacl'); if (!acl) return (NULL); @@ -1726,7 +1726,7 @@ zfs_set_security_root(struct vnode *vp) ASSERT(buflen != 0); - void *tmp = ExAllocatePoolWithTag(PagedPool, buflen, 'ZSEC'); + void *tmp = ExAllocatePoolUninitialized(PagedPool, buflen, 'ZSEC'); if (tmp == NULL) goto err; diff --git a/module/os/windows/zfs/zfs_vnops_windows_mount.c b/module/os/windows/zfs/zfs_vnops_windows_mount.c index 297205e4a456..4b8dd03fe447 100644 --- a/module/os/windows/zfs/zfs_vnops_windows_mount.c +++ b/module/os/windows/zfs/zfs_vnops_windows_mount.c @@ -325,15 +325,13 @@ SendVolumeArrivalNotification(PUNICODE_STRING DeviceName) dprintf("=> SendVolumeArrivalNotification: '%wZ'\n", DeviceName); length = sizeof (MOUNTMGR_TARGET_NAME) + DeviceName->Length - 1; - targetName = ExAllocatePool(PagedPool, length); + targetName = spl_ExAllocatePoolZero(PagedPool, length, 'ZVAN'); if (targetName == NULL) { dprintf(" can't allocate MOUNTMGR_TARGET_NAME\n"); return (STATUS_INSUFFICIENT_RESOURCES); } - RtlZeroMemory(targetName, length); - targetName->DeviceNameLength = DeviceName->Length; RtlCopyMemory(targetName->DeviceName, DeviceName->Buffer, DeviceName->Length); @@ -448,15 +446,13 @@ SendVolumeCreatePoint(__in PUNICODE_STRING DeviceName, length = sizeof (MOUNTMGR_CREATE_POINT_INPUT) + MountPoint->Length + DeviceName->Length; - point = ExAllocatePool(PagedPool, length); + point = spl_ExAllocatePoolZero(PagedPool, length, 'ZVCP'); if (point == NULL) { dprintf(" can't allocate MOUNTMGR_CREATE_POINT_INPUT\n"); return (STATUS_INSUFFICIENT_RESOURCES); } - RtlZeroMemory(point, length); - dprintf(" DeviceName: %wZ\n", DeviceName); point->DeviceNameOffset = sizeof (MOUNTMGR_CREATE_POINT_INPUT); point->DeviceNameLength = DeviceName->Length; @@ -1012,7 +1008,7 @@ generateVolumeNameMountpoint(wchar_t *vol_mpt) wchar_t wc_guid[50]; generateGUID(&GUID); mbstowcs(&wc_guid, GUID, 50); - int len = _snwprintf(vol_mpt, 50, L"\\??\\Volume{%s}", wc_guid); + (void) RtlStringCchPrintfW(vol_mpt, 50, L"\\??\\Volume{%s}", wc_guid); } int diff --git a/module/os/windows/zfs/zfs_windows_zvol.c b/module/os/windows/zfs/zfs_windows_zvol.c index d71f3d8810c2..b0055dab7906 100644 --- a/module/os/windows/zfs/zfs_windows_zvol.c +++ b/module/os/windows/zfs/zfs_windows_zvol.c @@ -104,19 +104,15 @@ zvol_start(PDRIVER_OBJECT DriverObject, PUNICODE_STRING pRegistryPath) // supporting more would mean bigger changes in the zv_targets // array. now we can go up to 32,640 zvols. pwzvolDrvInfo->NumberOfBuses = 1; + SIZE_T zvContextArraySize = (SIZE_T)pwzvolDrvInfo->MaximumNumberOfTargets * + pwzvolDrvInfo->MaximumNumberOfLogicalUnits * sizeof (wzvolContext); + pwzvolDrvInfo->zvContextArray = - (wzvolContext*)ExAllocatePoolWithTag(NonPagedPoolNx, - ((SIZE_T)pwzvolDrvInfo->MaximumNumberOfTargets * - pwzvolDrvInfo->MaximumNumberOfLogicalUnits * - sizeof (wzvolContext)), MP_TAG_GENERAL); + (wzvolContext*)spl_ExAllocatePoolZero(NonPagedPoolNx, + zvContextArraySize, MP_TAG_GENERAL); if (pwzvolDrvInfo->zvContextArray == NULL) return (STATUS_NO_MEMORY); - RtlZeroMemory(pwzvolDrvInfo->zvContextArray, - ((SIZE_T)pwzvolDrvInfo->MaximumNumberOfTargets * - pwzvolDrvInfo->MaximumNumberOfLogicalUnits * - (sizeof (wzvolContext)))); - RtlZeroMemory(&hwInitData, sizeof (VIRTUAL_HW_INITIALIZATION_DATA)); hwInitData.HwInitializationDataSize = @@ -345,11 +341,9 @@ wzvol_HwReportAdapter(__in pHW_HBA_EXT pHBAExt) WnodeSizeInstanceName + WnodeSizeDataBlock; - pWnode = ExAllocatePoolWithTag(NonPagedPoolNx, size, MP_TAG_GENERAL); + pWnode = spl_ExAllocatePoolZero(NonPagedPoolNx, size, MP_TAG_GENERAL); if (NULL != pWnode) { - RtlZeroMemory(pWnode, size); - // Fill out most of header. StorPort will set the // ProviderId and TimeStamp in the header. @@ -445,11 +439,9 @@ wzvol_HwReportLink(__in pHW_HBA_EXT pHBAExt) WnodeSizeInstanceName + WnodeSizeDataBlock; - pWnode = ExAllocatePoolWithTag(NonPagedPoolNx, size, MP_TAG_GENERAL); + pWnode = spl_ExAllocatePoolZero(NonPagedPoolNx, size, MP_TAG_GENERAL); if (NULL != pWnode) { - RtlZeroMemory(pWnode, size); - // Fill out most of header. StorPort will set the // ProviderId and TimeStamp in the header. @@ -535,11 +527,9 @@ wzvol_HwReportLog(__in pHW_HBA_EXT pHBAExt) WnodeSizeInstanceName + WnodeSizeDataBlock; - pWnode = ExAllocatePoolWithTag(NonPagedPoolNx, size, MP_TAG_GENERAL); + pWnode = spl_ExAllocatePoolZero(NonPagedPoolNx, size, MP_TAG_GENERAL); if (NULL != pWnode) { - RtlZeroMemory(pWnode, size); - // Fill out most of header. StorPort will set the // ProviderId and TimeStamp in the header. diff --git a/module/os/windows/zfs/zfs_windows_zvol_scsi.c b/module/os/windows/zfs/zfs_windows_zvol_scsi.c index d73fbaddaf63..10142719668c 100644 --- a/module/os/windows/zfs/zfs_windows_zvol_scsi.c +++ b/module/os/windows/zfs/zfs_windows_zvol_scsi.c @@ -156,7 +156,7 @@ wzvol_assign_targetid(zvol_state_t *zv) { wzvolContext* zv_targets = STOR_wzvolDriverInfo.zvContextArray; ASSERT(zv->zv_zso->zso_target_context == NULL); - PIO_REMOVE_LOCK pIoRemLock = ExAllocatePoolWithTag(NonPagedPoolNx, + PIO_REMOVE_LOCK pIoRemLock = ExAllocatePoolUninitialized(NonPagedPoolNx, sizeof (*pIoRemLock), MP_TAG_GENERAL); if (!pIoRemLock) { @@ -397,7 +397,7 @@ ScsiGetMPIOExt( } if (pNextEntry == &pHBAExt->pwzvolDrvObj->ListMPIOExt) { - pLUMPIOExt = ExAllocatePoolWithTag(NonPagedPoolNx, + pLUMPIOExt = spl_ExAllocatePoolZero(NonPagedPoolNx, sizeof (HW_LU_EXTENSION_MPIO), MP_TAG_GENERAL); if (!pLUMPIOExt) { @@ -405,8 +405,6 @@ ScsiGetMPIOExt( goto Done; } - RtlZeroMemory(pLUMPIOExt, sizeof (HW_LU_EXTENSION_MPIO)); - pLUMPIOExt->ScsiAddr.PathId = pSrb->PathId; pLUMPIOExt->ScsiAddr.TargetId = pSrb->TargetId; pLUMPIOExt->ScsiAddr.Lun = pSrb->Lun; @@ -515,7 +513,8 @@ ScsiOpInquiry( pHBAExt->ProductRevision, 4); memset((PCHAR)pInqData->VendorSpecific, ' ', sizeof (pInqData->VendorSpecific)); - sprintf(pInqData->VendorSpecific, "%.04d-%.04d-%.04d", + spl_snprintf(pInqData->VendorSpecific, + sizeof (pInqData->VendorSpecific), "%.04d-%.04d-%.04d", pSrb->PathId, pSrb->TargetId, pSrb->Lun); pInqData->VendorSpecific[strlen(pInqData->VendorSpecific)] = ' '; @@ -1088,7 +1087,11 @@ DiReadWriteSetup(zvol_state_t *zv, MpWkRtnAction action, zfsiodesc_t *pIo) { // cannot use kmem_alloc with sleep if IRQL dispatch so get straight // from NP pool. - pMP_WorkRtnParms pWkRtnParms = (pMP_WorkRtnParms)ExAllocatePoolWithTag( + // Not spl_ExAllocatePoolZero(): this allocation is deliberately + // larger than what gets zeroed below - the extra IoSizeofWorkItem() + // bytes are opaque storage that IoInitializeWorkItem() fills in + // itself, so zeroing them would be redundant work. + pMP_WorkRtnParms pWkRtnParms = (pMP_WorkRtnParms)ExAllocatePoolUninitialized( NonPagedPoolNx, ALIGN_UP_BY(sizeof (MP_WorkRtnParms), 16) + IoSizeofWorkItem(), MP_TAG_GENERAL); if (NULL == pWkRtnParms) { diff --git a/module/zcommon/zfs_fletcher.c b/module/zcommon/zfs_fletcher.c index ffd395474010..00bb44365a5d 100644 --- a/module/zcommon/zfs_fletcher.c +++ b/module/zcommon/zfs_fletcher.c @@ -900,12 +900,12 @@ fletcher_4_param_get(char *buffer, zfs_kernel_param_t *unused) /* list fastest */ fmt = IMPL_FMT(impl, IMPL_FASTEST); - cnt += sprintf(buffer + cnt, fmt, "fastest"); + cnt += spl_snprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, "fastest"); /* list all supported implementations */ for (uint32_t i = 0; i < fletcher_4_supp_impls_cnt; ++i) { fmt = IMPL_FMT(impl, i); - cnt += sprintf(buffer + cnt, fmt, + cnt += spl_snprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, fletcher_4_supp_impls[i]->name); } diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 551043fafbfc..31153df201cf 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -3000,8 +3000,8 @@ dmu_send_estimate_fast(dsl_dataset_t *origds, dsl_dataset_t *fromds, char dsname[ZFS_MAX_DATASET_NAME_LEN + 6]; dsl_dataset_name(origds, dsname); - (void) strcat(dsname, "/"); - (void) strcat(dsname, recv_clone_name); + (void) spl_strlcat(dsname, "/", sizeof (dsname)); + (void) spl_strlcat(dsname, recv_clone_name, sizeof (dsname)); err = dsl_dataset_hold(origds->ds_dir->dd_pool, dsname, FTAG, &ds); diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index e1c1e5d8ba81..48b4970eb5f7 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -441,8 +441,7 @@ getcomponent(const char *path, char *component, const char **nextp) if (p - path >= ZFS_MAX_DATASET_NAME_LEN) return (SET_ERROR(ENAMETOOLONG)); - (void) strncpy(component, path, p - path); - component[p - path] = '\0'; + (void) spl_strlcpy(component, path, (p - path) + 1); p++; } else if (p[0] == '@') { /* @@ -455,8 +454,7 @@ getcomponent(const char *path, char *component, const char **nextp) if (p - path >= ZFS_MAX_DATASET_NAME_LEN) return (SET_ERROR(ENAMETOOLONG)); - (void) strncpy(component, path, p - path); - component[p - path] = '\0'; + (void) spl_strlcpy(component, path, (p - path) + 1); } else { panic("invalid p=%p", (void *)p); } diff --git a/module/zfs/dsl_prop.c b/module/zfs/dsl_prop.c index ed42ac5aef3c..75afbaf78de3 100644 --- a/module/zfs/dsl_prop.c +++ b/module/zfs/dsl_prop.c @@ -58,7 +58,7 @@ dodefault(zfs_prop_t prop, int intsz, int numints, void *buf) if (intsz != 1) return (SET_ERROR(EOVERFLOW)); - (void) strncpy(buf, zfs_prop_default_string(prop), + (void) spl_strlcpy(buf, zfs_prop_default_string(prop), numints); } else { if (intsz != 8 || numints < 1) @@ -1029,8 +1029,8 @@ dsl_prop_get_all_impl(objset_t *mos, uint64_t propobj, if (flags & DSL_PROP_GET_LOCAL) continue; - (void) strncpy(buf, za.za_name, (suffix - za.za_name)); - buf[suffix - za.za_name] = '\0'; + (void) spl_strlcpy(buf, za.za_name, + (suffix - za.za_name) + 1); propname = buf; if (!(flags & DSL_PROP_GET_RECEIVED)) { diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c index 904256323143..0e3063ab9297 100644 --- a/module/zfs/spa_misc.c +++ b/module/zfs/spa_misc.c @@ -400,7 +400,7 @@ spa_load_failed(spa_t *spa, const char *fmt, ...) char buf[256]; va_start(adx, fmt); - (void) vsnprintf(buf, sizeof (buf), fmt, adx); + (void) spl_vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); zfs_dbgmsg("spa_load(%s, config %s): FAILED: %s", spa->spa_name, @@ -415,7 +415,7 @@ spa_load_note(spa_t *spa, const char *fmt, ...) char buf[256]; va_start(adx, fmt); - (void) vsnprintf(buf, sizeof (buf), fmt, adx); + (void) spl_vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); zfs_dbgmsg("spa_load(%s, config %s): %s", spa->spa_name, @@ -1677,7 +1677,7 @@ spa_altroot(spa_t *spa, char *buf, size_t buflen) if (spa->spa_root == NULL) buf[0] = '\0'; else - (void) strncpy(buf, spa->spa_root, buflen); + (void) spl_strlcpy(buf, spa->spa_root, buflen); } int diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 255202ab2bee..0935b32ed48e 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -145,7 +145,7 @@ vdev_dbgmsg(vdev_t *vd, const char *fmt, ...) char buf[256]; va_start(adx, fmt); - (void) vsnprintf(buf, sizeof (buf), fmt, adx); + (void) spl_vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); if (vd->vdev_path != NULL) { diff --git a/module/zfs/zcp.c b/module/zfs/zcp.c index 4c8af269ee01..08aced4c7f29 100644 --- a/module/zfs/zcp.c +++ b/module/zfs/zcp.c @@ -261,7 +261,7 @@ zcp_table_to_nvlist(lua_State *state, int index, int depth) /* check if this could collide with a number or bool */ long long tmp; int parselen; - if ((sscanf(key, "%lld%n", &tmp, &parselen) > 0 && + if ((sscanf_s(key, "%lld%n", &tmp, &parselen) > 0 && parselen == strlen(key)) || strcmp(key, "true") == 0 || strcmp(key, "false") == 0) { @@ -1244,7 +1244,7 @@ zcp_args_error(lua_State *state, const char *fname, const zcp_arg_t *pargs, va_list argp; va_start(argp, fmt); - VERIFY3U(len, >, vsnprintf(errmsg, len, fmt, argp)); + VERIFY3U(len, >, spl_vsnprintf(errmsg, len, fmt, argp)); va_end(argp); /* diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index 7256e4de1915..1d334b7783bb 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -611,8 +611,7 @@ parse_userquota_prop(const char *prop_name, zfs_userquota_prop_t *type, */ int domain_len = strrchr(cp, '-') - cp; domain_val = kmem_alloc(domain_len + 1, KM_SLEEP); - (void) strncpy(domain_val, cp, domain_len); - domain_val[domain_len] = '\0'; + (void) spl_strlcpy(domain_val, cp, domain_len + 1); cp += domain_len + 1; (void) ddi_strtoll(cp, &end, 10, (longlong_t *)rid); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index e7bb4a32f38f..c266aac80a62 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -745,7 +745,7 @@ zfs_get_parent(const char *datasetname, char *parent, int parentsize) /* * Remove the @bla or /bla from the end of the name to get the parent. */ - (void) strncpy(parent, datasetname, parentsize); + (void) spl_strlcpy(parent, datasetname, parentsize); cp = strrchr(parent, '@'); if (cp != NULL) { cp[0] = '\0'; diff --git a/module/zfs/zio.c b/module/zfs/zio.c index be06c386e3df..4cc3a2ef3584 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -929,7 +929,7 @@ zfs_blkptr_verify_log(spa_t *spa, const blkptr_t *bp, char buf[256]; va_start(adx, fmt); - (void) vsnprintf(buf, sizeof (buf), fmt, adx); + (void) spl_vsnprintf(buf, sizeof (buf), fmt, adx); va_end(adx); switch (blk_verify) { diff --git a/module/zfs/zio_inject.c b/module/zfs/zio_inject.c index f494db4a20ef..aacff6a6142f 100644 --- a/module/zfs/zio_inject.c +++ b/module/zfs/zio_inject.c @@ -894,7 +894,7 @@ zio_inject_list_next(int *id, char *name, size_t buflen, if (handler) { *record = handler->zi_record; *id = handler->zi_id; - (void) strncpy(name, spa_name(handler->zi_spa), buflen); + (void) spl_strlcpy(name, spa_name(handler->zi_spa), buflen); ret = 0; } else { ret = SET_ERROR(ENOENT);