Re: [PATCH 9/9] media: synopsys: hdmirx: get the 5V state from the upstream subdev

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: synopsys: hdmirx: UAF/Null dereference in hdmirx_notify() during teardown
--

commit c68bf2a72fd61990cd4b4472b5560e917e6f7cb9
Author: Gerald Loacker <gerald.loacker@wolfvision.net>

media: synopsys: hdmirx: get the 5V state from the upstream subdev

On a board whose HDMI connector belongs to a device in front of this
receiver, the connector's +5V line goes to that device and hpd-gpios is
absent. tx_5v_power_present() then reads a NULL descriptor, which
gpiod_get_value_cansleep() reports as zero, so the receiver never sees a
source and hdmirx_plugin() never runs.

> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index 23b4dd853be57..43d7857b4458b 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
[ ... ]
> @@ -2673,6 +2736,18 @@ static int hdmirx_register_cec(struct snps_hdmirx_dev *hdmirx_dev,
>  	return 0;
>  }
>  
> +static void hdmirx_notify(struct v4l2_subdev *sd, unsigned int notification,
> +			  void *arg)
> +{
> +	struct snps_hdmirx_dev *hdmirx_dev =
> +		container_of(sd->v4l2_dev, struct snps_hdmirx_dev, v4l2_dev);
> +
> +	if (notification != V4L2_DEVICE_NOTIFY_RX_POWER_PRESENT || !arg)
> +		return;
> +
> +	hdmirx_5v_source_edge(hdmirx_dev, !!*(unsigned int *)arg);
> +}
> +

[Severity: High]
Does this code introduce a potential NULL pointer dereference or use-after-free
regression during teardown?

During driver unbind, hdmirx_remove() calls v4l2_async_nf_unregister(), which
can set sd->v4l2_dev to NULL. Since the subdevice's IRQ remains active, it
could concurrently send a notification via v4l2_subdev_notify(). If
hdmirx_notify() is called during this time, sd->v4l2_dev may be NULL, causing
a bogus pointer dereference via container_of() and subsequently in
hdmirx_5v_source_edge().

[ ... ]
> @@ -2926,6 +3038,9 @@ static void hdmirx_remove(struct platform_device *pdev)
>  	v4l2_async_nf_unregister(&hdmirx_dev->notifier);
>  	v4l2_async_nf_cleanup(&hdmirx_dev->notifier);
>  
> +	/* The unbind above arms the hotplug work again. */
> +	cancel_delayed_work_sync(&hdmirx_dev->delayed_work_hotplug);

[Severity: High]
Can asynchronous subdevice notifications re-arm the hotplug work after this
cancel_delayed_work_sync() call?

If a subdevice notification fires after this line, hdmirx_notify() could queue
delayed_work_hotplug on a freed hdmirx_dev, leading to a use-after-free
regression.

> +
>  	media_device_unregister(&hdmirx_dev->mdev);
>  
>  	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-hdmirx-media-v1-0-ea3d77d4a5f3@pengutronix.de?part=9