Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Low] Unused local variable 'dev' left behind after removing error handling. Pre-existing issues: - [High] Missing drm_panel_remove() on devm_mipi_dsi_attach() failure causes Use-After-Free. - [High] DCS commands sent over a detached DSI bus during st7701 cleanup. -- --- Patch [6]: [PATCH v3 6/6] drm/panel: Use devm_mipi_dsi_attach in assorted panels --- commit 7405393b6171044f855c59f559f223d4f0b22c36 Author: Osama Abdelkader <[email protected]> drm/panel: Use devm_mipi_dsi_attach in assorted panels This commit converts multiple panel drivers to use the managed devm_mipi_dsi_attach helper. It drops the corresponding manual detach calls from the remove paths of these drivers. > diff --git a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c > b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c > index db006576d7046..63359416944fe 100644 > --- a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c > +++ b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c [ ... ] > @@ -264,7 +264,7 @@ static int tm5p5_nt35596_probe(struct mipi_dsi_device > *dsi) > > drm_panel_add(&ctx->panel); > > - ret = mipi_dsi_attach(dsi); > + ret = devm_mipi_dsi_attach(&dsi->dev, dsi); > if (ret < 0) { [Severity: High] This isn't a bug introduced by this patch, but if the attach fails here, does this return the error code without calling drm_panel_remove()? Because the panel struct was allocated using devm_drm_panel_alloc(), devres will automatically free the memory upon probe failure. However, the panel would remain linked in the global panel_list, which could cause a use-after-free when the list is iterated. This pattern appears to be present in multiple drivers modified in this patch (like panel-lg-sw43408.c, panel-summit.c, and panel-tdo-tl070wsh30.c). > dev_err(dev, "Failed to attach to DSI host: %d\n", ret); > return ret; > } [ ... ] > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7701.c > b/drivers/gpu/drm/panel/panel-sitronix-st7701.c > index f16e0de1ea606..b45e2e683fe9f 100644 > --- a/drivers/gpu/drm/panel/panel-sitronix-st7701.c > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7701.c [ ... ] > @@ -1347,7 +1347,7 @@ static int st7701_dsi_probe(struct mipi_dsi_device *dsi) > dsi->format = st7701->desc->format; > dsi->lanes = st7701->desc->lanes; > > - err = mipi_dsi_attach(dsi); > + err = devm_mipi_dsi_attach(&dsi->dev, dsi); [Severity: High] This is a pre-existing issue, but does this lead to DCS commands being sent over a detached DSI bus during cleanup? The st7701_probe() function registers the st7701_cleanup() devres action which eventually calls drm_panel_unprepare() and sends a sleep command. Because devm_mipi_dsi_attach() is called after st7701_probe(), the detach devres action is registered last. Since devres executes in LIFO order, the bus will be detached before st7701_cleanup() attempts to send the sleep command. > if (err) > return dev_err_probe(&dsi->dev, err, "Failed to init MIPI > DSI\n"); [ ... ] > diff --git a/drivers/gpu/drm/panel/panel-synaptics-r63353.c > b/drivers/gpu/drm/panel/panel-synaptics-r63353.c > index 3a74d48753d9c..ea3fbd0c8662e 100644 > --- a/drivers/gpu/drm/panel/panel-synaptics-r63353.c > +++ b/drivers/gpu/drm/panel/panel-synaptics-r63353.c [ ... ] > @@ -281,12 +281,6 @@ static void r63353_panel_remove(struct mipi_dsi_device > *dsi) > { > struct r63353_panel *rpanel = mipi_dsi_get_drvdata(dsi); > struct device *dev = &dsi->dev; > - int ret; > - > - ret = mipi_dsi_detach(dsi); > - if (ret < 0) > - dev_err(dev, "Failed to detach from host (%d)\n", ret); > - [Severity: Low] With the error logging removed, is the dev variable now unused and safe to remove from r63353_panel_remove() to avoid a compiler warning? > drm_panel_remove(&rpanel->base); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
