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 <[email protected]> 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/[email protected]?part=1
