Re: [PATCH] can: bcm: Replace strcpy() with strscpy(), simplify call
Oliver Hartkopp <[email protected]>
| Newsgroups | org.kernel.vger.kernel-janitors,org.kernel.vger.linux-can,org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hello Ethan,
On 19.04.26 04:00, Ethan Carter Edwards wrote:
> strcpy() is deprecated; use the safer strscpy() instead.
>
> While the src is char array at bcm_proc_getifname's call sites, the src
> is passed to the function as a char*. Including the IFNAMSIZ length
> makes the call safer.
I'm not sure what makes the call "safer" here.
The use of IFNAMSIZ in dev->name and in ifname[IFNAMSIZ] in
bcm_proc_show() makes it 100% clear that we only can copy data between
identical sized buffers. Adding strscpy() with IFNAMSIZ is a nop.
The only issue could be that someone reduces IFNAMSIZ to a value smaller
than sizeof("???") - which is not addressed here.
I see no real benefit changing this code in the proposed way.
Especially as we are not talking about some dynamic content here.
Best regards,
Oliver
>
> Use turnary over if/else to simplify code.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Ethan Carter Edwards <[email protected]>
> ---
> net/can/bcm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index a4bef2c48a55..c133dab1202e 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -196,10 +196,7 @@ static char *bcm_proc_getifname(struct net *net, char *result, int ifindex)
>
> rcu_read_lock();
> dev = dev_get_by_index_rcu(net, ifindex);
> - if (dev)
> - strcpy(result, dev->name);
> - else
> - strcpy(result, "???");
> + strscpy(result, dev ? dev->name : "???", IFNAMSIZ);
> rcu_read_unlock();
>
> return result;
>
> ---
> base-commit: c7275b05bc428c7373d97aa2da02d3a7fa6b9f66
> change-id: 20260418-can-bcm-strscpy-56a0e402a660
>
> Best regards,