netdev - Re: [PATCH net-next 1/3] net: dsa: microchip: copy string using strscpy

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241013095231.qoy3aa5zvrftezso@DEN-DL-M70577>
Date: Sun, 13 Oct 2024 09:52:31 +0000
From: Daniel Machon <daniel.machon@...rochip.com>
To: Simon Horman <horms@...nel.org>
CC: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Woojung Huh <woojung.huh@...rochip.com>, Andrew Lunn
	<andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean
	<olteanv@...il.com>, Richard Cochran <richardcochran@...il.com>, Jiawen Wu
	<jiawenwu@...stnetic.com>, Mengyuan Lou <mengyuanlou@...-swift.com>, "Nathan
 Chancellor" <nathan@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>,
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
	Jeffrey Hugo <quic_jhugo@...cinc.com>, Carl Vanderlip
	<quic_carlv@...cinc.com>, Oded Gabbay <ogabbay@...nel.org>,
	<UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org>,
	<llvm@...ts.linux.dev>, <linux-arm-msm@...r.kernel.org>,
	<dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH net-next 1/3] net: dsa: microchip: copy string using
 strscpy

> Prior to this patch ksz_ptp_msg_irq_setup() uses snprintf() to copy
> strings. It does so by passing strings as the format argument of
> snprintf(). This appears to be safe, due to the absence of format
> specifiers in the strings, which are declared within the same function.
> But nonetheless GCC 14 warns about it:
> 
> .../ksz_ptp.c:1109:55: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
>  1109 |         snprintf(ptpmsg_irq->name, sizeof(ptpmsg_irq->name), name[n]);
>       |                                                              ^~~~~~~
> .../ksz_ptp.c:1109:55: note: treat the string as an argument to avoid this
>  1109 |         snprintf(ptpmsg_irq->name, sizeof(ptpmsg_irq->name), name[n]);
>       |                                                              ^
>       |                                                              "%s",
> 
> As what we are really dealing with here is a string copy, it seems make
> sense to use a function designed for this purpose. In this case null
> padding is not required, so strscpy is appropriate. And as the
> destination is an array, the 2-argument variant may be used.

.. is an array - and of fixed size.

> 
> Signed-off-by: Simon Horman <horms@...nel.org>
> ---
>  drivers/net/dsa/microchip/ksz_ptp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 050f17c43ef6..22fb9ef4645c 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -1106,7 +1106,7 @@ static int ksz_ptp_msg_irq_setup(struct ksz_port *port, u8 n)
>         ptpmsg_irq->port = port;
>         ptpmsg_irq->ts_reg = ops->get_port_addr(port->num, ts_reg[n]);
> 
> -       snprintf(ptpmsg_irq->name, sizeof(ptpmsg_irq->name), name[n]);
> +       strscpy(ptpmsg_irq->name, name[n]);
> 
>         ptpmsg_irq->num = irq_find_mapping(port->ptpirq.domain, n);
>         if (ptpmsg_irq->num < 0)
> 
> --
> 2.45.2
>

This looks good to me.

Reviewed-by: Daniel Machon <daniel.machon@...rochip.com>