Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Medium] The function `drm_writeback_connector_init_with_encoder()` is completely unused dead code that should have been removed. - [Low] Kernel-doc comments still reference the removed `drm_writeback_connector_init()` function. Pre-existing issues: - [High] Device-wide writeback properties are unconditionally destroyed when a single writeback connector fails initialization, leading to a Use-After-Free for already initialized writeback connectors. -- --- Patch [8]: [PATCH v7 8/9] drm: writeback: drop excess connector initialization functions --- commit 7c0a676196b264733f17d7cfc56986c51e0d9fd8 Author: Dmitry Baryshkov <[email protected]> drm: writeback: drop excess connector initialization functions This patch removes the unused drm_writeback_connector_init() function and the encoder field from struct drm_writeback_connector now that drivers have been converted to drmm_writeback_connector_init(). > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c > index b4a002e6d043d..683da368e339a 100644 > --- a/drivers/gpu/drm/drm_writeback.c > +++ b/drivers/gpu/drm/drm_writeback.c [ ... ] > @@ -148,61 +148,6 @@ static int create_writeback_properties(struct drm_device > *dev) > return 0; > } > > -static const struct drm_encoder_funcs drm_writeback_encoder_funcs = { > - .destroy = drm_encoder_cleanup, > -}; [ ... ] > -int drm_writeback_connector_init(struct drm_device *dev, [ ... ] > -EXPORT_SYMBOL(drm_writeback_connector_init); > - [Severity: High] This is a pre-existing issue, but I noticed something about how these properties are cleaned up on error. If a device supports multiple writeback connectors and the first one successfully initializes but the second fails, the error path in __drm_writeback_connector_init() calls delete_writeback_properties(): __drm_writeback_connector_init() { ... failed_properties: delete_writeback_properties(dev); return ret; } This frees the shared device-level properties like writeback_pixel_formats_property. If the driver probe continues regardless of the error, can this cause a Use-After-Free when userspace accesses the freed properties via the drm_mode_getconnector ioctl on the surviving first connector? > static void delete_writeback_properties(struct drm_device *dev) > { > if (dev->mode_config.writeback_pixel_formats_property) { > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > index c380a7b8f55a3..879ca103320cc 100644 > --- a/include/drm/drm_writeback.h > +++ b/include/drm/drm_writeback.h > @@ -24,17 +24,6 @@ struct drm_writeback_connector { > */ > struct drm_connector base; > > - /** > - * @encoder: Internal encoder used by the connector to fulfill > - * the DRM framework requirements. The users of the [ ... ] > - struct drm_encoder encoder; > - > /** > * @pixel_formats_blob_ptr: > * [Severity: Low] Since drm_writeback_connector_init() is removed by this patch, should the kernel-doc comments referencing it also be updated to prevent dangling references? For example, the documentation block right here for pixel_formats_blob_ptr states: "See also drm_writeback_connector_init()" There are also similar references in include/drm/drm_mode_config.h for the writeback_fb_id_property, writeback_pixel_formats_property, and writeback_out_fence_ptr_property fields. [ ... ] > @@ -148,13 +137,6 @@ drm_connector_to_writeback(struct drm_connector > *connector) > return container_of(connector, struct drm_writeback_connector, base); > } > > -int drm_writeback_connector_init(struct drm_device *dev, > - struct drm_writeback_connector *wb_connector, > - const struct drm_connector_funcs *con_funcs, > - const struct drm_encoder_helper_funcs > *enc_helper_funcs, > - const u32 *formats, int n_formats, > - u32 possible_crtcs); > - > int drm_writeback_connector_init_with_encoder(struct drm_device *dev, > struct drm_writeback_connector *wb_connector, > struct drm_encoder *enc, [Severity: Medium] The commit message mentions dropping excess connector initialization functions. Since the final usage in drm/vc4 was converted to the drmm variant, is drm_writeback_connector_init_with_encoder() now unused dead code that should also be removed here? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
