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 <[email protected]> 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/[email protected]?part=14
