aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>2021-07-31 02:29:43 +0300
committerLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>2021-10-07 05:42:13 +0300
commit753f2674ad8db265986869ca07863758015deebf (patch)
treec791c331dc04161e432b0ed747a09334d7f3c763
parentdrm: rcar-du: Allow importing non-contiguous dma-buf with VSP (diff)
downloadlinux-dev-753f2674ad8db265986869ca07863758015deebf.tar.xz
linux-dev-753f2674ad8db265986869ca07863758015deebf.zip
drm: property: Replace strncpy() with strscpy_pad()
strncpy() is widely regarded as unsafe due to the fact that it may leave the destination string without a nul-termination when the source string size is too large. When compiling the kernel with W=1, the gcc warns about this: drivers/gpu/drm/drm_property.c: In function ‘drm_property_create’: drivers/gpu/drm/drm_property.c:130:2: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] 130 | strncpy(property->name, name, DRM_PROP_NAME_LEN); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ There are three occurrences of strncpy() in drm_property.c. None of them are actually unsafe, as the very next line forces nul-termination of the destination buffer. The warning is thus a false positive, but adds noise to the kernel log. It can easily be silenced by using strscpy_pad() instead. Do so. One of the three occurrences, in drm_property_add_enum(), fills a char array that is later copied to userspace with copy_to_user() in drm_mode_getproperty_ioctl(). To avoid leaking kernel data, strscpy_pad() is required. Similarly, a second occurrence, in drm_mode_getproperty_ioctl(), copies the string to an ioctl data buffer that isn't previously zero'ed, to strscpy_pad() is also required. The last occurrence, in drm_property_create(), would be safe to replace with strscpy(), as the destination buffer is copied to userspace with strscpy_pad(). However, given that this isn't in a hot path, let's avoid future data leaks in case someone copies the whole char array blindly. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-rw-r--r--drivers/gpu/drm/drm_property.c9
1 files changed, 3 insertions, 6 deletions
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 6c353c9dc772..dfec479830e4 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -127,8 +127,7 @@ struct drm_property *drm_property_create(struct drm_device *dev,
property->num_values = num_values;
INIT_LIST_HEAD(&property->enum_list);
- strncpy(property->name, name, DRM_PROP_NAME_LEN);
- property->name[DRM_PROP_NAME_LEN-1] = '\0';
+ strscpy_pad(property->name, name, DRM_PROP_NAME_LEN);
list_add_tail(&property->head, &dev->mode_config.property_list);
@@ -421,8 +420,7 @@ int drm_property_add_enum(struct drm_property *property,
if (!prop_enum)
return -ENOMEM;
- strncpy(prop_enum->name, name, DRM_PROP_NAME_LEN);
- prop_enum->name[DRM_PROP_NAME_LEN-1] = '\0';
+ strscpy_pad(prop_enum->name, name, DRM_PROP_NAME_LEN);
prop_enum->value = value;
property->values[index] = value;
@@ -475,8 +473,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
if (!property)
return -ENOENT;
- strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN);
- out_resp->name[DRM_PROP_NAME_LEN-1] = 0;
+ strscpy_pad(out_resp->name, property->name, DRM_PROP_NAME_LEN);
out_resp->flags = property->flags;
value_count = property->num_values;