[PATCH net-next] net: ipconfig: Replace strncpy with strscpy_pad in ic_proto_name

[PATCH net-next] net: ipconfig: Replace strncpy with strscpy_pad in ic_proto_name

[Thorsten Blum]

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@linux.dev>
---
 net/ipv4/ipconfig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 22a7889876c1..27cc6f8070b7 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -1690,7 +1690,8 @@ static int __init ic_proto_name(char *name)
 			*v = 0;
 			if (kstrtou8(client_id, 0, dhcp_client_identifier))
 				pr_debug("DHCP: Invalid client identifier type\n");
-			strncpy(dhcp_client_identifier + 1, v + 1, 251);
+			strscpy_pad(dhcp_client_identifier + 1, v + 1,
+				    sizeof(dhcp_client_identifier) - 1);
 			*v = ',';
 		}
 		return 1;
-- 
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6  9D84 7336 78FD 8DFE EAD4

Re: [PATCH net-next] net: ipconfig: Replace strncpy with strscpy_pad in ic_proto_name

[david laight]

On Wed, 26 Nov 2025 12:13:58 +0100
Thorsten Blum <thorsten.blum@linux.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@linux.dev>
> ---
>  net/ipv4/ipconfig.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
> index 22a7889876c1..27cc6f8070b7 100644
> --- a/net/ipv4/ipconfig.c
> +++ b/net/ipv4/ipconfig.c
> @@ -1690,7 +1690,8 @@ static int __init ic_proto_name(char *name)
>  			*v = 0;
>  			if (kstrtou8(client_id, 0, dhcp_client_identifier))
>  				pr_debug("DHCP: Invalid client identifier type\n");
> -			strncpy(dhcp_client_identifier + 1, v + 1, 251);
> +			strscpy_pad(dhcp_client_identifier + 1, v + 1,
> +				    sizeof(dhcp_client_identifier) - 1);

Wrong change...
There is no reason to pad the destination, and the correct alternative
is to bound 'v - client_id' and then use memcpy().
Then you don't need to modify the input buffer.

Although you might want to worry about the 'strange' strlen(dhcp_client_identifier + 1)
where the string is used.

	David

>  			*v = ',';
>  		}
>  		return 1;

Re: [PATCH net-next] net: ipconfig: Replace strncpy with strscpy_pad in ic_proto_name

[Thorsten Blum]

On 26. Nov 2025, at 14:50, david laight wrote:
> On Wed, 26 Nov 2025 12:13:58 +0100
> Thorsten Blum <thorsten.blum@linux.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@linux.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?

Thanks,
Thorsten

Re: [PATCH net-next] net: ipconfig: Replace strncpy with strscpy_pad in ic_proto_name

[david laight]

On Wed, 26 Nov 2025 15:45:50 +0100
Thorsten Blum <thorsten.blum@linux.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@linux.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@linux.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
> 
>