Thread (11 messages) 11 messages, 2 authors, 24d ago

Re: [PATCH net 3/3] netconsole: propagate device name truncation in dev_name_store()

From: Breno Leitao <leitao@debian.org>
Date: 2026-04-27 10:26:47
Also in: netdev

On Sun, Apr 26, 2026 at 09:39:05AM +0100, Simon Horman wrote:
From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
netconsole: propagate device name truncation in dev_name_store()

This commit rejects interface name writes that exceed the size of the
destination buffer to avoid silently truncating names and binding to
unintended devices.
quoted
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4bef003d9df64..3914fb90f9afd 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -816,6 +816,9 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf,
 {
 	struct netconsole_target *nt = to_target(item);
 
+	if (count >= IFNAMSIZ)
+		return -ENAMETOOLONG;
+
Does this check incorrectly reject valid maximum-length interface names when
written with a trailing newline?

Standard userspace tools like echo append a trailing newline when writing to
configfs attributes. If userspace writes a 15-character name, the count will
be exactly 16 bytes.

Since IFNAMSIZ is 16, the condition count >= IFNAMSIZ evaluates to true,
causing dev_name_store() to reject the valid input with -ENAMETOOLONG.

Prior to this patch, strscpy() safely truncated the trailing newline by
copying the 15 characters and a NUL terminator.
That is a valid issue, if someone is using a 16-byte ifname, it will fail, for
instance:

echo eth0123456789012 (15 chars) writes 16 bytes including the trailing \n

With IFNAMSIZ=16, a valid 15-character interface name written via echo arrives
as 16 bytes (15 chars + \n), and the count >= IFNAMSIZ check rejects it — a
regression compared to the prior strscpy() + trim_newline() behavior, which
silently dropped the newline.

I think a better approach would be:

  size_t len = count;

  if (len && buf[len - 1] == '\n')
      len--;
  if (len >= IFNAMSIZ)
      return -ENAMETOOLONG;

That keeps the length check consistent with what trim_newline() does to the
stored string.

I will send a v2.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help