Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Freeing `gma_crtc` on `drm_crtc_init_with_planes()` failure causes a Use-After-Free because the CRTC remains linked in `dev->mode_config.crtc_list`. --
commit 2d329a448a74770c14b7d09aef862579fa99672e Author: Shixiong Ou <[email protected]> drm/gma500: Create the primary plane in the driver drm_crtc_init() creates the primary plane from a fixed format list that includes ARGB8888. The display engine programs the primary plane with DISPPLANE_32BPP_NO_ALPHA, so it does not support per-pixel alpha and must not advertise alpha formats. This commit replaces drm_crtc_init() with a driver-owned primary plane that advertises only XRGB8888. > diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c > b/drivers/gpu/drm/gma500/psb_intel_display.c > index 0df75a4a77396..f79484b9d593e 100644 > --- a/drivers/gpu/drm/gma500/psb_intel_display.c > +++ b/drivers/gpu/drm/gma500/psb_intel_display.c [ ... ] > @@ -494,7 +511,17 @@ void psb_intel_crtc_init(struct drm_device *dev, int > pipe, > return; > } > > - drm_crtc_init(dev, &gma_crtc->base, &gma_crtc_funcs); > + primary = drmm_universal_plane_alloc(dev, struct drm_plane, dev, 0, > + &gma_primary_plane_funcs, > + gma_primary_formats, > + ARRAY_SIZE(gma_primary_formats), > + NULL, DRM_PLANE_TYPE_PRIMARY, > NULL); > + if (IS_ERR(primary)) > + goto err_free_crtc; > + > + if (drm_crtc_init_with_planes(dev, &gma_crtc->base, primary, NULL, > + &gma_crtc_funcs, NULL)) > + goto err_free_crtc; [Severity: High] Could this error handling path leave a freed pointer linked in the DRM core? Looking at __drm_crtc_init_with_planes(), the CRTC is added to the list early in the function: list_add_tail(&crtc->head, &config->crtc_list); crtc->index = config->num_crtc++; If a subsequent initialization step fails (such as drm_crtc_crc_init()), __drm_crtc_init_with_planes() returns an error without removing the CRTC from the list. When jumping to err_free_crtc here, gma_crtc is freed via kfree() below. Might this create a use-after-free condition when dev->mode_config.crtc_list is later traversed? [ ... ] > @@ -524,6 +551,11 @@ void psb_intel_crtc_init(struct drm_device *dev, int > pipe, > > /* Set to true so that the pipe is forced off on initial config. */ > gma_crtc->active = true; > + return; > + > +err_free_crtc: > + kfree(gma_crtc->crtc_state); > + kfree(gma_crtc); > } > > struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int > pipe) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
