* [PATCH] ieee802154: Replace strlcpy with strscpy
@ 2023-06-13 0:33 Azeem Shaikh
2023-06-13 19:25 ` Kees Cook
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Azeem Shaikh @ 2023-06-13 0:33 UTC (permalink / raw)
To: Alexander Aring, Stefan Schmidt, Miquel Raynal
Cc: linux-hardening, Azeem Shaikh, linux-wpan, linux-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
Direct replacement is safe here since the return values
from the helper macros are ignored by the callers.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89
Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
---
net/ieee802154/trace.h | 2 +-
net/mac802154/trace.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
index e5d8439b9e45..c16db0b326fa 100644
--- a/net/ieee802154/trace.h
+++ b/net/ieee802154/trace.h
@@ -13,7 +13,7 @@
#define MAXNAME 32
#define WPAN_PHY_ENTRY __array(char, wpan_phy_name, MAXNAME)
-#define WPAN_PHY_ASSIGN strlcpy(__entry->wpan_phy_name, \
+#define WPAN_PHY_ASSIGN strscpy(__entry->wpan_phy_name, \
wpan_phy_name(wpan_phy), \
MAXNAME)
#define WPAN_PHY_PR_FMT "%s"
diff --git a/net/mac802154/trace.h b/net/mac802154/trace.h
index 689396d6c76a..1574ecc48075 100644
--- a/net/mac802154/trace.h
+++ b/net/mac802154/trace.h
@@ -14,7 +14,7 @@
#define MAXNAME 32
#define LOCAL_ENTRY __array(char, wpan_phy_name, MAXNAME)
-#define LOCAL_ASSIGN strlcpy(__entry->wpan_phy_name, \
+#define LOCAL_ASSIGN strscpy(__entry->wpan_phy_name, \
wpan_phy_name(local->hw.phy), MAXNAME)
#define LOCAL_PR_FMT "%s"
#define LOCAL_PR_ARG __entry->wpan_phy_name
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ieee802154: Replace strlcpy with strscpy
2023-06-13 0:33 [PATCH] ieee802154: Replace strlcpy with strscpy Azeem Shaikh
@ 2023-06-13 19:25 ` Kees Cook
2023-06-14 12:09 ` Simon Horman
2023-06-16 21:32 ` Stefan Schmidt
2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2023-06-13 19:25 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Alexander Aring, Stefan Schmidt, Miquel Raynal, linux-hardening,
linux-wpan, linux-kernel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev
On Tue, Jun 13, 2023 at 12:33:25AM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
>
> Direct replacement is safe here since the return values
> from the helper macros are ignored by the callers.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ieee802154: Replace strlcpy with strscpy
2023-06-13 0:33 [PATCH] ieee802154: Replace strlcpy with strscpy Azeem Shaikh
2023-06-13 19:25 ` Kees Cook
@ 2023-06-14 12:09 ` Simon Horman
2023-06-16 21:32 ` Stefan Schmidt
2 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2023-06-14 12:09 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Alexander Aring, Stefan Schmidt, Miquel Raynal, linux-hardening,
linux-wpan, linux-kernel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev
On Tue, Jun 13, 2023 at 12:33:25AM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
>
> Direct replacement is safe here since the return values
> from the helper macros are ignored by the callers.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ieee802154: Replace strlcpy with strscpy
2023-06-13 0:33 [PATCH] ieee802154: Replace strlcpy with strscpy Azeem Shaikh
2023-06-13 19:25 ` Kees Cook
2023-06-14 12:09 ` Simon Horman
@ 2023-06-16 21:32 ` Stefan Schmidt
2 siblings, 0 replies; 4+ messages in thread
From: Stefan Schmidt @ 2023-06-16 21:32 UTC (permalink / raw)
To: Azeem Shaikh, Alexander Aring, Miquel Raynal
Cc: linux-hardening, linux-wpan, linux-kernel, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev
Hello.
On 13.06.23 02:33, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
>
> Direct replacement is safe here since the return values
> from the helper macros are ignored by the callers.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> ---
> net/ieee802154/trace.h | 2 +-
> net/mac802154/trace.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
> index e5d8439b9e45..c16db0b326fa 100644
> --- a/net/ieee802154/trace.h
> +++ b/net/ieee802154/trace.h
> @@ -13,7 +13,7 @@
>
> #define MAXNAME 32
> #define WPAN_PHY_ENTRY __array(char, wpan_phy_name, MAXNAME)
> -#define WPAN_PHY_ASSIGN strlcpy(__entry->wpan_phy_name, \
> +#define WPAN_PHY_ASSIGN strscpy(__entry->wpan_phy_name, \
> wpan_phy_name(wpan_phy), \
> MAXNAME)
> #define WPAN_PHY_PR_FMT "%s"
> diff --git a/net/mac802154/trace.h b/net/mac802154/trace.h
> index 689396d6c76a..1574ecc48075 100644
> --- a/net/mac802154/trace.h
> +++ b/net/mac802154/trace.h
> @@ -14,7 +14,7 @@
>
> #define MAXNAME 32
> #define LOCAL_ENTRY __array(char, wpan_phy_name, MAXNAME)
> -#define LOCAL_ASSIGN strlcpy(__entry->wpan_phy_name, \
> +#define LOCAL_ASSIGN strscpy(__entry->wpan_phy_name, \
> wpan_phy_name(local->hw.phy), MAXNAME)
> #define LOCAL_PR_FMT "%s"
> #define LOCAL_PR_ARG __entry->wpan_phy_name
This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-16 21:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-13 0:33 [PATCH] ieee802154: Replace strlcpy with strscpy Azeem Shaikh
2023-06-13 19:25 ` Kees Cook
2023-06-14 12:09 ` Simon Horman
2023-06-16 21:32 ` Stefan Schmidt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.