Re: [PATCH] media: rockchip: rkcif: Add missing MUST_CONNECT flag to pads

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Dang Huynh <dang.huynh@mainlining.org>
Cc: Mehdi Djait <mehdi.djait@linux.intel.com>,
	Michael Riesch <michael.riesch@collabora.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Gerald Loacker <gerald.loacker@wolfvision.net>,
	Hans Verkuil <hverkuil+cisco@kernel.org>,
	linux-media@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: rockchip: rkcif: Add missing MUST_CONNECT flag to pads
Date: Sat, 31 Jan 2026 00:43:01 +0200	[thread overview]
Message-ID: <20260130224301.GS3374091@killaraus> (raw)
In-Reply-To: <20260129-rkcif-null-v1-1-90788ec347f5@mainlining.org>

On Thu, Jan 29, 2026 at 02:24:02PM +0700, Dang Huynh wrote:
> The pads missed checks for connected devices which may a null dereference
> when the stream is enabled.
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000020
> pc : rkcif_interface_enable_streams+0x48/0xf0
> lr : rkcif_interface_enable_streams+0x44/0xf0
> Call trace:
>  rkcif_interface_enable_streams+0x48/0xf0
>  v4l2_subdev_enable_streams+0x26c/0x3f0
>  rkcif_stream_start_streaming+0x140/0x278
>  vb2_start_streaming+0x74/0x188
>  vb2_core_streamon+0xe0/0x1d8
>  vb2_ioctl_streamon+0x60/0xa8
>  v4l_streamon+0x2c/0x40
>  __video_do_ioctl+0x34c/0x400
>  video_usercopy+0x2d0/0x800
>  video_ioctl2+0x20/0x60
>  v4l2_ioctl+0x48/0x78
> 
> Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
> Fixes: 85411d17bee9 ("media: rockchip: rkcif: add abstraction for interface and crop blocks")
> Signed-off-by: Dang Huynh <dang.huynh@mainlining.org>
> ---
> The RKCIF driver missed MEDIA_PAD_FL_MUST_CONNECT, which causes a null
> dereference when the stream starts if the links are misconfigured.

The change looks fine,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I'd like a review from Michael though.

> ---
>  drivers/media/platform/rockchip/rkcif/rkcif-interface.c | 3 ++-
>  drivers/media/platform/rockchip/rkcif/rkcif-stream.c    | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-interface.c b/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
> index 523103872b7a..414a9980cf2e 100644
> --- a/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
> @@ -378,7 +378,8 @@ int rkcif_interface_register(struct rkcif_device *rkcif,
>  		snprintf(sd->name, sizeof(sd->name), "rkcif-mipi%d",
>  			 interface->index - RKCIF_MIPI_BASE);
>  
> -	pads[RKCIF_IF_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> +	pads[RKCIF_IF_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
> +					MEDIA_PAD_FL_MUST_CONNECT;
>  	pads[RKCIF_IF_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
>  	ret = media_entity_pads_init(&sd->entity, RKCIF_IF_PAD_MAX, pads);
>  	if (ret)
> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> index e00010a91e8b..201542b55f00 100644
> --- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> @@ -555,7 +555,7 @@ int rkcif_stream_register(struct rkcif_device *rkcif,
>  	vdev->vfl_dir = VFL_DIR_RX;
>  	video_set_drvdata(vdev, stream);
>  
> -	stream->pad.flags = MEDIA_PAD_FL_SINK;
> +	stream->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
>  
>  	stream->pix.height = CIF_MIN_HEIGHT;
>  	stream->pix.width = CIF_MIN_WIDTH;
> 
> ---
> base-commit: 3f24e4edcd1b8981c6b448ea2680726dedd87279
> change-id: 20260129-rkcif-null-fbc0be41f81a

-- 
Regards,

Laurent Pinchart