NetBSD Problem Report #60034
From www@netbsd.org Tue Feb 24 22:53:23 2026
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
client-signature RSA-PSS (2048 bits) client-digest SHA256)
(Client CN "mail.netbsd.org", Issuer "R13" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 87BFB1A9239
for <gnats-bugs@gnats.NetBSD.org>; Tue, 24 Feb 2026 22:53:23 +0000 (UTC)
Message-Id: <20260224225321.E81591A923C@mollari.NetBSD.org>
Date: Tue, 24 Feb 2026 22:53:21 +0000 (UTC)
From: joaoboni017@gmail.com
Reply-To: joaoboni017@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Eliminate -Wformat-nonliteral warnings and improve security in ahd, ddb, and dev_verbose.c
X-Send-Pr-Version: www-1.0
X-From4GNATS: "joaoboni017@gmail.com via gnats" <gnats-admin@NetBSD.org>
>Number: 60034
>Category: kern
>Synopsis: Eliminate -Wformat-nonliteral warnings and improve security in ahd, ddb, and dev_verbose.c
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: change-request
>Submitter-Id: net
>Arrival-Date: Tue Feb 24 22:55:00 +0000 2026
>Last-Modified: Fri Feb 27 15:45:01 +0000 2026
>Originator: João Bonifácio
>Release: NetBSD-current (11.99.5) as of February 2026
>Organization:
>Environment:
NetBSD 11.99.5 amd64 (Cross-compiled on Linux 6.18.12-100.fc42.x86_64)
>Description:
This PR addresses several -Wformat-nonliteral warnings across different
subsystems (pci, ddb, and dev_verbose). These warnings are currently
blocking the transition to a higher WARNS level (WARNS=5) for these files.
Summary of changes:
1. sys/dev/pci/ahd_pci.c: Refactored pci_status_strings and split_status_strings
to contain plain messages, passing them as "%s" arguments to printf.
2. sys/ddb/db_output.c: Refactored db_format_radix and db_format_hex to use
explicit snprintf format literals. Casted quad_t values to (unsigned long long)
to ensure format string compatibility.
3. sys/dev/dev_verbose.c: Replaced dynamic snprintf format strings with
explicit literal checks.
These changes improve kernel security by preventing potential format string
vulnerabilities and ensure the code is compatible with strict compiler checks.
>How-To-Repeat:
Compile the kernel with WARNS=5 or manually add -Wformat-nonliteral to
COPTS. The build will fail with -Werror in the mentioned files.
>Fix:
Apply the attached patches (0001 to 0003).
All patches have been tested for compilation on amd64 and preserve the
original dmesg output/formatting.
Patch 1: fix: sys/dev/pci/ahd_pci.c -Wformat-nonliteral errors
>From 6f8770ee46ab6c90e573ea3337a6920acb851ecb Mon Sep 17 00:00:00 2001
From: Joao Bonifacio <joaoboni017@gmail.com>
Date: Mon, 16 Feb 2026 11:03:48 -0300
Subject: [PATCH 1/3] fix: sys/dev/pci/ahd_pci.c -Wformat-nonliteral errors
Avoid potential security issues and compilation errors by ensuring
printf format strings are literals. Variable messages from
pci_status_strings and split_status_strings are now passed as
arguments (%s).
No functional change intended.
Signed-off-by: Joao Bonifacio <joaoboni017@gmail.com>
---
sys/dev/pci/ahd_pci.c | 54 +++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/sys/dev/pci/ahd_pci.c b/sys/dev/pci/ahd_pci.c
index daba375a9..cebeaaa94 100644
--- a/sys/dev/pci/ahd_pci.c
+++ b/sys/dev/pci/ahd_pci.c
@@ -924,26 +924,26 @@ static const char *pci_status_source[] =
static const char *split_status_strings[] =
{
- "%s: Received split response in %s.\n",
- "%s: Received split completion error message in %s\n",
- "%s: Receive overrun in %s\n",
- "%s: Count not complete in %s\n",
- "%s: Split completion data bucket in %s\n",
- "%s: Split completion address error in %s\n",
- "%s: Split completion byte count error in %s\n",
- "%s: Signaled Target-abort to early terminate a split in %s\n"
+ "Received split response",
+ "Received split completion error message",
+ "Receive overrun",
+ "Count not complete",
+ "Split completion data bucket",
+ "Split completion address error",
+ "Split completion byte count error",
+ "Signaled Target-abort to early terminate a split"
};
static const char *pci_status_strings[] =
{
- "%s: Data Parity Error has been reported via PERR# in %s\n",
- "%s: Target initial wait state error in %s\n",
- "%s: Split completion read data parity error in %s\n",
- "%s: Split completion address attribute parity error in %s\n",
- "%s: Received a Target Abort in %s\n",
- "%s: Received a Master Abort in %s\n",
- "%s: Signal System Error Detected in %s\n",
- "%s: Address or Write Phase Parity Error Detected in %s.\n"
+ "Data Parity Error has been reported via PERR#",
+ "Target initial wait state error",
+ "Split completion read data parity error",
+ "Split completion address attribute parity error",
+ "Received a Target Abort",
+ "Received a Master Abort",
+ "Signal System Error Detected",
+ "Address or Write Phase Parity Error Detected"
};
static int
@@ -987,12 +987,12 @@ ahd_pci_intr(struct ahd_softc *ahd)
for (bit = 0; bit < 8; bit++) {
if ((pci_status[i] & (0x1 << bit)) != 0) {
- static const char *s;
+ static const char *msg;
- s = pci_status_strings[bit];
+ msg = pci_status_strings[bit];
if (i == 7/*TARG*/ && bit == 3)
- s = "%s: Signaled Target Abort\n";
- printf(s, ahd_name(ahd), pci_status_source[i]);
+ msg = "Signaled Target Abort\n";
+ printf("%s: %s in %s\n", ahd_name(ahd), msg, pci_status_source[i]);
}
}
}
@@ -1051,21 +1051,21 @@ ahd_pci_split_intr(struct ahd_softc *ahd, u_int intstat)
for (bit = 0; bit < 8; bit++) {
if ((split_status[i] & (0x1 << bit)) != 0) {
- static const char *s;
+ static const char *msg;
- s = split_status_strings[bit];
- printf(s, ahd_name(ahd),
- split_status_source[i]);
+ msg = split_status_strings[bit];
+ printf("%s: %s in %s\n",
+ ahd_name(ahd), msg, split_status_source[i]);
}
if (i > 0)
continue;
if ((sg_split_status[i] & (0x1 << bit)) != 0) {
- static const char *s;
+ static const char *msg;
- s = split_status_strings[bit];
- printf(s, ahd_name(ahd), "SG");
+ msg = split_status_strings[bit];
+ printf("%s: %s in %s\n", ahd_name(ahd), msg, "SG");
}
}
}
--
2.53.0
Patch 2: Fix format-nonliteral and type validation in db_output.c
>From 53ad467d9b2a430fa5d012baa34a6e2488c346c2 Mon Sep 17 00:00:00 2001
From: Joao Bonifacio <joaoboni017@gmail.com>
Date: Fri, 20 Feb 2026 23:45:49 -0300
Subject: [PATCH 2/3] sys/ddb: Fix format-nonliteral and type validation in
db_output.c
Refactor db_format_radix() and db_format_hex() to use explicit string
literals in snprintf() calls.
The previous implementation relied on dynamic pointer manipulation and
arithmetic to toggle the negative sign prefix. This approach bypasses
static analysis of the format string, leading to potential type safety
issues and build failures under strict compiler security flags.
This change replaces the dynamic logic with explicit conditional blocks
and static format strings. This enables proper validation of 'quad_t'
arguments against their respective conversion specifiers, ensuring
better code safety and compatibility with modern toolchains.
No functional changes intended.
Signed-off-by: Joao Bonifacio <joaoboni017@gmail.com>
---
sys/ddb/db_output.c | 50 ++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/sys/ddb/db_output.c b/sys/ddb/db_output.c
index a0662dbb2..03d111849 100644
--- a/sys/ddb/db_output.c
+++ b/sys/ddb/db_output.c
@@ -219,25 +219,27 @@ db_end_line(void)
void
db_format_radix(char *buf, size_t bufsiz, quad_t val, int altflag)
{
- const char *fmt;
if (db_radix == 16) {
db_format_hex(buf, bufsiz, val, altflag);
return;
}
- if (db_radix == 8)
- fmt = altflag ? "-%#qo" : "-%qo";
- else
- fmt = altflag ? "-%#qu" : "-%qu";
-
- if (val < 0)
- val = -val;
- else
- ++fmt;
-
- snprintf(buf, bufsiz, fmt, val);
-}
+ if (val < 0)
+ {
+ val = -val;
+ if (db_radix == 8)
+ snprintf(buf, bufsiz, altflag ? "-%#llo" : "-%llo", (unsigned long long)val);
+ else
+ snprintf(buf, bufsiz, "-%llu", (unsigned long long)val);
+ }
+ else {
+ if (db_radix == 8)
+ snprintf(buf, bufsiz, altflag ? "%#llo" : "%llo",(unsigned long long)val);
+ else
+ snprintf(buf, bufsiz, "%llu", (unsigned long long)val);
+ }
+ }
/*
* Replacement for old '%z' kprintf format.
@@ -245,15 +247,21 @@ db_format_radix(char *buf, size_t bufsiz, quad_t val, int altflag)
void
db_format_hex(char *buf, size_t bufsiz, quad_t val, int altflag)
{
- /* Only use alternate form if val is nonzero. */
- const char *fmt = (altflag && val) ? "-%#qx" : "-%qx";
-
- if (val < 0)
+ /*Negative number*/
+ if (val < 0){
val = -val;
- else
- ++fmt;
-
- snprintf(buf, bufsiz, fmt, val);
+ if (altflag)
+ snprintf(buf, bufsiz, "-%#llx", (unsigned long long)val);
+ else
+ snprintf(buf, bufsiz, "-%llx", (unsigned long long)val);
+ }
+ /*Positive number*/
+ else{
+ if (altflag && val)
+ snprintf(buf, bufsiz, "%#llx",(unsigned long long)val);
+ else
+ snprintf(buf, bufsiz, "%llx", (unsigned long long)val);
+ }
}
/*
--
2.53.0
Patch 3: Fix -Wformat-nonliteral and improve security in dev_verbose.c
>From e0770ec54298276f07c27b06a89a56dc853a0cc5 Mon Sep 17 00:00:00 2001
From: Joao Bonifacio <joaoboni017@gmail.com>
Date: Sat, 21 Feb 2026 17:32:52 -0300
Subject: [PATCH 3/3] sys/dev: Fix -Wformat-nonliteral and improve security in
dev_verbose.c
This patch replaces the use of a dynamic format string with static literals in dev_findvendor and dev_findproduct.
By using fixed strings like '0x%04x', we ensure that the compiler can verify the arguments at build time. More importantly, this makes the code immune to 'format string attacks'. Previously, a buggy driver or malicious hardware could theoretically pass a crafted format string (like '%s%s%s') to trigger a kernel panic or leak sensitive memory. Now, the format is controlled by the kernel, not by the input data.
Tested on amd64: compiles cleanly with -Werror.
Signed-off-by: Joao Bonifacio <joaoboni017@gmail.com>
---
sys/dev/dev_verbose.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/sys/dev/dev_verbose.c b/sys/dev/dev_verbose.c
index 5c46ed425..9df5300d1 100644
--- a/sys/dev/dev_verbose.c
+++ b/sys/dev/dev_verbose.c
@@ -73,7 +73,16 @@ dev_findvendor(char *buf, size_t len, const char *words, size_t nwords,
while (n < nvendors && vendors[n] != 0)
n++;
}
- snprintf(buf, len, fmt, vendor);
+
+ /*
+ * Avoid -Wformat-nonliteral:
+ */
+ if (fmt != NULL && strcmp(fmt, "0x%04x") == 0){
+ snprintf(buf, len, "0x%04x", vendor);
+ } else {
+ snprintf(buf, len, "0x%08x", vendor);
+ }
+
return NULL;
}
@@ -94,6 +103,14 @@ dev_findproduct(char *buf, size_t len, const char *words, size_t nwords,
while (n < nproducts && products[n] != 0)
n++;
}
- snprintf(buf, len, fmt, product);
+ /*
+ * Avoid Wformat-nonliteral
+ */
+ if (fmt != NULL && strcmp(fmt, "0x%04x") == 0){
+ snprintf(buf, len, "0x%04x", product);
+ } else {
+ snprintf(buf, len, "0x%08x", product);
+ }
+
return NULL;
}
--
2.53.0
Similar fixes are needed for ixgbe(4), which I am still investigating.
>Audit-Trail:
From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/60034: Eliminate -Wformat-nonliteral warnings and improve
security in ahd, ddb, and dev_verbose.c
Date: Wed, 25 Feb 2026 03:46:02 +0300
Just a quick drive-by comment, I haven't actually looked at the code
too closely.
On Tue, Feb 24, 2026 at 22:55:00 +0000, joaoboni017@gmail.com via gnats wrote:
> This PR addresses several -Wformat-nonliteral warnings across
> different subsystems (pci, ddb, and dev_verbose). These warnings
> are currently blocking the transition to a higher WARNS level
> (WARNS=5) for these files.
I haven't checked, but all of these variable format strings actually
come from our own source code, so it's not _too_ bad. And you can
shut up gcc with something like:
https://hg.sr.ht/~nbuwe/printf_checkformat/browse/printf_checkformat.c?rev=edc408c65ea3#L61
/*
* A do-nothing counterpart to printf_checkformat(3) (or fmtcheck(3))
* that only supplies the __format_arg marker. Actual check is
* expected to have been done once, in advance.
*
* printf(format_ok("%d", userfmt), arg);
*/
__always_inline __format_arg(1)
static inline const char *
format_ok(const char *template, const char *userfmt)
{
(void) template;
return userfmt;
}
> @@ -94,6 +103,14 @@ dev_findproduct(char *buf, size_t len, const char *words, size_t nwords,
> while (n < nproducts && products[n] != 0)
> n++;
> }
> - snprintf(buf, len, fmt, product);
> + /*
> + * Avoid Wformat-nonliteral
> + */
> + if (fmt != NULL && strcmp(fmt, "0x%04x") == 0){
> + snprintf(buf, len, "0x%04x", product);
> + } else {
> + snprintf(buf, len, "0x%08x", product);
> + }
> +
This seems to completely ignore the formats passed to this function,
like
#define mii_id1_format "oui %6.6x"
#define mii_id2_format "model %4.4x"
-uwe
From: Joao Bonifacio <joaoboni017@gmail.com>
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: kern/60034: Eliminate -Wformat-nonliteral warnings and improve
security in ahd, ddb, and dev_verbose.c
Date: Fri, 27 Feb 2026 12:44:26 -0300
Hi Valery Ushakov, i wish you're fine.
Thanks for the feedback.
I agree that since these formats are internal, using a __format_arg
helper is a much more better way to satisfy WARNS=3D5 without breaking
flexibility or bloating the code with conditionals.
I will rework the three patches into a V2 using a format_ok inline
helper (or similar) to keep the original logic intact. This should also
solve the issues I was seeing with ixgbe.
I'll send the updated patches soon.
Em qua, 2026-02-25 =C3=A0s 01:05 +0000, Valery Ushakov via gnats escreveu:
> The following reply was made to PR kern/60034; it has been noted by
> GNATS.
>=20
> From: Valery Ushakov <uwe@stderr.spb.ru>
> To: gnats-bugs@netbsd.org
> Cc:=20
> Subject: Re: kern/60034: Eliminate -Wformat-nonliteral warnings and
> improve
> =C2=A0security in ahd, ddb, and dev_verbose.c
> Date: Wed, 25 Feb 2026 03:46:02 +0300
>=20
> =C2=A0Just a quick drive-by comment, I haven't actually looked at the cod=
e
> =C2=A0too closely.
> =C2=A0
> =C2=A0On Tue, Feb 24, 2026 at 22:55:00 +0000, joaoboni017@gmail.com=C2=A0=
via
> gnats wrote:
> =C2=A0
> =C2=A0> This PR addresses several -Wformat-nonliteral warnings across
> =C2=A0> different subsystems (pci, ddb, and dev_verbose).=C2=A0 These war=
nings
> =C2=A0> are currently blocking the transition to a higher WARNS level
> =C2=A0> (WARNS=3D5) for these files.
> =C2=A0
> =C2=A0I haven't checked, but all of these variable format strings actuall=
y
> =C2=A0come from our own source code, so it's not _too_ bad.=C2=A0 And you=
can
> =C2=A0shut up gcc with something like:
> =C2=A0
> =C2=A0
> https://hg.sr.ht/~nbuwe/printf_checkformat/browse/printf_checkformat.c
> ?rev=3Dedc408c65ea3#L61
> =C2=A0
> =C2=A0/*
> =C2=A0 * A do-nothing counterpart to printf_checkformat(3) (or
> fmtcheck(3))
> =C2=A0 * that only supplies the __format_arg marker.=C2=A0 Actual check i=
s
> =C2=A0 * expected to have been done once, in advance.
> =C2=A0 *
> =C2=A0 *=C2=A0=C2=A0 printf(format_ok("%d", userfmt), arg);
> =C2=A0 */
> =C2=A0__always_inline __format_arg(1)
> =C2=A0static inline const char *
> =C2=A0format_ok(const char *template, const char *userfmt)
> =C2=A0{
> =C2=A0 (void) template;
> =C2=A0 return userfmt;
> =C2=A0}
> =C2=A0
> =C2=A0
> =C2=A0
> =C2=A0> @@ -94,6 +103,14 @@ dev_findproduct(char *buf, size_t len, const
> char *words, size_t nwords,
> =C2=A0>=C2=A0=C2=A0 while (n < nproducts && products[n] !=3D 0)
> =C2=A0>=C2=A0=C2=A0 n++;
> =C2=A0>=C2=A0=C2=A0 }
> =C2=A0> - snprintf(buf, len, fmt, product);
> =C2=A0> +=C2=A0=C2=A0=C2=A0 /*
> =C2=A0> +=C2=A0=C2=A0=C2=A0=C2=A0 * Avoid Wformat-nonliteral
> =C2=A0> +=C2=A0=C2=A0=C2=A0=C2=A0 */
> =C2=A0> +=C2=A0=C2=A0=C2=A0=C2=A0 if (fmt !=3D NULL && strcmp(fmt, "0x%04=
x") =3D=3D 0){
> =C2=A0> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 snprintf(buf, len, "0=
x%04x", product);
> =C2=A0> +=C2=A0=C2=A0=C2=A0 } else {
> =C2=A0> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 snprintf(buf, len, "0=
x%08x", product);
> =C2=A0> +=C2=A0=C2=A0=C2=A0 }
> =C2=A0> +
> =C2=A0
> =C2=A0This seems to completely ignore the formats passed to this function=
,
> =C2=A0like
> =C2=A0
> =C2=A0#define mii_id1_format "oui %6.6x"
> =C2=A0#define mii_id2_format "model %4.4x"
> =C2=A0
> =C2=A0-uwe
> =C2=A0
(Contact us)
$NetBSD: query-full-pr,v 1.51 2026/08/10 02:28:17 riastradh Exp $
$NetBSD: gnats_config.sh,v 1.10 2026/05/13 22:00:09 riastradh Exp $
Copyright © 1994-2026
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home