Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

+3 -6
+3 -6
drivers/gpu/drm/drm_property.c
··· 127 127 property->num_values = num_values; 128 128 INIT_LIST_HEAD(&property->enum_list); 129 129 130 - strncpy(property->name, name, DRM_PROP_NAME_LEN); 131 - property->name[DRM_PROP_NAME_LEN-1] = '\0'; 130 + strscpy_pad(property->name, name, DRM_PROP_NAME_LEN); 132 131 133 132 list_add_tail(&property->head, &dev->mode_config.property_list); 134 133 ··· 420 421 if (!prop_enum) 421 422 return -ENOMEM; 422 423 423 - strncpy(prop_enum->name, name, DRM_PROP_NAME_LEN); 424 - prop_enum->name[DRM_PROP_NAME_LEN-1] = '\0'; 424 + strscpy_pad(prop_enum->name, name, DRM_PROP_NAME_LEN); 425 425 prop_enum->value = value; 426 426 427 427 property->values[index] = value; ··· 473 475 if (!property) 474 476 return -ENOENT; 475 477 476 - strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN); 477 - out_resp->name[DRM_PROP_NAME_LEN-1] = 0; 478 + strscpy_pad(out_resp->name, property->name, DRM_PROP_NAME_LEN); 478 479 out_resp->flags = property->flags; 479 480 480 481 value_count = property->num_values;