linux-kernel - [PATCH net-next v2 1/2] 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: <20241014-string-thing-v2-1-b9b29625060a@kernel.org>
Date: Mon, 14 Oct 2024 09:52:25 +0100
From: Simon Horman <horms@...nel.org>
To: Andrew Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>
Cc: Bill Wendling <morbo@...gle.com>, 
 Daniel Machon <daniel.machon@...rochip.com>, 
 Florian Fainelli <f.fainelli@...il.com>, 
 Jiawen Wu <jiawenwu@...stnetic.com>, Justin Stitt <justinstitt@...gle.com>, 
 Mengyuan Lou <mengyuanlou@...-swift.com>, 
 Nathan Chancellor <nathan@...nel.org>, 
 Nick Desaulniers <ndesaulniers@...gle.com>, 
 Richard Cochran <richardcochran@...il.com>, 
 Vladimir Oltean <olteanv@...il.com>, 
 Woojung Huh <woojung.huh@...rochip.com>, UNGLinuxDriver@...rochip.com, 
 netdev@...r.kernel.org, llvm@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: [PATCH net-next v2 1/2] 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 of fixed size, the 2-argument variant may be used.

Reviewed-by: Daniel Machon <daniel.machon@...rochip.com>
Signed-off-by: Simon Horman <horms@...nel.org>
--
v2
- Add Reviewed-by from Daniel Machon
- Tweaked patch description, thanks to Daniel Machon
---
 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