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, debian-qa@lists.debian.org,
Nicolas Dufresne <nicolas@ndufresne.ca>
Cc: linux-media@vger.kernel.org, kernel@pengutronix.de,
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: Thu, 25 May 2023 17:16:36 +0200 [thread overview]
Message-ID: <2952965.Kq1lg62c9O@prancing-pony> (raw)
In-Reply-To: <a651486dbfdc2f755098b3f400c48ae3958eb6d8.camel@ndufresne.ca>
[-- Attachment #1.1: Type: text/plain, Size: 3499 bytes --]
Hi Nicolas,
On Thursday, 25 May 2023 16:36:13 CEST Nicolas Dufresne wrote:
> Le mardi 23 mai 2023 à 16:22 +0200, Diederik de Haas a écrit :
> > 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.
>
> An official form of this comment would be Tested-by: <name> <email>, would
> you agree to have this added ?
I don't have a principle objection to that, but I already did give my
"Tested-by" for what I think is the latest version to fix this issue:
https://lore.kernel.org/linux-media/3187393.mvXUDI8C0e@bagend/
I was about to 'raise concern', but I just saw that Hans Verkuil already went
with the patch for which I had (officially) added my "Tested-by"
Cheers,
Diederik
[-- 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
next prev parent reply other threads:[~2023-05-25 15:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-16 9:12 [PATCH] media: verisilicon: Fix null pointer dereference in try_fmt Michael Tretter
2023-05-23 14:22 ` Diederik de Haas
2023-05-25 14:36 ` Nicolas Dufresne
2023-05-25 15:16 ` Diederik de Haas [this message]
2023-05-25 14:38 ` Nicolas Dufresne
2023-05-25 14:46 ` Hans Verkuil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2952965.Kq1lg62c9O@prancing-pony \
--to=didi.debian@cknow.org \
--cc=benjamin.gaignard@collabora.com \
--cc=debian-qa@lists.debian.org \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=hverkuil-cisco@xs4all.nl \
--cc=kernel@pengutronix.de \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=m.tretter@pengutronix.de \
--cc=mchehab@kernel.org \
--cc=nicolas@ndufresne.ca \
--cc=p.zabel@pengutronix.de \
--cc=regressions@leemhuis.info \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).