Re: [PATCH 03/60] drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unchecked dereference of optional `destroy_plane_state` function pointer in `drm_simple_kms_plane_reset()`.
--

--- Patch [3]: [PATCH 03/60] drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state ---
commit 134f3a4eb906f19765fc34b15053dd50db1a56e3
Author: Maxime Ripard <mripard@kernel.org>

    drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state
    
    drm_gem_simple_kms_reset_shadow_plane() implements the reset_plane
    hook for simple-kms shadow-buffered planes. Since the reset hook
    conflates initial state creation with hardware and software reset
    during suspend/resume, convert it to the create_plane_state pattern.
    
    Replace it with drm_gem_simple_kms_create_shadow_plane_state(), which
    implements the create_plane_state hook using
    drm_gem_create_shadow_plane_state(). Update
    DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS accordingly.

> diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
> index 43657d8fb1c8c..8a3bb27233db0 100644
> --- a/include/drm/drm_gem_atomic_helper.h
> +++ b/include/drm/drm_gem_atomic_helper.h
[ ... ]
> @@ -150,7 +151,7 @@ void drm_gem_simple_kms_destroy_shadow_plane_state(struct drm_simple_display_pip
>  #define DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS \
>  	.begin_fb_access = drm_gem_simple_kms_begin_shadow_fb_access, \
>  	.end_fb_access = drm_gem_simple_kms_end_shadow_fb_access, \
> -	.reset_plane = drm_gem_simple_kms_reset_shadow_plane, \
> +	.create_plane_state = drm_gem_simple_kms_create_shadow_plane_state, \
>  	.duplicate_plane_state = drm_gem_simple_kms_duplicate_shadow_plane_state, \
>  	.destroy_plane_state = drm_gem_simple_kms_destroy_shadow_plane_state

[Severity: High]
Does converting to the create_plane_state pattern expose a crash in
drm_simple_kms_plane_reset() if any drivers omit the destroy_plane_state
callback?

In drm_simple_kms_plane_reset(), if a driver provides the create_plane_state
hook but leaves the destroy_plane_state hook as NULL (relying on the default
fallback behavior), the reset path will dereference a NULL function pointer
when plane->state is not NULL:

drivers/gpu/drm/drm_simple_kms_helper.c:drm_simple_kms_plane_reset() {
	if (pipe->funcs->create_plane_state) {
		struct drm_plane_state *state;

		if (plane->state) {
			pipe->funcs->destroy_plane_state(pipe, plane->state);
			plane->state = NULL;
		}
        ...
}

drm_simple_kms_plane_destroy_state() natively supports an absent
destroy_plane_state callback by falling back to
drm_atomic_helper_plane_destroy_state(), so omitting this callback appears
to be a valid configuration that this newly introduced pattern might fail
to support.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org?part=3