Re: [PATCH] drm/msm/dpu: add the missing GC block to the SM8750 DSPP catalog

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/msm/dpu: out-of-bounds read due to missing gamma_lut size check
- [High] drm/msm: GFP_KERNEL allocation in atomic_begin callback causes sleep in atomic context
--

commit 3f1bdbf4572ec65ee6d3f256b00cb20d05a40d96
Author: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

drm/msm/dpu: add the missing GC block to the SM8750 DSPP catalog

This commit adds the missing GC (gamma correction) sub-block to the SM8750 
DSPP catalog. This change enables the GAMMA_LUT property for CRTCs on SM8750 
boards, allowing userspace to perform night light and color temperature 
adjustments.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 9a993cdfab856..11dc08e4a4846 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -428,6 +428,13 @@ static const struct dpu_dspp_sub_blks sdm845_dspp_sblk = {
>  static const struct dpu_dspp_sub_blks sm8750_dspp_sblk = {
>  	.pcc = {.name = "pcc", .base = 0x1700,
>  		.len = 0x90, .version = 0x60000},
> +	/*
> +	 * SM8750 has the same GC block as SM8650/SDM845.  Without it
> +	 * dpu_crtc.c does not call drm_crtc_enable_color_mgmt(), so no
> +	 * CRTC exposes GAMMA_LUT (night light/color temperature).
> +	 */
> +	.gc = {.name = "gc", .base = 0x17c0,
> +		.len = 0x40, .version = 0x10008},

[Severity: High]
By exposing the GAMMA_LUT property to userspace here, does this code
overflow the lut array?

Looking at _dpu_crtc_get_gc_lut():

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:_dpu_crtc_get_gc_lut():
	/* Pack 1024 10-bit entries in 512 32-bit registers */
	for (i = 0; i < PGC_TBL_LEN; i++) {
		val_even = drm_color_lut_extract(lut[i * 2].green, 10);
		val_odd = drm_color_lut_extract(lut[i * 2 + 1].green, 10);

This loop unconditionally accesses up to 1024 elements of the user-provided
lut array. Because the DRM core allows blobs smaller than the property's
maximum size, and the driver's dpu_crtc_atomic_check() lacks a validation
check on the blob size, providing an undersized blob will cause an
out-of-bounds read here.

The read memory is then written to hardware registers (gc_lut->c0/c1/c2),
allowing a local attacker to leak adjacent kernel heap memory.

[Severity: High]
Can exposing the GC block trigger a sleep in an atomic context?

The DRM subsystem requires CRTC atomic callbacks to be non-sleeping, but
dpu_crtc_atomic_begin() calls into _dpu_crtc_setup_cp_blocks():

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:dpu_crtc_atomic_begin():
	_dpu_crtc_blend_setup(crtc);

	_dpu_crtc_setup_cp_blocks(crtc);

which performs a GFP_KERNEL allocation when state->gamma_lut is present:

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:_dpu_crtc_setup_cp_blocks():
			} else {
				gc_lut = kzalloc_obj(*gc_lut);
				if (!gc_lut)
					continue;
				_dpu_crtc_get_gc_lut(state, gc_lut);
				dspp->ops.setup_gc(dspp, gc_lut);
				kfree(gc_lut);
			}

The kzalloc_obj() macro implicitly uses GFP_KERNEL as its allocation flag.
GFP_KERNEL allocations can sleep (e.g., while waiting for memory reclaim or
I/O), which violates the atomic context constraints and will trigger
scheduling while atomic kernel warnings, system instability, or deadlocks.

>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013002124.332074-1-xiaqinghuan@mail.ustc.edu.cn?part=1