| Message ID | 20231023-strncpy-drivers-s390-char-sclp-c-v1-1-eaeef80522bb@google.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | e37988bcd1fddb726cf08e4f04a1b8dc4a2f80aa |
| Headers | show |
| Series | s390/sclp: replace deprecated strncpy with strtomem | expand |
On Mon, Oct 23, 2023 at 07:14:49PM +0000, Justin Stitt wrote: > Let's move away from using strncpy() as it is deprecated [1]. > > Instead use strtomem() as `e.id` is already marked as nonstring: > | char id[4] __nonstring; I'm surprised the compiler didn't complain about this -- I thought that was the point of the __nonstring attribute: to diagnose when used with a string function. Hmpf. > > We don't need strtomem_pad() because `e` is already memset to 0 -- > rendering any additional NUL-padding useless. > > 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@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Note: build-tested only. > > Found with: $ rg "strncpy\(" > --- > drivers/s390/char/sclp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c > index 8f74db689a0c..486cf57a1605 100644 > --- a/drivers/s390/char/sclp.c > +++ b/drivers/s390/char/sclp.c > @@ -81,7 +81,7 @@ static inline void sclp_trace(int prio, char *id, u32 a, u64 b, bool err) > struct sclp_trace_entry e; > > memset(&e, 0, sizeof(e)); > - strncpy(e.id, id, sizeof(e.id)); > + strtomem(e.id, id); Looks right to me; thanks! Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > e.a = a; > e.b = b; > debug_event(&sclp_debug, prio, &e, sizeof(e)); > > --- > base-commit: 9c5d00cb7b6bbc5a7965d9ab7d223b5402d1f02c > change-id: 20231023-strncpy-drivers-s390-char-sclp-c-bb66226a7eaa > > Best regards, > -- > Justin Stitt <justinstitt@google.com> > >
On Tue, Oct 24, 2023 at 04:46:04PM -0700, Kees Cook wrote: > > Instead use strtomem() as `e.id` is already marked as nonstring: > > | char id[4] __nonstring; > > I'm surprised the compiler didn't complain about this -- I thought that > was the point of the __nonstring attribute: to diagnose when used with a > string function. Hmpf. Wow. GCC only warns if you call strlen() on it, but literally nothing else... :(
On Mon, Oct 23, 2023 at 07:14:49PM +0000, Justin Stitt wrote: > Let's move away from using strncpy() as it is deprecated [1]. > > Instead use strtomem() as `e.id` is already marked as nonstring: > | char id[4] __nonstring; > > We don't need strtomem_pad() because `e` is already memset to 0 -- > rendering any additional NUL-padding useless. > > 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@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Note: build-tested only. > > Found with: $ rg "strncpy\(" > --- > drivers/s390/char/sclp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thank you!
diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c index 8f74db689a0c..486cf57a1605 100644 --- a/drivers/s390/char/sclp.c +++ b/drivers/s390/char/sclp.c @@ -81,7 +81,7 @@ static inline void sclp_trace(int prio, char *id, u32 a, u64 b, bool err) struct sclp_trace_entry e; memset(&e, 0, sizeof(e)); - strncpy(e.id, id, sizeof(e.id)); + strtomem(e.id, id); e.a = a; e.b = b; debug_event(&sclp_debug, prio, &e, sizeof(e));
Let's move away from using strncpy() as it is deprecated [1]. Instead use strtomem() as `e.id` is already marked as nonstring: | char id[4] __nonstring; We don't need strtomem_pad() because `e` is already memset to 0 -- rendering any additional NUL-padding useless. 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@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build-tested only. Found with: $ rg "strncpy\(" --- drivers/s390/char/sclp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 9c5d00cb7b6bbc5a7965d9ab7d223b5402d1f02c change-id: 20231023-strncpy-drivers-s390-char-sclp-c-bb66226a7eaa Best regards, -- Justin Stitt <justinstitt@google.com>