Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Calling `drm_vblank_init` before creating CRTCs results in a zero-size allocation, leading to a kernel crash when vblank features are later used. --
--- Patch [4]: [PATCH 4/5] drm/vblank: Use drm_for_each_crtc() in drm_vblank_init() --- commit 4aec980231f556c8a36b5294d67b3d582d49d623 Author: Lyude Paul <[email protected]> drm/vblank: Use drm_for_each_crtc() in drm_vblank_init() This patch updates drm_vblank_init() to use the drm_for_each_crtc() iterator instead of manually counting the number of CRTCs. > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 17283e8920378..d57405050554f 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -547,19 +547,20 @@ static void drm_vblank_init_release(struct drm_device > *dev, void *ptr) > int drm_vblank_init(struct drm_device *dev) > { > int ret; > - unsigned int num_crtcs = dev->mode_config.num_crtc; > - unsigned int i; > + struct drm_crtc *crtc; > > spin_lock_init(&dev->vbl_lock); > spin_lock_init(&dev->vblank_time_lock); > > - dev->vblank = drmm_kcalloc(dev, num_crtcs, sizeof(*dev->vblank), > GFP_KERNEL); > + dev->vblank = drmm_kcalloc(dev, dev->mode_config.num_crtc, > + sizeof(*dev->vblank), GFP_KERNEL); > if (!dev->vblank) > return -ENOMEM; > > dev->has_hw_vblank = true; > > - for (i = 0; i < num_crtcs; i++) { > + drm_for_each_crtc(crtc, dev) { [Severity: Critical] Could this change lead to an uninitialized vblank array and subsequent page fault for drivers that initialize vblank before creating CRTCs? For example, in vkms_drv.c:vkms_create(), drm_vblank_init() is called before vkms_modeset_init() creates the CRTCs: ret = drm_vblank_init(&vkms_device->drm); if (ret) { DRM_ERROR("Failed to vblank\n"); goto out_devres; } ret = vkms_modeset_init(vkms_device); Because the CRTCs are not yet created at this point, dev->mode_config.num_crtc is 0. The allocation for the vblank array becomes a 0-byte allocation, returning ZERO_SIZE_PTR (0x10). The drm_for_each_crtc() loop then executes 0 times, and drm_vblank_init() successfully returns 0. When userspace later calls DRM_IOCTL_WAIT_VBLANK, the kernel looks up the CRTC using drm_vblank_crtc(dev, pipe). This returns a pointer into the array, which evaluates to ZERO_SIZE_PTR + pipe * sizeof(...). Will dereferencing this pointer cause an immediate kernel OOPS? > + unsigned int i = crtc->index; > struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, i); > > vblank->dev = dev; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
