devlink/param: replace deprecated strcpy() with strscpy()

[PATCH] devlink/param: replace deprecated strcpy() with strscpy()

Posted by Álvaro Costa 1 month, 1 week ago

Replace strcpy() call used to extract a string parameter from param_data
with strscpy(). Since strscpy() already performs bounds checking and
ensures the destination string is NUL-terminated, remove the string
length check as well.

Signed-off-by: Álvaro Costa <alvaroc.dev@gmail.com>
---
 net/devlink/param.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/devlink/param.c b/net/devlink/param.c
index cf95268da5b0..26695b7e2861 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -536,11 +536,9 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
 		value->vu64 = nla_get_u64(param_data);
 		break;
 	case DEVLINK_PARAM_TYPE_STRING:
-		len = strnlen(nla_data(param_data), nla_len(param_data));
-		if (len == nla_len(param_data) ||
-		    len >= __DEVLINK_PARAM_MAX_STRING_VALUE)
+		len = strscpy(value->vstr, nla_data(param_data));
+		if (len < 0)
 			return -EINVAL;
-		strcpy(value->vstr, nla_data(param_data));
 		break;
 	case DEVLINK_PARAM_TYPE_BOOL:
 		if (param_data && nla_len(param_data))
-- 
2.53.0

Re: [PATCH] devlink/param: replace deprecated strcpy() with strscpy()

Posted by David Laight 1 month, 1 week ago

On Wed,  6 May 2026 18:14:11 -0300
Álvaro Costa <alvaroc.dev@gmail.com> wrote:

> Replace strcpy() call used to extract a string parameter from param_data
> with strscpy(). Since strscpy() already performs bounds checking and
> ensures the destination string is NUL-terminated, remove the string
> length check as well.
> 
> Signed-off-by: Álvaro Costa <alvaroc.dev@gmail.com>
> ---
>  net/devlink/param.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/devlink/param.c b/net/devlink/param.c
> index cf95268da5b0..26695b7e2861 100644
> --- a/net/devlink/param.c
> +++ b/net/devlink/param.c
> @@ -536,11 +536,9 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
>  		value->vu64 = nla_get_u64(param_data);
>  		break;
>  	case DEVLINK_PARAM_TYPE_STRING:
> -		len = strnlen(nla_data(param_data), nla_len(param_data));
> -		if (len == nla_len(param_data) ||
> -		    len >= __DEVLINK_PARAM_MAX_STRING_VALUE)
> +		len = strscpy(value->vstr, nla_data(param_data));
> +		if (len < 0)
>  			return -EINVAL;
> -		strcpy(value->vstr, nla_data(param_data));

The only sensible thing here is to replace the strcpy() with:
		memcpy(value->vstr, nla_data(param_data), len + 1);

-- David

>  		break;
>  	case DEVLINK_PARAM_TYPE_BOOL:
>  		if (param_data && nla_len(param_data))

Re: [PATCH] devlink/param: replace deprecated strcpy() with strscpy()

Posted by Jakub Kicinski 1 month ago

On Thu, 7 May 2026 09:04:45 +0100 David Laight wrote:
> >  	case DEVLINK_PARAM_TYPE_STRING:
> > -		len = strnlen(nla_data(param_data), nla_len(param_data));
> > -		if (len == nla_len(param_data) ||
> > -		    len >= __DEVLINK_PARAM_MAX_STRING_VALUE)
> > +		len = strscpy(value->vstr, nla_data(param_data));
> > +		if (len < 0)
> >  			return -EINVAL;
> > -		strcpy(value->vstr, nla_data(param_data));  
> 
> The only sensible thing here is to replace the strcpy() with:
> 		memcpy(value->vstr, nla_data(param_data), len + 1);

That'd probably be a good move, to avoid getting the broken strscpy()
conversion submissions. Care to send a patch?

Re: [PATCH] devlink/param: replace deprecated strcpy() with strscpy()

Posted by David Laight 1 month ago

On Thu, 7 May 2026 07:52:34 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 7 May 2026 09:04:45 +0100 David Laight wrote:
> > >  	case DEVLINK_PARAM_TYPE_STRING:
> > > -		len = strnlen(nla_data(param_data), nla_len(param_data));
> > > -		if (len == nla_len(param_data) ||
> > > -		    len >= __DEVLINK_PARAM_MAX_STRING_VALUE)
> > > +		len = strscpy(value->vstr, nla_data(param_data));
> > > +		if (len < 0)
> > >  			return -EINVAL;
> > > -		strcpy(value->vstr, nla_data(param_data));    
> > 
> > The only sensible thing here is to replace the strcpy() with:
> > 		memcpy(value->vstr, nla_data(param_data), len + 1);  
> 
> That'd probably be a good move, to avoid getting the broken strscpy()
> conversion submissions. Care to send a patch?

I'll lob one in soon.

I'm working my way through a compile that errors strcpy() except for
constant strings into arrays (ie the safe ones).
Why is it that the worst examples is in the 'secerity code' :-)

-- David

Re: [PATCH] devlink/param: replace deprecated strcpy() with strscpy()

Posted by Jakub Kicinski 1 month, 1 week ago

On Wed,  6 May 2026 18:14:11 -0300 Álvaro Costa wrote:
> Replace strcpy() call used to extract a string parameter from param_data
> with strscpy(). Since strscpy() already performs bounds checking and
> ensures the destination string is NUL-terminated, remove the string
> length check as well.

The current code validates the netlink attr is nul-terminated.
I'm not sure if your patch retains current behavior.
I am sure, however, that checking whether the change is correct
is a poor use of everyone's time.
Please don't send these sort of patches for networking.
-- 
pw-bot: reject

Re: [PATCH] devlink/param: replace deprecated strcpy() with strscpy()

Posted by Álvaro Costa 1 month, 1 week ago

On Wed, May 6, 2026 at 7:42 PM Jakub Kicinski <kuba@kernel.org> wrote:
> The current code validates the netlink attr is nul-terminated.
> I'm not sure if your patch retains current behavior.
> I am sure, however, that checking whether the change is correct
> is a poor use of everyone's time.
> Please don't send these sort of patches for networking.
> --
> pw-bot: reject

Ok, thank you for the quick feedback!