Re: [PATCH v4 2/7] drm/panel: Add Hitachi TX10D07VM0BAA MIPI DBI Type B panel driver

[sashiko-bot@kernel.org]

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 <clamor95@gmail.com>

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/20261022132259.240717-1-clamor95@gmail.com?part=2