Re: [PATCH RESEND 47/83] drm/sun4i: Switch to drm_mode_config_create_initial_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/sun4i: sun4i_framebuffer_init() ignores state creation failure
--

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

drm/sun4i: Switch to drm_mode_config_create_initial_state

This commit replaces the probe-time drm_mode_config_reset() call in
sun4i_framebuffer_init() with drm_mode_config_create_initial_state() to
create the initial KMS state.

> diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
> index fc822bc2c62b5..0ce3691dedade 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
> @@ -10,6 +10,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_blend.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_print.h>
>  
>  #include "sun4i_drv.h"
>  #include "sun4i_framebuffer.h"
> @@ -26,7 +27,8 @@ static const struct drm_mode_config_helper_funcs sun4i_de_mode_config_helpers =
>  
>  void sun4i_framebuffer_init(struct drm_device *drm)
>  {
> -	drm_mode_config_reset(drm);
> +	drm_WARN(drm, drm_mode_config_create_initial_state(drm),
> +		 "Failed to create state\n");

[Severity: High]
Should sun4i_framebuffer_init() return an int and propagate errors from
drm_mode_config_create_initial_state(drm) back to sun4i_drv_bind()?

When drm_mode_config_create_initial_state(drm) fails to allocate state for a
private object, colorop, plane, CRTC, or connector, it returns early with a
negative errno, leaving that object and all subsequent KMS objects with a
NULL state pointer.

Because sun4i_framebuffer_init() only warns and returns void, its caller
sun4i_drv_bind() in drivers/gpu/drm/sun4i/sun4i_drv.c continues execution:

	sun4i_framebuffer_init(drm);

	/* Enable connectors polling */
	drm_kms_helper_poll_init(drm);

	ret = drm_dev_register(drm, 0);
	if (ret)
		goto finish_poll;

	drm_client_setup(drm, NULL);

When drm_client_setup(), a userspace atomic commit, or
drm_atomic_helper_shutdown() duplicates plane state via
sun4i_backend_layer_duplicate_state() in drivers/gpu/drm/sun4i/sun4i_layer.c,
plane->state is dereferenced without a NULL check:

	struct sun4i_layer_state *orig = state_to_sun4i_layer_state(plane->state);
	struct sun4i_layer_state *copy;

	copy = kzalloc_obj(*copy);
	if (!copy)
		return NULL;

	__drm_atomic_helper_plane_duplicate_state(plane, &copy->state);
	copy->uses_frontend = orig->uses_frontend;

Could this cause a NULL pointer dereference in
__drm_atomic_helper_plane_duplicate_state(), orig->uses_frontend, or during
userspace property queries via drm_atomic_get_property() if
sun4i_drv_bind() does not abort and jump to unbind_all on state creation
failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-drm-no-more-config-reset-v1-0-1f58d1e33380@kernel.org?part=47