Re: [PATCH 2/2] drm/vkms: Skip pre_blend_color_transform when pipeline is all bypassed

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The VKMS asynchronous composer worker accesses the global `colorop->state` directly without locking or using a snapshotted atomic state, leading to Use-After-Free (UAF) and screen tearing.
--

--- Patch [2]: [PATCH 2/2] drm/vkms: Skip pre_blend_color_transform when pipeline is all bypassed ---
commit 8ff3a905b0e8440dde34c1e49a7de87eb5e0a637
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/vkms: Skip pre_blend_color_transform when pipeline is all bypassed

This commit skips the per-pixel unpacking and processing loop in
pre_blend_color_transform() when the color pipeline is completely empty
or all color operations are bypassed. It adds a pipeline_all_bypassed()
helper to check the bypass state.

> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index f3fb203946e83..1806c249da6fc 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
[ ... ]
> @@ -190,13 +190,29 @@ static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop *colo
>  	}
>  }
>  
> +static bool pipeline_all_bypassed(struct drm_colorop *colorop)
> +{
> +	while (colorop) {
> +		struct drm_colorop_state *colorop_state = colorop->state;

[Severity: High]
This is a pre-existing issue, but does accessing the global colorop->state
locklessly in the async composer worker risk a use-after-free?

If the composer worker reads colorop->state while userspace queues new
non-blocking commits, the following sequence seems possible:

1. The composer worker reads colorop->state (pointing to commit C2's state).
2. The worker is preempted.
3. Userspace queues another commit (C3), making C2 the old state.
4. C3 finishes its tail work and frees C2's state via cleanup_planes().
5. The preempted worker resumes and dereferences the now-freed C2 state.

The existing per-pixel loop in pre_blend_color_transform() in vkms_composer.c
already reads colorop->state directly, so this architectural flaw is
pre-existing. However, this new pipeline_all_bypassed() helper repeats the
pattern. Should the worker be using a snapshotted atomic state instead of
reading the global colorop->state?

> +
> +		if (!colorop_state || !colorop_state->bypass)
> +			return false;
> +		colorop = colorop->next;
> +	}
> +	return true;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820083123.544655-1-oushixiong1025@163.com?part=2