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

[Simon Horman]

On Mon, Apr 27, 2026 at 03:26:29AM -0700, Breno Leitao wrote:
> On Sun, Apr 26, 2026 at 09:39:05AM +0100, Simon Horman wrote:
> > From: 'Simon Horman' <horms@xxxxxxxxxx>
> > 
> > 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.

Thanks. The approach above looks good to me.