Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Calling kfree() on a pointer allocated by drmm_kzalloc() will cause a kernel panic or slab corruption. - [Medium] The dynamically allocated `formats` array is leaked if `drmm_plain_encoder_alloc()` fails. --
commit 05d31d945f92ca6cf7a26ad265d8715c134b9631 Author: Dmitry Baryshkov <[email protected]> drm/komeda: use drmm_writeback_connector_init() This commit converts the writeback connector and its encoder initialization to use DRM managed equivalents. > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c [ ... ] > @@ -143,13 +137,15 @@ static int komeda_wb_connector_add(struct > komeda_kms_dev *kms, > struct komeda_wb_connector *kwb_conn; > struct drm_writeback_connector *wb_conn; > struct drm_display_info *info; > + struct drm_encoder *encoder; > + > u32 *formats, n_formats = 0; > int err; > > if (!kcrtc->master->wb_layer) > return 0; > > - kwb_conn = kzalloc_obj(*kwb_conn); > + kwb_conn = drmm_kzalloc(&kms->base, sizeof(*kwb_conn), GFP_KERNEL); > if (!kwb_conn) > return -ENOMEM; > > @@ -165,11 +161,19 @@ static int komeda_wb_connector_add(struct > komeda_kms_dev *kms, > return -ENOMEM; > } [Severity: High] Does this error path in komeda_wb_connector_add() call kfree() on a DRM managed allocation? The kwb_conn pointer is now allocated via drmm_kzalloc(), which manages memory via the DRM device lifecycle and returns a pointer pointing inside a slab object (after the drmres tracking header). The error path just above this context shows: if (!formats) { kfree(kwb_conn); return -ENOMEM; } Calling kfree() on this managed pointer will pass an offset pointer to the slab allocator, which can result in an invalid free that triggers a kernel panic. DRM-managed memory must not be manually freed with kfree(). > - err = drm_writeback_connector_init(&kms->base, wb_conn, > - &komeda_wb_connector_funcs, > - &komeda_wb_encoder_helper_funcs, > - formats, n_formats, > - BIT(drm_crtc_index(&kcrtc->base))); > + encoder = drmm_plain_encoder_alloc(&kms->base, NULL, > + DRM_MODE_ENCODER_VIRTUAL, NULL); > + if (IS_ERR(encoder)) > + return PTR_ERR(encoder); [Severity: Medium] Does this error path leak the formats array in komeda_wb_connector_add()? The formats array was dynamically allocated earlier, but returning directly here skips calling komeda_put_fourcc_list(formats), leaking the memory. > + > + drm_encoder_helper_add(encoder, &komeda_wb_encoder_helper_funcs); > + > + encoder->possible_crtcs = drm_crtc_mask(&kcrtc->base); > + > + err = drmm_writeback_connector_init(&kms->base, wb_conn, > + &komeda_wb_connector_funcs, > + encoder, > + formats, n_formats); > komeda_put_fourcc_list(formats); > if (err) { > kfree(kwb_conn); [Severity: High] Can this kfree() corrupt memory in komeda_wb_connector_add()? Similar to the previous error path, kwb_conn is now allocated with drmm_kzalloc(), and manually calling kfree() on a DRM managed pointer can lead to a kernel panic. > return err; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
