| Message ID | 20231013-strncpy-drivers-net-wireless-ath-ath5k-led-c-v1-1-3acb0b5a21f2@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 24709752bfe8bc0147c9379a6ec8fe8d75874066 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath5k: replace deprecated strncpy with strscpy | expand |
On 13. 10. 23, 22:53, 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 led->name to be NUL-terminated based on the presence of a > manual NUL-byte assignment. > > This NUL-byte assignment was added in Commit daf9669bea30aa22 ("ath5k: > ensure led name is null terminated"). If strscpy() had existed and had > been used back when this code was written then potential bugs and the > need to manually NUL-terminate could have been avoided. Since we now > have the technology, let's use it :) > > Considering the above, a suitable replacement is `strscpy` [2] due to > the fact that it guarantees NUL-termination on the destination buffer > without unnecessarily NUL-padding. If NUL-padding is required let's opt > for strscpy_pad(). > > 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> LGTM Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Justin Stitt <justinstitt@google.com> 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 led->name to be NUL-terminated based on the presence of a > manual NUL-byte assignment. > > This NUL-byte assignment was added in Commit daf9669bea30aa22 ("ath5k: > ensure led name is null terminated"). If strscpy() had existed and had > been used back when this code was written then potential bugs and the > need to manually NUL-terminate could have been avoided. Since we now > have the technology, let's use it :) > > Considering the above, a suitable replacement is `strscpy` [2] due to > the fact that it guarantees NUL-termination on the destination buffer > without unnecessarily NUL-padding. If NUL-padding is required let's opt > for strscpy_pad(). > > 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> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. 24709752bfe8 wifi: ath5k: replace deprecated strncpy with strscpy
diff --git a/drivers/net/wireless/ath/ath5k/led.c b/drivers/net/wireless/ath/ath5k/led.c index 33e9928af363..439052984796 100644 --- a/drivers/net/wireless/ath/ath5k/led.c +++ b/drivers/net/wireless/ath/ath5k/led.c @@ -131,8 +131,7 @@ ath5k_register_led(struct ath5k_hw *ah, struct ath5k_led *led, int err; led->ah = ah; - strncpy(led->name, name, sizeof(led->name)); - led->name[sizeof(led->name)-1] = 0; + strscpy(led->name, name, sizeof(led->name)); led->led_dev.name = led->name; led->led_dev.default_trigger = trigger; led->led_dev.brightness_set = ath5k_led_brightness_set;
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 led->name to be NUL-terminated based on the presence of a manual NUL-byte assignment. This NUL-byte assignment was added in Commit daf9669bea30aa22 ("ath5k: ensure led name is null terminated"). If strscpy() had existed and had been used back when this code was written then potential bugs and the need to manually NUL-terminate could have been avoided. Since we now have the technology, let's use it :) Considering the above, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. If NUL-padding is required let's opt for strscpy_pad(). 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/wireless/ath/ath5k/led.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2 change-id: 20231013-strncpy-drivers-net-wireless-ath-ath5k-led-c-12487cee93be Best regards, -- Justin Stitt <justinstitt@google.com>