Re: [PATCH 11/13] media: nxp: imx8-isi: Don't use GFP_DMA when calling dma_alloc_coherent()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Teardown symmetry violation in `mxc_isi_remove()` causing a NULL pointer dereference / Use-After-Free.
--

commit 846491721093e7e929fd40da0be5709669dc9e9f
Author: Baoquan He <hebaoquan@kylinos.cn>

media: nxp: imx8-isi: Don't use GFP_DMA when calling dma_alloc_coherent()

dma_alloc_coherent() allocates the DMA buffer with the device's
addressing limitation in mind. The DMA core picks the zone from the
device's coherent DMA mask and ignores GFP_DMA passed by the caller.
This commit removes the redundant GFP_DMA flag.

> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> index f45c2aae59ce9..fc907d357149e 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> @@ -773,7 +773,7 @@ static int mxc_isi_video_alloc_discard_buffers(struct mxc_isi_video *video)
>  
>  		buf->size = PAGE_ALIGN(video->pix.plane_fmt[i].sizeimage);
>  		buf->addr = dma_alloc_coherent(video->pipe->isi->dev, buf->size,
> -					       &buf->dma, GFP_DMA | GFP_KERNEL);
> +					       &buf->dma, GFP_KERNEL);
>  		if (!buf->addr) {
>  			mxc_isi_video_free_discard_buffers(video);
>  			return -ENOMEM;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the teardown
sequence in drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c:

drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c:mxc_isi_remove() {
	...
	for (i = 0; i < isi->pdata->num_channels; ++i) {
		struct mxc_isi_pipe *pipe = &isi->pipes[i];

		mxc_isi_pipe_cleanup(pipe);
	}

	mxc_isi_v4l2_cleanup(isi);
	...
}

Does this code have a teardown symmetry violation causing a use-after-free or
NULL pointer dereference?

mxc_isi_pipe_cleanup(pipe) is called before mxc_isi_v4l2_cleanup(isi), freeing
the subdevice active state and setting it to NULL before the user-space
interfaces are unregistered.

Because mxc_isi_v4l2_cleanup() has not yet run, the video device nodes are
still fully registered.

Could a concurrent ioctl from user-space, such as VIDIOC_STREAMON, call
mxc_isi_vb2_prepare_streaming() -> mxc_isi_pipe_acquire() ->
v4l2_subdev_lock_and_get_active_state() and dereference the now-NULL
active_state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903111836.1777265-1-hebaoquan@kylinos.cn?part=11