* [PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad
@ 2023-10-16 22:38 Justin Stitt
2023-10-18 23:51 ` Kees Cook
2023-11-30 22:00 ` Kees Cook
0 siblings, 2 replies; 3+ messages in thread
From: Justin Stitt @ 2023-10-16 22:38 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter
Cc: dri-devel, linux-kernel, linux-hardening, Xu Panda, Justin Stitt
`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 should NUL-pad as there are full struct copies happening in places:
| struct drm_mode_modeinfo umode;
|
| ...
| struct drm_property_blob *blob;
|
| drm_mode_convert_to_umode(&umode, mode);
| blob = drm_property_create_blob(crtc->dev,
| sizeof(umode), &umode);
A suitable replacement is `strscpy_pad` due to the fact that it
guarantees both NUL-termination and NUL-padding on the destination
buffer.
Additionally, replace size macro `DRM_DISPLAY_MODE_LEN` with sizeof() to
more directly tie the maximum buffer size to the destination buffer:
| struct drm_display_mode {
| ...
| char name[DRM_DISPLAY_MODE_LEN];
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: Xu Panda <xu.panda@zte.com.cn>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Changes in v2:
- use strscpy_pad (thanks Kees)
- rebase onto mainline
- Link to v1: https://lore.kernel.org/r/20230914-strncpy-drivers-gpu-drm-drm_modes-c-v1-1-079b532553a3@google.com
---
Note: build-tested only.
---
drivers/gpu/drm/drm_modes.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..893f52ee4926 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2617,8 +2617,7 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
break;
}
- strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
- out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ strscpy_pad(out->name, in->name, sizeof(out->name));
}
/**
@@ -2659,8 +2658,7 @@ int drm_mode_convert_umode(struct drm_device *dev,
* useful for the kernel->userspace direction anyway.
*/
out->type = in->type & DRM_MODE_TYPE_ALL;
- strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
- out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ strscpy_pad(out->name, in->name, sizeof(out->name));
/* Clearing picture aspect ratio bits from out flags,
* as the aspect-ratio information is not stored in
---
base-commit: 58720809f52779dc0f08e53e54b014209d13eebb
change-id: 20230914-strncpy-drivers-gpu-drm-drm_modes-c-a35d782cad01
Best regards,
--
Justin Stitt <justinstitt@google.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad
2023-10-16 22:38 [PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad Justin Stitt
@ 2023-10-18 23:51 ` Kees Cook
2023-11-30 22:00 ` Kees Cook
1 sibling, 0 replies; 3+ messages in thread
From: Kees Cook @ 2023-10-18 23:51 UTC (permalink / raw)
To: Justin Stitt
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel, linux-kernel,
linux-hardening, Xu Panda
On Mon, Oct 16, 2023 at 10:38:20PM +0000, 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 should NUL-pad as there are full struct copies happening in places:
> | struct drm_mode_modeinfo umode;
> |
> | ...
> | struct drm_property_blob *blob;
> |
> | drm_mode_convert_to_umode(&umode, mode);
> | blob = drm_property_create_blob(crtc->dev,
> | sizeof(umode), &umode);
>
> A suitable replacement is `strscpy_pad` due to the fact that it
> guarantees both NUL-termination and NUL-padding on the destination
> buffer.
>
> Additionally, replace size macro `DRM_DISPLAY_MODE_LEN` with sizeof() to
> more directly tie the maximum buffer size to the destination buffer:
> | struct drm_display_mode {
> | ...
> | char name[DRM_DISPLAY_MODE_LEN];
>
> 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: Xu Panda <xu.panda@zte.com.cn>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
Thanks for the respin; this looks good to me.
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad
2023-10-16 22:38 [PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad Justin Stitt
2023-10-18 23:51 ` Kees Cook
@ 2023-11-30 22:00 ` Kees Cook
1 sibling, 0 replies; 3+ messages in thread
From: Kees Cook @ 2023-11-30 22:00 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Justin Stitt
Cc: Kees Cook, dri-devel, linux-kernel, linux-hardening, Xu Panda
On Mon, 16 Oct 2023 22:38:20 +0000, 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 should NUL-pad as there are full struct copies happening in places:
> | struct drm_mode_modeinfo umode;
> |
> | ...
> | struct drm_property_blob *blob;
> |
> | drm_mode_convert_to_umode(&umode, mode);
> | blob = drm_property_create_blob(crtc->dev,
> | sizeof(umode), &umode);
>
> [...]
Applied to for-next/hardening, thanks!
[1/1] drm/modes: replace deprecated strncpy with strscpy_pad
https://git.kernel.org/kees/c/d8d273c595db
Take care,
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-11-30 22:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-16 22:38 [PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad Justin Stitt
2023-10-18 23:51 ` Kees Cook
2023-11-30 22:00 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).