Re: [PATCH v4] drm/tiny: arcpgu: Replace simple display pipe with regular atomic helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in `arc_pgu_crtc_atomic_check` due to improper plane state retrieval API.
--

commit 80a9a4e01830c51d120962eff87086ac506b2424
Author: Davide Bonatto <bonatto.davide@gmail.com>

drm/tiny: arcpgu: Replace simple display pipe with regular atomic helpers
    
This commit migrates the arcpgu driver from the deprecated simple display pipe
helpers to explicitly managing individual plane, CRTC, and encoder objects
using standard atomic helpers.

> diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c
> index c93d61ac0bb75..adda406927332 100644
> --- a/drivers/gpu/drm/tiny/arcpgu.c
> +++ b/drivers/gpu/drm/tiny/arcpgu.c

[ ... ]

> @@ -189,54 +203,133 @@ static void arc_pgu_mode_set(struct arcpgu_drm_private *arcpgu)

[ ... ]

> +static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
> +				     struct drm_atomic_commit *state)
> +{
> +	struct drm_crtc_state *crct_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> +	struct drm_plane_state *plane_state;
> +
> +	if (!crct_state->enable)
> +		return 0;
> +
> +	plane_state = drm_atomic_get_new_plane_state(state, &arcpgu->plane);
> +	if (IS_ERR(plane_state))
> +		return PTR_ERR(plane_state);
> +

[Severity: High]
Can this result in a NULL pointer dereference during partial atomic commits?

When an atomic commit updates a CRTC property but does not modify the plane
and does not trigger a modeset, the plane might not be added to the atomic
state. In that case, drm_atomic_get_new_plane_state() returns NULL, not an
ERR_PTR.

Because IS_ERR(NULL) evaluates to false, the error check is bypassed, and
evaluating !plane_state->fb will dereference the NULL pointer. Should this
use drm_atomic_get_plane_state() instead, which safely attaches the plane to
the commit and returns an ERR_PTR on failure, or alternatively check for
NULL before dereferencing?

> +	if (!plane_state->fb)
> +		return -EINVAL;
> +
> +	return 0;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909124108.168371-1-bonatto.davide@gmail.com?part=1