Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] The kernel-doc comment for `drm_writeback_connector_init` has mismatched parameter ordering compared to the newly adapted function signature. Pre-existing issues: - [Critical] Failure to initialize a subsequent writeback connector destroys global properties attached to already initialized writeback connectors, causing a Use-After-Free. -- commit 04d131d999200c125f2670f5258a856880015f21 Author: Dmitry Baryshkov <[email protected]> drm: writeback: rename drm_writeback_connector_init_with_encoder() Rename drm_writeback_connector_init_with_encoder() to drm_writeback_connector_init() and adapt its interface to follow drmm_writeback_connector_init(). > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c > index 9fc15168c34f6..68fdac745f42f 100644 > --- a/drivers/gpu/drm/drm_writeback.c > +++ b/drivers/gpu/drm/drm_writeback.c > @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct > drm_device *dev, > } [Severity: Critical] This isn't a bug introduced by this patch, but does the error path in __drm_writeback_connector_init() improperly destroy global properties? When __drm_writeback_connector_init() fails, it jumps to the failed_properties label: failed_properties: delete_writeback_properties(dev); return ret; Then delete_writeback_properties() destroys the properties attached to the device: if (dev->mode_config.writeback_pixel_formats_property) { drm_property_destroy(dev, dev->mode_config.writeback_pixel_formats_property); dev->mode_config.writeback_pixel_formats_property = NULL; } If a driver attempts to initialize a second writeback connector and it fails, won't this unconditionally free the global property objects that are still attached to the first successfully initialized connector, causing a use-after-free when those dangling pointers are later accessed? > > /** > - * drm_writeback_connector_init_with_encoder - Initialize a writeback > connector with > + * drm_writeback_connector_init - Initialize a writeback connector with > * a custom encoder [ ... ] > @@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct > drm_device *dev, > * > * Returns: 0 on success, or a negative error code > */ > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev, > - struct drm_writeback_connector > *wb_connector, > - struct drm_encoder *enc, > - const struct drm_connector_funcs > *con_funcs, > - const u32 *formats, int n_formats) > +int drm_writeback_connector_init(struct drm_device *dev, > + struct drm_writeback_connector *wb_connector, > + const struct drm_connector_funcs *con_funcs, > + struct drm_encoder *enc, > + const u32 *formats, int n_formats) [Severity: Low] Since the parameter order of con_funcs and enc has been swapped in the signature, does the kernel-doc comment above this function need to be updated? The kernel-doc still lists @enc before @con_funcs, which creates a mismatch between the documentation and the new function signature. > { > struct drm_connector *connector = &wb_connector->base; > int ret; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
