All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 0/3] don't build printf formats out of names
@ 2026-10-19 20:03 Stephen Hemminger
  2026-10-19 20:03 ` [PATCH iproute 1/3] lib: " Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-10-19 20:03 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

Several print helpers built a printf format string by concatenating a
name or key onto a conversion specifier, then passed the result to
printf as the format. This is unsafe since '%' in name would
be converted by printf.

Most of the affected names are string literals, but the rdma driver
attribute and hwcounter keys and the vdpa vendor statistics names come
from the kernel, so they are not under iproute2's control. No failures
have been reported in practice. Each patch passes the name as a normal
string argument with a literal format instead; text and JSON output are
otherwise unchanged.

Found by an automated source scan (ANT-2026-JAWA8082).

Stephen Hemminger (3):
  lib: don't build printf formats out of names
  rdma: don't build printf formats out of keys
  vdpa: don't build printf format out of vendor stats name

 lib/json_print.c |  7 ++-----
 rdma/res-cmid.c  |  3 ++-
 rdma/res.c       | 10 +++++-----
 rdma/utils.c     |  4 ++--
 vdpa/vdpa.c      | 14 +++-----------
 5 files changed, 14 insertions(+), 24 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH iproute 1/3] lib: don't build printf formats out of names
  2026-10-19 20:03 [PATCH iproute2 0/3] don't build printf formats out of names Stephen Hemminger
@ 2026-10-19 20:03 ` Stephen Hemminger
  2026-10-19 20:03 ` [PATCH iproute 2/3] rdma: don't build printf formats out of keys Stephen Hemminger
  2026-10-19 20:03 ` [PATCH iproute 3/3] vdpa: don't build printf format out of vendor stats name Stephen Hemminger
  2 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-10-19 20:03 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

print_string_name_value() and print_uint_name_value() snprintf'd the
caller's name into a 64-byte buffer and used it as the printf format, so
a '%' in the name was taken as a conversion and names over 63 bytes lost
their value to truncation. Pass the name as a string argument with a
literal format instead.

Fixes: 31ca29b2bec4 ("json_print: Introduce print_#type_name_value")
Reported-by: Anthropic OSS scanner
Assiste-By: Claude:claude-opus-5-5
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/json_print.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/lib/json_print.c b/lib/json_print.c
index f604f265..e028e766 100644
--- a/lib/json_print.c
+++ b/lib/json_print.c
@@ -147,11 +147,8 @@ _PRINT_FUNC(float, double);
 #define _PRINT_NAME_VALUE_FUNC(type_name, type, format_char)		 \
 	void print_##type_name##_name_value(const char *name, type value)\
 	{								 \
-		SPRINT_BUF(format);					 \
-									 \
-		snprintf(format, SPRINT_BSIZE,				 \
-			 "%s %%"#format_char, name);			 \
-		print_##type_name(PRINT_ANY, name, format, value);	 \
+		print_string(PRINT_FP, NULL, "%s ", name);		 \
+		print_##type_name(PRINT_ANY, name, "%"#format_char, value); \
 	}
 _PRINT_NAME_VALUE_FUNC(uint, unsigned int, u);
 _PRINT_NAME_VALUE_FUNC(string, const char*, s);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH iproute 2/3] rdma: don't build printf formats out of keys
  2026-10-19 20:03 [PATCH iproute2 0/3] don't build printf formats out of names Stephen Hemminger
  2026-10-19 20:03 ` [PATCH iproute 1/3] lib: " Stephen Hemminger
@ 2026-10-19 20:03 ` Stephen Hemminger
  2026-10-20 13:10   ` Leon Romanovsky
  2026-10-19 20:03 ` [PATCH iproute 3/3] vdpa: don't build printf format out of vendor stats name Stephen Hemminger
  2 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2026-10-19 20:03 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

print_key(), res_print_u32(), res_print_u64(), print_driver_string() and
print_ipaddr() passed the attribute key as both the json key and the
printf format, so a '%' in a kernel-supplied driver or hwcounter key was
taken as a conversion. Pass the key as a string argument with a literal
format; output is unchanged.

Fixes: 9b578bbaded6 ("rdma: use print_XXX instead of COLOR_NONE")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 rdma/res-cmid.c |  3 ++-
 rdma/res.c      | 10 +++++-----
 rdma/utils.c    |  4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c
index 17a89cc4..956253fe 100644
--- a/rdma/res-cmid.c
+++ b/rdma/res-cmid.c
@@ -53,7 +53,8 @@ static void print_ipaddr(const char *key, char *addrstr, uint16_t port)
 	char json_name[name_size];
 
 	snprintf(json_name, name_size, "%s:%u", addrstr, port);
-	print_string(PRINT_ANY, key, key, json_name);
+	print_string(PRINT_JSON, key, NULL, json_name);
+	print_string(PRINT_FP, NULL, "%s", key);
 	print_string(PRINT_FP, NULL, " %s:", addrstr);
 	print_uint(PRINT_FP, NULL, "%u ", port);
 }
diff --git a/rdma/res.c b/rdma/res.c
index 7efced8d..185612f7 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -222,7 +222,7 @@ void print_key(const char *name, uint64_t val, struct nlattr *nlattr)
 {
 	if (!nlattr)
 		return;
-	print_string(PRINT_FP, NULL, name, NULL);
+	print_string(PRINT_FP, NULL, "%s", name);
 	print_hex(PRINT_ANY, name, " 0x%" PRIx64 " ", val);
 }
 
@@ -230,16 +230,16 @@ void res_print_u32(const char *name, uint32_t val, struct nlattr *nlattr)
 {
 	if (!nlattr)
 		return;
-	print_uint(PRINT_ANY, name, name, val);
-	print_uint(PRINT_FP, NULL, " %" PRIu32 " ", val);
+	print_string(PRINT_FP, NULL, "%s", name);
+	print_uint(PRINT_ANY, name, " %" PRIu32 " ", val);
 }
 
 void res_print_u64(const char *name, uint64_t val, struct nlattr *nlattr)
 {
 	if (!nlattr)
 		return;
-	print_u64(PRINT_ANY, name, name, val);
-	print_u64(PRINT_FP, NULL, " %" PRIu64 " ", val);
+	print_string(PRINT_FP, NULL, "%s", name);
+	print_u64(PRINT_ANY, name, " %" PRIu64 " ", val);
 }
 
 RES_FUNC(res_no_args,	RDMA_NLDEV_CMD_RES_GET,	NULL, true, 0);
diff --git a/rdma/utils.c b/rdma/utils.c
index 90ea1c55..f6605eef 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -795,8 +795,8 @@ void newline_indent(void)
 
 static int print_driver_string(const char *key_str, const char *val_str)
 {
-	print_string(PRINT_ANY, key_str, key_str, val_str);
-	print_string(PRINT_FP, NULL, " %s ", val_str);
+	print_string(PRINT_FP, NULL, "%s", key_str);
+	print_string(PRINT_ANY, key_str, " %s ", val_str);
 	return 0;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH iproute 3/3] vdpa: don't build printf format out of vendor stats name
  2026-10-19 20:03 [PATCH iproute2 0/3] don't build printf formats out of names Stephen Hemminger
  2026-10-19 20:03 ` [PATCH iproute 1/3] lib: " Stephen Hemminger
  2026-10-19 20:03 ` [PATCH iproute 2/3] rdma: don't build printf formats out of keys Stephen Hemminger
@ 2026-10-19 20:03 ` Stephen Hemminger
  2 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-10-19 20:03 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

pr_out_dev_net_vstats() appended " %lu " to the driver-supplied vendor
attribute name and used it as the printf format, so a '%' in the name was
taken as a conversion, and names over MAX_KEY_LEN aborted the rest of the
dump. Pass the name as a string argument with a literal format, which
drops the length limit, and use PRIu64 since uint64_t is not unsigned
long on 32-bit.

Fixes: 6f97e9c9337b ("vdpa: Add support for reading vdpa device statistics")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 vdpa/vdpa.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
index e2b0a5b1..8719a636 100644
--- a/vdpa/vdpa.c
+++ b/vdpa/vdpa.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
 #include <stdio.h>
+#include <inttypes.h>
 #include <getopt.h>
 #include <errno.h>
 #include <sys/time.h>
@@ -890,10 +891,6 @@ static int cmd_dev_config(struct vdpa *vdpa, int argc, char **argv)
 	return -ENOENT;
 }
 
-#define MAX_KEY_LEN 200
-/* 5 bytes for format */
-#define MAX_FMT_LEN (MAX_KEY_LEN + 5 + 1)
-
 static void print_queue_type(struct nlattr *attr, uint16_t max_vqp, uint64_t features)
 {
 	bool is_ctrl = false;
@@ -918,7 +915,6 @@ static void pr_out_dev_net_vstats(const struct nlmsghdr *nlh)
 {
 	const char *name = NULL;
 	uint64_t features = 0;
-	char fmt[MAX_FMT_LEN];
 	uint16_t max_vqp = 0;
 	struct nlattr *attr;
 	uint64_t v64;
@@ -936,15 +932,11 @@ static void pr_out_dev_net_vstats(const struct nlmsghdr *nlh)
 			break;
 		case VDPA_ATTR_DEV_VENDOR_ATTR_NAME:
 			name = mnl_attr_get_str(attr);
-			if (strlen(name) > MAX_KEY_LEN)
-				return;
-
-			strcpy(fmt, name);
-			strcat(fmt, " %lu ");
 			break;
 		case VDPA_ATTR_DEV_VENDOR_ATTR_VALUE:
 			v64 = mnl_attr_get_u64(attr);
-			print_u64(PRINT_ANY, name, fmt, v64);
+			print_string(PRINT_FP, NULL, "%s ", name);
+			print_u64(PRINT_ANY, name, "%" PRIu64 " ", v64);
 			break;
 		}
 	}
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute 2/3] rdma: don't build printf formats out of keys
  2026-10-19 20:03 ` [PATCH iproute 2/3] rdma: don't build printf formats out of keys Stephen Hemminger
@ 2026-10-20 13:10   ` Leon Romanovsky
  0 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2026-10-20 13:10 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

On Mon, Oct 19, 2026 at 01:03:03PM -0700, Stephen Hemminger wrote:
> print_key(), res_print_u32(), res_print_u64(), print_driver_string() and
> print_ipaddr() passed the attribute key as both the json key and the
> printf format, so a '%' in a kernel-supplied driver or hwcounter key was
> taken as a conversion. Pass the key as a string argument with a literal
> format; output is unchanged.

I'm not aware of any real firmware that sets `%` there. However, why not?

Thanks,
Reviewed-by: Leon Romanovsky <leon@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-20 13:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 20:03 [PATCH iproute2 0/3] don't build printf formats out of names Stephen Hemminger
2026-10-19 20:03 ` [PATCH iproute 1/3] lib: " Stephen Hemminger
2026-10-19 20:03 ` [PATCH iproute 2/3] rdma: don't build printf formats out of keys Stephen Hemminger
2026-10-20 13:10   ` Leon Romanovsky
2026-10-19 20:03 ` [PATCH iproute 3/3] vdpa: don't build printf format out of vendor stats name Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.