| Message ID | 20241011-string-thing-v1-1-acc506568033@kernel.org |
|---|---|
| State | Accepted |
| Commit | 26919411acfafe35365bbb02205aa8484b2d2726 |
| Headers | show |
| Series | [net-next,1/3] net: dsa: microchip: copy string using strscpy | expand |
> 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@kernel.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@microchip.com>
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)
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. Signed-off-by: Simon Horman <horms@kernel.org> --- drivers/net/dsa/microchip/ksz_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)