| Message ID | 20231012-strncpy-drivers-net-mdio-mdio-gpio-c-v1-1-ab9b06cfcdab@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: mdio: replace deprecated strncpy with strscpy | expand |
On Thu, Oct 12, 2023 at 09:43:02PM +0000, Justin Stitt wrote: > 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 new_bus->id to be NUL-terminated but not NUL-padded based on > its prior assignment through snprintf: > | snprintf(new_bus->id, MII_BUS_ID_SIZE, "gpio-%x", bus_id); > > Due to this, a suitable replacement is `strscpy` [2] due to the fact > that it guarantees NUL-termination on the destination buffer without > unnecessarily NUL-padding. > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Thu, Oct 12, 2023 at 09:43:02PM +0000, Justin Stitt wrote: > 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 new_bus->id to be NUL-terminated but not NUL-padded based on > its prior assignment through snprintf: > | snprintf(new_bus->id, MII_BUS_ID_SIZE, "gpio-%x", bus_id); > > Due to this, a suitable replacement is `strscpy` [2] due to the fact > that it guarantees NUL-termination on the destination buffer without > unnecessarily NUL-padding. > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Note: build-tested only. > > Found with: $ rg "strncpy\(" > --- > drivers/net/mdio/mdio-gpio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/mdio/mdio-gpio.c b/drivers/net/mdio/mdio-gpio.c > index 0fb3c2de0845..a1718d646504 100644 > --- a/drivers/net/mdio/mdio-gpio.c > +++ b/drivers/net/mdio/mdio-gpio.c > @@ -125,7 +125,7 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev, > if (bus_id != -1) > snprintf(new_bus->id, MII_BUS_ID_SIZE, "gpio-%x", bus_id); > else > - strncpy(new_bus->id, "gpio", MII_BUS_ID_SIZE); > + strscpy(new_bus->id, "gpio", sizeof(new_bus->id)); struct mii_bus { ... char id[MII_BUS_ID_SIZE]; Yup, looks good. (I wonder about changing to sizeof() in the snprintf() above it, but for a strscpy() refactor, I think this is fine.) Reviewed-by: Kees Cook <keescook@chromium.org>
On Thu, 12 Oct 2023 21:43:02 +0000, Justin Stitt wrote: > 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 new_bus->id to be NUL-terminated but not NUL-padded based on > its prior assignment through snprintf: > | snprintf(new_bus->id, MII_BUS_ID_SIZE, "gpio-%x", bus_id); > > [...] Applied to for-next/hardening, thanks! [1/1] net: mdio: replace deprecated strncpy with strscpy https://git.kernel.org/kees/c/3247bb945786 Take care,
On Thu, 30 Nov 2023 14:00:33 -0800 Kees Cook wrote: > Applied to for-next/hardening, thanks! > > [1/1] net: mdio: replace deprecated strncpy with strscpy > https://git.kernel.org/kees/c/3247bb945786 newer version of this was posted...
On Thu, Nov 30, 2023 at 10:43:34PM -0800, Jakub Kicinski wrote: > On Thu, 30 Nov 2023 14:00:33 -0800 Kees Cook wrote: > > Applied to for-next/hardening, thanks! > > > > [1/1] net: mdio: replace deprecated strncpy with strscpy > > https://git.kernel.org/kees/c/3247bb945786 > > newer version of this was posted... Hm, I didn't see anything land for this for the other with the same subject. I've dropped both from my tree now. Justin, can you chase down the mdio patches?
diff --git a/drivers/net/mdio/mdio-gpio.c b/drivers/net/mdio/mdio-gpio.c index 0fb3c2de0845..a1718d646504 100644 --- a/drivers/net/mdio/mdio-gpio.c +++ b/drivers/net/mdio/mdio-gpio.c @@ -125,7 +125,7 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev, if (bus_id != -1) snprintf(new_bus->id, MII_BUS_ID_SIZE, "gpio-%x", bus_id); else - strncpy(new_bus->id, "gpio", MII_BUS_ID_SIZE); + strscpy(new_bus->id, "gpio", sizeof(new_bus->id)); if (pdata) { new_bus->phy_mask = pdata->phy_mask;
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 new_bus->id to be NUL-terminated but not NUL-padded based on its prior assignment through snprintf: | snprintf(new_bus->id, MII_BUS_ID_SIZE, "gpio-%x", bus_id); Due to this, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build-tested only. Found with: $ rg "strncpy\(" --- drivers/net/mdio/mdio-gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2 change-id: 20231012-strncpy-drivers-net-mdio-mdio-gpio-c-bddd9ed0c630 Best regards, -- Justin Stitt <justinstitt@google.com>