[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231016-strncpy-drivers-net-hamradio-baycom_epp-c-v2-1-39f72a72de30@google.com>
Date: Mon, 16 Oct 2023 18:42:42 +0000
From: Justin Stitt <justinstitt@...gle.com>
To: Thomas Sailer <t.sailer@...mni.ethz.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: linux-hams@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
Justin Stitt <justinstitt@...gle.com>
Subject: [PATCH v2] hamradio: replace deprecated strncpy with strscpy_pad
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
We expect both hi.data.modename and hi.data.drivername to be
NUL-terminated based on its usage with sprintf:
| sprintf(hi.data.modename, "%sclk,%smodem,fclk=%d,bps=%d%s",
| bc->cfg.intclk ? "int" : "ext",
| bc->cfg.extmodem ? "ext" : "int", bc->cfg.fclk, bc->cfg.bps,
| bc->cfg.loopback ? ",loopback" : "");
Note that this data is copied out to userspace with:
| if (copy_to_user(data, &hi, sizeof(hi)))
... however, the data was also copied FROM the user here:
| if (copy_from_user(&hi, data, sizeof(hi)))
Considering the above, a suitable replacement is strscpy_pad() as it
guarantees NUL-termination on the destination buffer while also
NUL-padding (which is good+wanted behavior when copying data to
userspace).
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@...r.kernel.org
Signed-off-by: Justin Stitt <justinstitt@...gle.com>
---
Changes in v2:
- use strscpy_pad (thanks Kees)
- bytes arrays are now considered "buffets" :) (thanks Kees)
- Link to v1: https://lore.kernel.org/r/20231012-strncpy-drivers-net-hamradio-baycom_epp-c-v1-1-8f4097538ee4@google.com
---
Note: build-tested only.
Also, there are 33 instances of trailing whitespace in this file alone.
I've opted to not remove them in this patch.
---
drivers/net/hamradio/baycom_epp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hamradio/baycom_epp.c b/drivers/net/hamradio/baycom_epp.c
index 83ff882f5d97..ccfc83857c26 100644
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -1074,7 +1074,7 @@ static int baycom_siocdevprivate(struct net_device *dev, struct ifreq *ifr,
return 0;
case HDLCDRVCTL_DRIVERNAME:
- strncpy(hi.data.drivername, "baycom_epp", sizeof(hi.data.drivername));
+ strscpy_pad(hi.data.drivername, "baycom_epp", sizeof(hi.data.drivername));
break;
case HDLCDRVCTL_GETMODE:
@@ -1091,7 +1091,7 @@ static int baycom_siocdevprivate(struct net_device *dev, struct ifreq *ifr,
return baycom_setmode(bc, hi.data.modename);
case HDLCDRVCTL_MODELIST:
- strncpy(hi.data.modename, "intclk,extclk,intmodem,extmodem,divider=x",
+ strscpy_pad(hi.data.modename, "intclk,extclk,intmodem,extmodem,divider=x",
sizeof(hi.data.modename));
break;
---
base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
change-id: 20231012-strncpy-drivers-net-hamradio-baycom_epp-c-6e11c9483b9f
Best regards,
--
Justin Stitt <justinstitt@...gle.com>
Powered by blists - more mailing lists