Re: [PATCH v2 19/20] drm/msm/dp: Serialize video test state changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm/dp: missing plugged_lock in msm_dp_panel_get_mode_bpp() leaves TOCTOU race
--

commit 016f17497dc5f474b86e152324fba4d9409781b6
Author: Xilin Wu <sophon@radxa.com>

drm/msm/dp: Serialize video test state changes

This commit serializes video test state access by routing it through the
display and using plugged_lock for HPD and debugfs updates. This prevents
race conditions during mode programming.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 34e1c48cdd794..c51d033598a5b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c

[ ... ]

> @@ -689,8 +699,10 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
>  
>  	dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>  
> -	if (msm_dp_display_check_video_test(msm_dp_display))
> -		bpp = msm_dp_display_get_test_bpp(msm_dp_display);
> +	guard(mutex)(&dp->plugged_lock);
> +
> +	if (dp->panel->video_test)
> +		bpp = msm_dp_link_bit_depth_to_bpp(dp->link->test_video.test_bit_depth);

[Severity: Medium]
The commit message notes that reading the test flag and requested depth
separately allows values to change mid-read, and this function was safely
updated to hold plugged_lock.

However, does this patch miss the similar reads in msm_dp_panel_get_mode_bpp()
in drivers/gpu/drm/msm/dp/dp_panel.c? 

That function is called during DRM mode validation and still reads these
fields sequentially without holding plugged_lock:

drivers/gpu/drm/msm/dp/dp_panel.c:msm_dp_panel_get_mode_bpp() {
    ...
    if (msm_dp_panel->video_test)
        bpp = msm_dp_link_bit_depth_to_bpp(
                panel->link->test_video.test_bit_depth);
    ...
}

If HPD or debugfs updates these values concurrently, could this function
observe a torn state where the test flag is true but the bit depth has not
yet been updated, causing it to incorrectly accept or reject a mode based
on inconsistent parameters?

>  	else
>  		bpp = state->bpp;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=19

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic