Re: [PATCH v5 14/14] drm/vkms: Add driver-specific plane property for testing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vkms: NULL pointer dereference in vkms_plane_create_state on property alloc failure
--

commit d0a4c2c6cd5d0119f8db12db7c2a842adce4aa8a
Author: Maxime Ripard <mripard@kernel.org>

drm/vkms: Add driver-specific plane property for testing

This commit adds a driver-specific range property to vkms planes for testing
the DRM_MODE_ATOMIC_RESET flag. It initializes the property during plane
creation and duplicates its value in the plane state.

> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index fd3d34dcf0fe5..0ebc131c2788f 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
[ ... ]
> @@ -85,7 +92,9 @@ static void vkms_plane_destroy_state(struct drm_plane *plane,
>  
>  static struct drm_plane_state *vkms_plane_create_state(struct drm_plane *plane)
>  {
> +	struct vkms_plane *vkms_plane = to_vkms_plane(plane);
>  	struct vkms_plane_state *vkms_state;
> +	u64 val;
>  
>  	vkms_state = kzalloc_obj(*vkms_state);
>  	if (!vkms_state) {
> @@ -95,14 +104,53 @@ static struct drm_plane_state *vkms_plane_create_state(struct drm_plane *plane)
>  
>  	__drm_gem_shadow_plane_state_init(plane, &vkms_state->base);
>  
> +	if (!drm_object_property_get_default_value(&plane->base,
> +						   vkms_plane->vkms_test_do_not_use_property,
> +						   &val))

[Severity: High]
Can vkms_plane->vkms_test_do_not_use_property be NULL here?

In vkms_plane_init(), if drm_property_create_range() fails, the property
pointer is left as NULL, but initialization succeeds:

	plane->vkms_test_do_not_use_property =
		drm_property_create_range(dev, 0,
					  "vkms-test-do-not-use-in-prod",
					  0, U32_MAX);
	if (plane->vkms_test_do_not_use_property)
		drm_object_attach_property(&plane->base.base,
					   plane->vkms_test_do_not_use_property,
					   0);

If the property pointer is NULL, calling
drm_object_property_get_default_value() will trigger a NULL pointer
dereference because the DRM core unconditionally dereferences it:

drivers/gpu/drm/drm_mode_object.c:drm_object_property_get_default_value() {
	WARN_ON(!drm_drv_uses_atomic_modeset(property->dev));
    ...
}

Should there be a NULL check for vkms_test_do_not_use_property before passing
it to drm_object_property_get_default_value(), or should vkms_plane_init()
return an error if the property allocation fails?

> +		vkms_state->vkms_test_do_not_use_value = val;
> +
>  	return &(&vkms_state->base)->base;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-drm-reset-state-flag-v5-0-dad2d64b693b@kernel.org?part=14