Re: [PATCH] media: verisilicon: Fix null pointer dereference in try_fmt

From: Diederik de Haas <didi.debian@cknow.org>
To: ezequiel@vanguardiasur.com.ar, p.zabel@pengutronix.de,
	mchehab@kernel.org, hverkuil-cisco@xs4all.nl,
	benjamin.gaignard@collabora.com,
	linux-rockchip@lists.infradead.org
Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	kernel@pengutronix.de, m.tretter@pengutronix.de,
	Michael Tretter <m.tretter@pengutronix.de>,
	"Linux regression tracking (Thorsten Leemhuis)"
	<regressions@leemhuis.info>
Subject: Re: [PATCH] media: verisilicon: Fix null pointer dereference in try_fmt
Date: Tue, 23 May 2023 16:22:56 +0200	[thread overview]
Message-ID: <6444204.lOV4Wx5bFT@bagend> (raw)
In-Reply-To: <20230516091209.3098262-1-m.tretter@pengutronix.de>

[-- Attachment #1.1: Type: text/plain, Size: 2334 bytes --]

On Tuesday, 16 May 2023 11:12:09 CEST Michael Tretter wrote:
> Since commit db6f68b51e5c ("media: verisilicon: Do not set context
> src/dst formats in reset functions"), vpu_src_fmt is not set in the
> reset function, but only set in hantro_set_fmt_out, which calls
> hantro_try_fmt before setting the format. Therefore, hantro_try_fmt
> might be called with vpu_src_fmt still being null.
> 
> Add a test if the format is actually set before checking the format.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> Fixes: db6f68b51e5c ("media: verisilicon: Do not set context src/dst formats
> in reset functions") ---
>  drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c
> b/drivers/media/platform/verisilicon/hantro_v4l2.c index
> 835518534e3b..ec37d2646fde 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -313,17 +313,20 @@ static int hantro_try_fmt(const struct hantro_ctx
> *ctx, /* Fill remaining fields */
>  		v4l2_fill_pixfmt_mp(pix_mp, fmt->fourcc, pix_mp->width,
>  				    pix_mp->height);
> -		if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE 
&&
> +		if (ctx->vpu_src_fmt &&
> +		    ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE 
&&
>  		    !hantro_needs_postproc(ctx, fmt))
>  			pix_mp->plane_fmt[0].sizeimage +=
>  				hantro_h264_mv_size(pix_mp-
>width,
>  						    pix_mp-
>height);
> -		else if (ctx->vpu_src_fmt->fourcc == 
V4L2_PIX_FMT_VP9_FRAME &&
> +		else if (ctx->vpu_src_fmt &&
> +			 ctx->vpu_src_fmt->fourcc == 
V4L2_PIX_FMT_VP9_FRAME &&
>  			 !hantro_needs_postproc(ctx, fmt))
>  			pix_mp->plane_fmt[0].sizeimage +=
>  				hantro_vp9_mv_size(pix_mp->width,
>  						   pix_mp-
>height);
> -		else if (ctx->vpu_src_fmt->fourcc == 
V4L2_PIX_FMT_HEVC_SLICE &&
> +		else if (ctx->vpu_src_fmt &&
> +			 ctx->vpu_src_fmt->fourcc == 
V4L2_PIX_FMT_HEVC_SLICE &&
>  			 !hantro_needs_postproc(ctx, fmt))
>  			pix_mp->plane_fmt[0].sizeimage +=
>  				hantro_hevc_mv_size(pix_mp-
>width,

I have verified that this patch fixes the crash I was seeing since 6.4.
https://lore.kernel.org/linux-media/12724349.O9o76ZdvQC@bagend/ is a/the other 
patch which also fixed the crash.

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip