Re: [PATCH 32/60] drm/sun4i: sun8i: Convert to atomic_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] sun4i_crtc_init() returns NULL on plane creation failure, but the caller checks for errors using IS_ERR(), leading to a NULL pointer dereference.
--

--- Patch [32]: [PATCH 32/60] drm/sun4i: sun8i: Convert to atomic_create_state ---
commit f2f31601b0b32de3a56fe2a7c233cf40904cfee8
Author: Maxime Ripard <mripard@kernel.org>

drm/sun4i: sun8i: Convert to atomic_create_state

This commit converts the sun8i plane reset hooks to use
atomic_create_state instead of drm_atomic_helper_plane_reset. It was
generated using a Coccinelle semantic patch.

> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index 00756ab78b4eb..bad102134726b 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer 
dereference in the sun4i display driver initialization path. 

In sun4i_crtc_init(), if plane creation fails, it returns NULL:

drivers/gpu/drm/sun4i/sun4i_crtc.c:sun4i_crtc_init() {
	planes = sunxi_engine_layers_init(drm, engine);
	if (IS_ERR(planes)) {
		dev_err(drm->dev, "Couldn't create the planes\n");
		return NULL;
	}
    ...
}

However, the caller in sun4i_tcon_bind() checks the return value using 
IS_ERR():

drivers/gpu/drm/sun4i/sun4i_tcon.c:sun4i_tcon_bind() {
    ...
	tcon->crtc = sun4i_crtc_init(drm, engine, tcon);
	if (IS_ERR(tcon->crtc)) {
		dev_err(dev, "Couldn't create our CRTC\n");
    ...
}

Since IS_ERR(NULL) evaluates to false, the error is missed and execution
continues. This NULL pointer is later dereferenced in sun4i_rgb_init():

drivers/gpu/drm/sun4i/sun4i_rgb.c:sun4i_rgb_init() {
    ...
	/* The RGB encoder can only work with the TCON channel 0 */
	rgb->encoder.possible_crtcs = drm_crtc_mask(&tcon->crtc->crtc);
    ...
}

Should sun4i_crtc_init() return an ERR_PTR (like ERR_PTR(-ENOMEM)) instead
of NULL when plane creation fails, so that the caller's IS_ERR() check 
catches it?

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