Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/panel: hitachi-tx10d07vm0baa: NULL deref of priv->dbi in enable/disable --
commit 918fbda401f49537dce4afff189990897a904ac4 Author: Svyatoslav Ryhel <[email protected]> drm/panel: Add Hitachi TX10D07VM0BAA MIPI DBI Type B panel driver This commit adds a DRM panel driver for the Hitachi TX10D07VM0BAA 4" WVGA MIPI DBI Type B panel used in the LG Optimus 2X P990 smartphone. > diff --git a/drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c > b/drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c > new file mode 100644 > index 0000000000000..83c2af1be9755 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c > @@ -0,0 +1,297 @@ [ ... ] > +#define panel_dbi_command(priv, cmd, seq...) \ > +({ \ > + const u8 d[] = { seq }; \ > + struct drm_panel *panel = &(priv)->panel; \ > + struct device *dev = panel->dev; \ > + int ret; \ > + ret = mipi_dbi_command_stackbuf((priv)->dbi, cmd, d, ARRAY_SIZE(d)); \ > + if (ret) \ > + dev_err_ratelimited(dev, "error %d when sending command > %#02x\n", ret, cmd); \ > + ret; \ > +}) > + > +static int hitachi_tx10d07vm0baa_prepare(struct drm_panel *panel) > +{ > + struct hitachi_tx10d07vm0baa *priv = to_hitachi_tx10d07vm0baa(panel); > + struct device *dev = panel->dev; > + int ret; > + > + priv->dbi = dev_get_drvdata(dev->parent); > + if (!priv->dbi) > + return -ENODEV; > + > + gpiod_set_value_cansleep(priv->reset_gpio, 1); > + > + ret = regulator_bulk_enable(ARRAY_SIZE(hitachi_tx10d07vm0baa_supplies), > priv->supplies); > + if (ret) { > + dev_err(dev, "failed to enable power supplies: %d\n", ret); > + return ret; > + } > + > + usleep_range(1000, 2000); > + > + gpiod_set_value_cansleep(priv->reset_gpio, 0); > + > + usleep_range(10000, 11000); > + > + return 0; > +}; > + > +static int hitachi_tx10d07vm0baa_enable(struct drm_panel *panel) > +{ > + struct hitachi_tx10d07vm0baa *priv = to_hitachi_tx10d07vm0baa(panel); > + > + panel_dbi_command(priv, MIPI_DCS_SET_PARTIAL_ROWS, 0x00, 0x00, 0x03, > 0x1f); [Severity: High] Can this dereference a NULL priv->dbi pointer in hitachi_tx10d07vm0baa_enable() and hitachi_tx10d07vm0baa_disable() if hitachi_tx10d07vm0baa_prepare() fails to obtain priv->dbi? In hitachi_tx10d07vm0baa_probe(), priv->dbi is left NULL and only looked up in hitachi_tx10d07vm0baa_prepare() via dev_get_drvdata(dev->parent). If dev_get_drvdata(dev->parent) returns NULL (for example, when the panel node is instantiated outside a bound tegra-dbi parent, as in the standalone OF-graph panel node shown in hit,tx10d07vm0baa.yaml, or after parent unbind), hitachi_tx10d07vm0baa_prepare() returns -ENODEV with priv->dbi still NULL and panel->prepared remaining false. During an atomic modeset commit in drm_atomic_helper_commit_modeset_enables(), panel_bridge_atomic_pre_enable() calls drm_panel_prepare(), which cannot abort the commit on error. Next, drm_atomic_helper_commit_encoder_bridge_enable() invokes panel_bridge_atomic_enable() (and tegra_dbi_atomic_enable() in commit "drm/tegra: Add support for 8-bit CPU interface"), which calls drm_panel_enable(panel). Because drm_panel_enable() checks panel->enabled but does not check panel->prepared: drivers/gpu/drm/drm_panel.c:drm_panel_enable() { ... if (panel->funcs && panel->funcs->enable) { ret = panel->funcs->enable(panel); if (ret < 0) goto exit; } ... } it proceeds to call hitachi_tx10d07vm0baa_enable() (and later hitachi_tx10d07vm0baa_disable()), passing priv->dbi == NULL through panel_dbi_command() -> mipi_dbi_command_stackbuf() -> mipi_dbi_command_buf(): drivers/gpu/drm/drm_mipi_dbi.c:mipi_dbi_command_buf() { ... mutex_lock(&dbi->cmdlock); ret = dbi->command(dbi, cmdbuf, data, len); mutex_unlock(&dbi->cmdlock); ... } Would it make sense to look up priv->dbi in hitachi_tx10d07vm0baa_probe() (or guard against a NULL priv->dbi before issuing DBI commands)? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
