linux-kernel - Re: [PATCH net-next] net: ipconfig: Replace strncpy with strscpy_pad in ic_proto

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251126184415.28f7fdc2@pumpkin>
Date: Wed, 26 Nov 2025 18:44:15 +0000
From: david laight <david.laight@...box.com>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: "David S. Miller" <davem@...emloft.net>, David Ahern
 <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
 <horms@...nel.org>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net: ipconfig: Replace strncpy with
 strscpy_pad in ic_proto_name

On Wed, 26 Nov 2025 15:45:50 +0100
Thorsten Blum <thorsten.blum@...ux.dev> wrote:

> On 26. Nov 2025, at 14:50, david laight wrote:
> > On Wed, 26 Nov 2025 12:13:58 +0100
> > Thorsten Blum <thorsten.blum@...ux.dev> wrote:
> >   
> >> strncpy() is deprecated [1] for NUL-terminated destination buffers since
> >> it does not guarantee NUL termination. Replace it with strscpy_pad() to
> >> ensure NUL termination of the destination buffer while retaining the
> >> NUL-padding behavior of strncpy().
> >> 
> >> Even though the identifier buffer has 252 usable bytes, strncpy()
> >> intentionally copied only 251 bytes into the zero-initialized buffer,
> >> implicitly relying on the last byte to act as the terminator. Switching
> >> to strscpy_pad() removes the need for this trick and avoids using magic
> >> numbers.
> >> 
> >> The source string is also NUL-terminated and satisfies the
> >> __must_be_cstr() requirement of strscpy_pad().
> >> 
> >> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> >> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> >> ---
> >> [...]  
> > 
> > Wrong change...
> > There is no reason to pad the destination, and the correct alternative  
> 
> I agree, padding isn't necessary and strscpy() is enough.
> 
> > is to bound 'v - client_id' and then use memcpy().
> > Then you don't need to modify the input buffer.  
> 
> Just to confirm - this comment is about the type parsing ('client_id'
> before the comma), not about copying the value after the comma, right?

I've misread the code :-(
Probably because it should be:
static struct {
	u8	type;
	char	value[252]
} dhcp_client_identifier;

There are also better conversion routines than kstrtou8().
You really want one that returns the address of the failing character.
Then the parsing would be easier to follow and wouldn't need to write
to the buffer.

	David

> 
> Thanks,
> Thorsten
> 
>