Re: [PATCH v6 8/8] gpu: drm: meson: Fix DMA max segment size for DMABUF imports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Dereferencing a NULL `priv` pointer on the `devm_kzalloc` error path.
- [High] Missing `drm_bridge_remove()` on encoder probe error paths leaves a dangling bridge in the global DRM list, leading to a Use-After-Free.
- [High] Prematurely dropping the canvas device reference without a device link causes a Use-After-Free if the canvas driver is unbound.
--

commit a3547c6366bfd30d17cb91e687b8ed0c53c24146
Author: Anand Moon <linux.amoon@gmail.com>

gpu: drm: meson: Fix DMA max segment size for DMABUF imports

When using hardware-accelerated video decoding via v4l2m2m-copy with mpv or
similar players, the video decoder driver (meson_vdec) exports contiguous
memory allocations as DMABUFs. When these buffers are subsequently imported
by the display controller driver (meson-drm) for rendering via the GPU or
compositor, the DMA API throws constraint validation warnings.

> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 49ff9f1f16d32..8570add8b8315 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -247,6 +247,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  		goto free_drm;
>  	}

[Severity: High]
This is a pre-existing issue, but does the error path in
meson_drv_bind_master() cause a NULL pointer dereference?

If devm_kzalloc() fails earlier in this function:

drivers/gpu/drm/meson/meson_drv.c:meson_drv_bind_master() {
    ...
	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
	if (!priv) {
		ret = -ENOMEM;
		goto free_drm;
	}
    ...
}

It jumps to the free_drm label with priv set to NULL. The cleanup path
sequentially calls meson_encoder_dsi_remove(priv),
meson_encoder_hdmi_remove(priv), and meson_encoder_cvbs_remove(priv).

Can this cause a kernel panic when these functions unconditionally dereference
priv, such as here in meson_encoder_dsi_remove()?

drivers/gpu/drm/meson/meson_encoder_dsi.c:meson_encoder_dsi_remove() {
    ...
	if (priv->encoders[MESON_ENC_DSI]) {
    ...
}

[Severity: High]
This is a pre-existing issue, but does meson_canvas_get() leave the system
vulnerable to a use-after-free?

Looking at meson_canvas_get() in drivers/soc/amlogic/meson-canvas.c, the
device reference is dropped after extracting the canvas pointer:

drivers/soc/amlogic/meson-canvas.c:meson_canvas_get() {
    ...
	canvas = dev_get_drvdata(&canvas_pdev->dev);
	put_device(&canvas_pdev->dev);
	if (!canvas)
    ...
}

Since no device_link_add() is used to track the dependency between the DRM
driver and the canvas driver, the driver core permits unbinding the
amlogic-canvas driver.

Would doing so free its devm-managed memory and leave meson-drm with a
dangling pointer that causes a use-after-free when dereferenced later, such
as when locking canvas->lock in meson_canvas_alloc()?

[Severity: High]
This is a pre-existing issue, but does meson_encoder_cvbs_probe() leak a
bridge reference on error paths?

In drivers/gpu/drm/meson/meson_encoder_cvbs.c, the bridge is added globally:

drivers/gpu/drm/meson/meson_encoder_cvbs.c:meson_encoder_cvbs_probe() {
    ...
	drm_bridge_add(&meson_encoder_cvbs->bridge);

	meson_encoder_cvbs->priv = priv;

	/* Encoder */
	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
				      DRM_MODE_ENCODER_TVDAC);
	if (ret)
		return dev_err_probe(priv->dev, ret,
				     "Failed to init CVBS encoder\n");
    ...
}

If a subsequent initialization step fails, the function returns an error
without calling drm_bridge_remove(). The master cleanup function fails to
remove it because priv->encoders[MESON_ENC_CVBS] is only populated upon
successful probe completion.

During devres teardown, the reference count drops to 1, but the bridge is
not freed. Can this leave the bridge accessible in the global bridge_list
with dangling pointers to the freed priv structure, leading to memory
corruption if accessed by other DRM subsystems?

>  
> +	dma_set_max_seg_size(dev, UINT_MAX);
> +
>  	ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
>  	if (ret)
>  		goto free_drm;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530094326.11892-1-linux.amoon@gmail.com?part=8