| Message ID | 20230613003326.3538391-1-azeemshaikh38@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ieee802154: Replace strlcpy with strscpy | expand |
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>
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>
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
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
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(-)