Re: [PATCH v3 1/2] media: v4l2-subdev: Fix NULL pointer dereference in subdev_open()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thu, Oct 08, 2026 at 09:06:01AM +0200, Nicola Fiorillo wrote:
> Hi Laurent,
> 
> Thank you for looking at it.
> 
> On 6.12.86: agreed, that line does not belong in the commit message.

Why was it there in the first place ? It gives me very little confidence
on the quality of your contribution.

> The runs that matter are the ones on media next described in the cover
> letter, and those are what the commit message should have referred to.
> 
> On the fix: agreed as well, it does not fix the problem. subdev_open()
> now goes through vdev->v4l2_dev->mdev, which is only valid as long as
> the driver keeps the v4l2_device and the media_device alive. vimc does,
> through its v4l2_device release callback, but ipu6-isys embeds both in
> struct ipu6_isys, allocated with devm_kzalloc(), so they are freed when
> the driver is unbound.
> 
> I checked this on the IPU6 tablet (media next 9cfc1aca0781 plus the
> sensor drivers mentioned in the cover letter, without this series):
> with the media device, a video node and a CSI-2 sub-device node of
> isys held open, I unbound isys, then issued one ioctl on each node and
> closed them. KASAN reports use-after-free in v4l2_ioctl() on the video
> node, then on close in v4l2_release(), v4l2_prio_close() and
> v4l2_device_release() on the struct ipu6_isys allocated in
> isys_probe() and freed by devres at unbind, and more in subdev_close()
> and __vb2_queue_free().
> 
> Patch 2/2 rests on the same vdev->v4l2_dev assumption, so I am
> withdrawing the whole series rather than keeping half of it. The
> underlying issue is the lifetime of the objects in the driver, and
> that needs a proper fix along the lines of what Hans did for em28xx,
> not another check in the file operations.

-- 
Regards,

Laurent Pinchart