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

[Breno Leitao <leitao@debian.org>]

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.
> 
> > 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.