| Message ID | 20230911-strncpy-drivers-auxdisplay-panel-c-v1-1-b60bd0ae8552@google.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | a952abcdaa22116d940ca9cb9253caad1622ae93 |
| Headers | show |
| Series | auxdisplay: panel: refactor deprecated strncpy | expand |
On Mon, Sep 11, 2023 at 08:51:04PM +0000, Justin Stitt wrote: > `strncpy` is deprecated and as such we should prefer more robust and > less ambiguous interfaces. > > In this case, all of `press_str`, `repeat_str` and `release_str` are > explicitly marked as nonstring: > | struct { /* valid when type == INPUT_TYPE_KBD */ > | char press_str[sizeof(void *) + sizeof(int)] __nonstring; > | char repeat_str[sizeof(void *) + sizeof(int)] __nonstring; > | char release_str[sizeof(void *) + sizeof(int)] __nonstring; > | } kbd; > > ... which makes `strtomem_pad` a suitable replacement as it is > functionally the same whilst being more obvious about its behavior. Yup, this is exactly what strtomem_pad() was made for. :) Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > > 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 > Cc: Kees Cook <keescook@chromium.org> > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Note: build-tested > --- > drivers/auxdisplay/panel.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c > index eba04c0de7eb..e20d35bdf5fe 100644 > --- a/drivers/auxdisplay/panel.c > +++ b/drivers/auxdisplay/panel.c > @@ -1449,10 +1449,9 @@ static struct logical_input *panel_bind_key(const char *name, const char *press, > key->rise_time = 1; > key->fall_time = 1; > > - strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str)); > - strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str)); > - strncpy(key->u.kbd.release_str, release, > - sizeof(key->u.kbd.release_str)); > + strtomem_pad(key->u.kbd.press_str, press, '\0'); > + strtomem_pad(key->u.kbd.repeat_str, repeat, '\0'); > + strtomem_pad(key->u.kbd.release_str, release, '\0'); > list_add(&key->list, &logical_inputs); > return key; > } > > --- > base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c > change-id: 20230911-strncpy-drivers-auxdisplay-panel-c-83bce51f32cb > > Best regards, > -- > Justin Stitt <justinstitt@google.com> >
On Mon, 11 Sep 2023 20:51:04 +0000, Justin Stitt wrote: > `strncpy` is deprecated and as such we should prefer more robust and > less ambiguous interfaces. > > In this case, all of `press_str`, `repeat_str` and `release_str` are > explicitly marked as nonstring: > | struct { /* valid when type == INPUT_TYPE_KBD */ > | char press_str[sizeof(void *) + sizeof(int)] __nonstring; > | char repeat_str[sizeof(void *) + sizeof(int)] __nonstring; > | char release_str[sizeof(void *) + sizeof(int)] __nonstring; > | } kbd; > > [...] Applied to for-next/hardening, thanks! [1/1] auxdisplay: panel: refactor deprecated strncpy https://git.kernel.org/kees/c/0456f788873d Take care,
On Fri, Sep 29, 2023 at 8:42 PM Kees Cook <keescook@chromium.org> wrote: > > Applied to for-next/hardening, thanks! Thanks for picking it up! Cheers, Miguel
diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c index eba04c0de7eb..e20d35bdf5fe 100644 --- a/drivers/auxdisplay/panel.c +++ b/drivers/auxdisplay/panel.c @@ -1449,10 +1449,9 @@ static struct logical_input *panel_bind_key(const char *name, const char *press, key->rise_time = 1; key->fall_time = 1; - strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str)); - strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str)); - strncpy(key->u.kbd.release_str, release, - sizeof(key->u.kbd.release_str)); + strtomem_pad(key->u.kbd.press_str, press, '\0'); + strtomem_pad(key->u.kbd.repeat_str, repeat, '\0'); + strtomem_pad(key->u.kbd.release_str, release, '\0'); list_add(&key->list, &logical_inputs); return key; }
`strncpy` is deprecated and as such we should prefer more robust and less ambiguous interfaces. In this case, all of `press_str`, `repeat_str` and `release_str` are explicitly marked as nonstring: | struct { /* valid when type == INPUT_TYPE_KBD */ | char press_str[sizeof(void *) + sizeof(int)] __nonstring; | char repeat_str[sizeof(void *) + sizeof(int)] __nonstring; | char release_str[sizeof(void *) + sizeof(int)] __nonstring; | } kbd; ... which makes `strtomem_pad` a suitable replacement as it is functionally the same whilst being more obvious about its behavior. 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 Cc: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build-tested --- drivers/auxdisplay/panel.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) --- base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c change-id: 20230911-strncpy-drivers-auxdisplay-panel-c-83bce51f32cb Best regards, -- Justin Stitt <justinstitt@google.com>