[PATCH] media: cedrus: Fix NULL buf dereference

From: Nicolas Dufresne
Date: Fri Jul 15 2022 - 15:28:44 EST

Next message: kernel test robot: "[djwong-xfs:vectorized-scrub 23/378] htmldocs: Documentation/filesystems/xfs-online-fsck-design.rst:2347: WARNING: undefined label: _quotacheck (if the link has no caption the label must precede a section header)"
Previous message: Vlastimil Babka: "[RFC] mm, slub: restrict sysfs validation to debug caches and make it safe"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

This is a regression introduced after porting to vb2_find_buffer.
The function returnis NULL when the buffer isn't found. The HEVC
decoder would call a helper to get the motion vector buffer address
by passing the index. This is fixed by passing the buffer
pointer instead of the index.

Fixes: 7f3614514ab0 ("cedrus: Use vb2_find_buffer")
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
---
drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 28d90fec9aea..d359726b4966 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -91,16 +91,13 @@ static void cedrus_h265_sram_write_data(struct cedrus_d=
ev
*dev, void *data,
=20
static inline dma_addr_t
cedrus_h265_frame_info_mv_col_buf_addr(struct cedrus_ctx *ctx,
- unsigned int index,
+ struct vb2_buffer *buf,
const struct v4l2_ctrl_hevc_sps *sps)
{
struct cedrus_buffer *cedrus_buf =3D NULL;
- struct vb2_buffer *buf =3D NULL;
struct vb2_queue *vq;
=20
vq =3D v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
- if (vq)
- buf =3D vb2_get_buffer(vq, index);
=20
if (buf)
cedrus_buf =3D vb2_to_cedrus_buffer(buf);
@@ -148,8 +145,8 @@ static void cedrus_h265_frame_info_write_single(struct
cedrus_ctx *ctx,
dma_addr_t dst_luma_addr =3D cedrus_dst_buf_addr(ctx, buf, 0);
dma_addr_t dst_chroma_addr =3D cedrus_dst_buf_addr(ctx, buf, 1);
dma_addr_t mv_col_buf_addr[2] =3D {
- cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf->index, sps),
- cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf->index, sps)
+ cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf, sps),
+ cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf, sps)
};
u32 offset =3D VE_DEC_H265_SRAM_OFFSET_FRAME_INFO +
VE_DEC_H265_SRAM_OFFSET_FRAME_INFO_UNIT * index;
--=20
GitLab

>=20
> Thanks,
> Ezequiel
>=20
> > Best regards,
> > Jernej
> >=20
> > >=20
> > > static inline dma_addr_t
> > > cedrus_h265_frame_info_mv_col_buf_addr(struct cedrus_ctx *ctx,
> > > unsigned int index, unsigned i=
nt
> > > field) {
> > > return ctx->codec.h265.mv_col_buf_addr + index *
> > > ctx->codec.h265.mv_col_buf_unit_size +
> > > field * ctx->codec.h265.mv_col_buf_unit_size / 2;
> > > }
> > >=20
> > > Fixing the driver to go back to the previous behavior is trivial,
> > > but this looks odd.
> > >=20
> > > Jernej, Paul, any thoughts?
> > >=20
> > > Thanks,
> > > Ezequiel
> > >=20
> > > > > };
> > > > > u32 offset =3D VE_DEC_H265_SRAM_OFFSET_FRAME_INFO +
> > > > >=20
> > > > > @@ -141,7 +141,7 @@ static void cedrus_h265_frame_info_write_dpb(=
struct
> > > > > cedrus_ctx *ctx,> >
> > > > > unsigned int i;
> > > > >=20
> > > > > for (i =3D 0; i < num_active_dpb_entries; i++) {
> > > > >=20
> > > > > - int buffer_index =3D vb2_find_timestamp(vq,
> > > > > dpb[i].timestamp, 0); + struct vb2_buffer *buf =3D
> > > > > vb2_find_buffer(vq, dpb[i].timestamp);> >
> > > > > u32 pic_order_cnt[2] =3D {
> > > > >=20
> > > > > dpb[i].pic_order_cnt[0],
> > > > > dpb[i].pic_order_cnt[1]
> > > > >=20
> > > > > @@ -149,7 +149,7 @@ static void cedrus_h265_frame_info_write_dpb(=
struct
> > > > > cedrus_ctx *ctx,> >
> > > > > cedrus_h265_frame_info_write_single(ctx, i,
> > > > > dpb[i].field_pic,
> > > > >=20
> > > > > pic_order_cnt,
> > > > >=20
> > > > > - buffer_index);
> > > > > + buf);
> > > > >=20
> > > > > }
> > > > >=20
> > > > > }
> > > > >=20
> > > > > @@ -616,7 +616,7 @@ static void cedrus_h265_setup(struct cedrus_c=
tx
> > > > > *ctx,
> > > > >=20
> > > > > cedrus_h265_frame_info_write_single(ctx, output_pic_list_in=
dex,
> > > > >=20
> > > > > slice_params->pic_struc=
t !=3D 0,
> > > > > pic_order_cnt,
> > > > >=20
> > > > > - run->dst->vb2_buf.index=
);
> > > > > + &run->dst->vb2_buf);
> > > > >=20
> > > > > cedrus_write(dev, VE_DEC_H265_OUTPUT_FRAME_IDX,
> > > > > output_pic_list_index);
> > > > >=20
> > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index
> > > > > 5dad2f296c6d..22d6cae9a710 100644
> > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> > > > > @@ -54,13 +54,9 @@ static void cedrus_mpeg2_setup(struct cedrus_c=
tx
> > > > > *ctx, struct cedrus_run *run)> >
> > > > > const struct v4l2_ctrl_mpeg2_picture *pic;
> > > > > const struct v4l2_ctrl_mpeg2_quantisation *quantisation;
> > > > > dma_addr_t src_buf_addr, dst_luma_addr, dst_chroma_addr;
> > > > >=20
> > > > > - dma_addr_t fwd_luma_addr, fwd_chroma_addr;
> > > > > - dma_addr_t bwd_luma_addr, bwd_chroma_addr;
> > > > >=20
> > > > > struct cedrus_dev *dev =3D ctx->dev;
> > > > > struct vb2_queue *vq;
> > > > > const u8 *matrix;
> > > > >=20
> > > > > - int forward_idx;
> > > > > - int backward_idx;
> > > > >=20
> > > > > unsigned int i;
> > > > > u32 reg;
> > > > >=20
> > > > > @@ -123,27 +119,19 @@ static void cedrus_mpeg2_setup(struct cedru=
s_ctx
> > > > > *ctx, struct cedrus_run *run)> >
> > > > > cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
> > > > >=20
> > > > > /* Forward and backward prediction reference buffers. */
> > > > >=20
> > > > > -
> > > > >=20
> > > > > vq =3D v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> > > > > V4L2_BUF_TYPE_VIDEO_CAPTURE);
> > > > >=20
> > > > > - forward_idx =3D vb2_find_timestamp(vq, pic->forward_ref_ts,=
0);
> > > > > - fwd_luma_addr =3D cedrus_dst_buf_addr(ctx, forward_idx, 0);
> > > > > - fwd_chroma_addr =3D cedrus_dst_buf_addr(ctx, forward_idx, 1=
);
> > > > > -
> > > > > - cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_a=
ddr);
> > > > > - cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR,
> > > > > fwd_chroma_addr);
> > > > > -
> > > > > - backward_idx =3D vb2_find_timestamp(vq, pic->backward_ref_t=
s, 0);
> > > > > - bwd_luma_addr =3D cedrus_dst_buf_addr(ctx, backward_idx, 0)=
;
> > > > > - bwd_chroma_addr =3D cedrus_dst_buf_addr(ctx, backward_idx, =
1);
> > > > > -
> > > > > - cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR, bwd_luma_a=
ddr);
> > > > > - cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR,
> > > > > bwd_chroma_addr);
> > > > > + cedrus_write_ref_buf_addr(ctx, vq, pic->forward_ref_ts,
> > > > > + VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
> > > > > + VE_DEC_MPEG_FWD_REF_CHROMA_ADDR);
> > > > > + cedrus_write_ref_buf_addr(ctx, vq, pic->backward_ref_ts,
> > > > > + VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
> > > > > + VE_DEC_MPEG_BWD_REF_CHROMA_ADDR);
> > > > >=20
> > > > > /* Destination luma and chroma buffers. */
> > > > >=20
> > > > > - dst_luma_addr =3D cedrus_dst_buf_addr(ctx, run->dst->vb2_bu=
f.index,
> > > > > 0);
> > > > > - dst_chroma_addr =3D cedrus_dst_buf_addr(ctx,
> > > > > run->dst->vb2_buf.index, 1); + dst_luma_addr =3D
> > > > > cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 0); + dst_chroma=
_addr
> > > > > =3D cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 1);> >
> > > > > cedrus_write(dev, VE_DEC_MPEG_REC_LUMA, dst_luma_addr);
> > > > > cedrus_write(dev, VE_DEC_MPEG_REC_CHROMA, dst_chroma_addr);
> > > > >=20
> > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c index
> > > > > f4016684b32d..196cf692186d 100644
> > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> > > > > @@ -661,7 +661,6 @@ static void cedrus_vp8_setup(struct cedrus_ct=
x *ctx,
> > > > >=20
> > > > > dma_addr_t luma_addr, chroma_addr;
> > > > > dma_addr_t src_buf_addr;
> > > > > int header_size;
> > > > >=20
> > > > > - int qindex;
> > > > >=20
> > > > > u32 reg;
> > > > >=20
> > > > > cedrus_engine_enable(ctx, CEDRUS_CODEC_VP8);
> > > > >=20
> > > > > @@ -805,43 +804,17 @@ static void cedrus_vp8_setup(struct cedrus_=
ctx
> > > > > *ctx,
> > > > >=20
> > > > > reg |=3D VE_VP8_LF_DELTA0(slice->lf.mb_mode_delta[0]);
> > > > > cedrus_write(dev, VE_VP8_MODE_LF_DELTA, reg);
> > > > >=20
> > > > > - luma_addr =3D cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.in=
dex, 0);
> > > > > - chroma_addr =3D cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.=
index,
> > > > > 1);
> > > > > + luma_addr =3D cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, =
0);
> > > > > + chroma_addr =3D cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf=
, 1);
> > > > >=20
> > > > > cedrus_write(dev, VE_VP8_REC_LUMA, luma_addr);
> > > > > cedrus_write(dev, VE_VP8_REC_CHROMA, chroma_addr);
> > > > >=20
> > > > > - qindex =3D vb2_find_timestamp(cap_q, slice->last_frame_ts, =
0);
> > > > > - if (qindex >=3D 0) {
> > > > > - luma_addr =3D cedrus_dst_buf_addr(ctx, qindex, 0);
> > > > > - chroma_addr =3D cedrus_dst_buf_addr(ctx, qindex, 1)=
;
> > > > > - cedrus_write(dev, VE_VP8_FWD_LUMA, luma_addr);
> > > > > - cedrus_write(dev, VE_VP8_FWD_CHROMA, chroma_addr);
> > > > > - } else {
> > > > > - cedrus_write(dev, VE_VP8_FWD_LUMA, 0);
> > > > > - cedrus_write(dev, VE_VP8_FWD_CHROMA, 0);
> > > > > - }
> > > > > -
> > > > > - qindex =3D vb2_find_timestamp(cap_q, slice->golden_frame_ts=
, 0);
> > > > > - if (qindex >=3D 0) {
> > > > > - luma_addr =3D cedrus_dst_buf_addr(ctx, qindex, 0);
> > > > > - chroma_addr =3D cedrus_dst_buf_addr(ctx, qindex, 1)=
;
> > > > > - cedrus_write(dev, VE_VP8_BWD_LUMA, luma_addr);
> > > > > - cedrus_write(dev, VE_VP8_BWD_CHROMA, chroma_addr);
> > > > > - } else {
> > > > > - cedrus_write(dev, VE_VP8_BWD_LUMA, 0);
> > > > > - cedrus_write(dev, VE_VP8_BWD_CHROMA, 0);
> > > > > - }
> > > > > -
> > > > > - qindex =3D vb2_find_timestamp(cap_q, slice->alt_frame_ts, 0=
);
> > > > > - if (qindex >=3D 0) {
> > > > > - luma_addr =3D cedrus_dst_buf_addr(ctx, qindex, 0);
> > > > > - chroma_addr =3D cedrus_dst_buf_addr(ctx, qindex, 1)=
;
> > > > > - cedrus_write(dev, VE_VP8_ALT_LUMA, luma_addr);
> > > > > - cedrus_write(dev, VE_VP8_ALT_CHROMA, chroma_addr);
> > > > > - } else {
> > > > > - cedrus_write(dev, VE_VP8_ALT_LUMA, 0);
> > > > > - cedrus_write(dev, VE_VP8_ALT_CHROMA, 0);
> > > > > - }
> > > > > + cedrus_write_ref_buf_addr(ctx, cap_q, slice->last_frame_ts,
> > > > > + VE_VP8_FWD_LUMA, VE_VP8_FWD_CHROM=
A);
> > > > > + cedrus_write_ref_buf_addr(ctx, cap_q, slice->golden_frame_t=
s,
> > > > > + VE_VP8_BWD_LUMA, VE_VP8_BWD_CHROM=
A);
> > > > > + cedrus_write_ref_buf_addr(ctx, cap_q, slice->alt_frame_ts,
> > > > > + VE_VP8_ALT_LUMA, VE_VP8_ALT_CHROM=
A);
> > > > >=20
> > > > > cedrus_write(dev, VE_H264_CTRL, VE_H264_CTRL_VP8 |
> > > > >=20
> > > > > VE_H264_CTRL_DECODE_ERR_INT |
> >=20
> >=20
> >=20
> >=20

Next message: kernel test robot: "[djwong-xfs:vectorized-scrub 23/378] htmldocs: Documentation/filesystems/xfs-online-fsck-design.rst:2347: WARNING: undefined label: _quotacheck (if the link has no caption the label must precede a section header)"
Previous message: Vlastimil Babka: "[RFC] mm, slub: restrict sysfs validation to debug caches and make it safe"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]