diff mbox series

[v2] drm/modes: replace deprecated strncpy with strscpy_pad

Message ID 20231016-strncpy-drivers-gpu-drm-drm_modes-c-v2-1-d0b60686e1c6@google.com (mailing list archive)
State Mainlined
Commit 3b2894c967377a49be084b9b39b21b2315bd9b2c
Headers show
Series [v2] drm/modes: replace deprecated strncpy with strscpy_pad | expand

Commit Message

Justin Stitt Oct. 16, 2023, 10:38 p.m. UTC
`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(-)


---
base-commit: 58720809f52779dc0f08e53e54b014209d13eebb
change-id: 20230914-strncpy-drivers-gpu-drm-drm_modes-c-a35d782cad01

Best regards,
--
Justin Stitt <justinstitt@google.com>

Comments

Kees Cook Oct. 18, 2023, 11:51 p.m. UTC | #1
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 Nov. 30, 2023, 10 p.m. UTC | #2
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,
diff mbox series

Patch

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