Message ID | DS7PR11MB7949CF2C01F31B4B8597EC61B17A2@DS7PR11MB7949.namprd11.prod.outlook.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v1] avcodec/cbs_vp8: Improve the bitstream position check | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Dai, > Jianhui J > Sent: Thursday, January 25, 2024 8:54 AM > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream > position check > > The VP8 compressed header may not be byte-aligned due to boolean coding. Use > bitwise comparison to prevent the potential overread. > > Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com> > --- > libavcodec/cbs_vp8.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index > 065156c248..13acad3724 100644 > --- a/libavcodec/cbs_vp8.c > +++ b/libavcodec/cbs_vp8.c > @@ -327,9 +327,10 @@ static int cbs_vp8_read_unit(CodedBitstreamContext > *ctx, > if (err < 0) > return err; > > + // Position may not be byte-aligned after compressed header; using bits > + // count comparison for accuracy. > pos = get_bits_count(&gbc); > - pos /= 8; > - av_assert0(pos <= unit->data_size); > + av_assert0(pos <= unit->data_size * 8); > > frame->data_ref = av_buffer_ref(unit->data_ref); > if (!frame->data_ref) Ping reviewers to help to apply. The review history can be found here: https://patchwork.ffmpeg.org/project/ffmpeg/patch/CH3PR11MB793797554CDB411074364733B1742@CH3PR11MB7937.namprd11.prod.outlook.com/ > -- > 2.25.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with > subject "unsubscribe".
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Dai, > Jianhui J > Sent: Friday, February 23, 2024 8:43 AM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the > bitstream position check > > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Dai, > > Jianhui J > > Sent: Thursday, January 25, 2024 8:54 AM > > To: ffmpeg-devel@ffmpeg.org > > Subject: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the > > bitstream position check > > > > The VP8 compressed header may not be byte-aligned due to boolean > > coding. Use bitwise comparison to prevent the potential overread. > > > > Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com> > > --- > > libavcodec/cbs_vp8.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index > > 065156c248..13acad3724 100644 > > --- a/libavcodec/cbs_vp8.c > > +++ b/libavcodec/cbs_vp8.c > > @@ -327,9 +327,10 @@ static int > > cbs_vp8_read_unit(CodedBitstreamContext > > *ctx, > > if (err < 0) > > return err; > > > > + // Position may not be byte-aligned after compressed header; using bits > > + // count comparison for accuracy. > > pos = get_bits_count(&gbc); > > - pos /= 8; > > - av_assert0(pos <= unit->data_size); > > + av_assert0(pos <= unit->data_size * 8); > > > > frame->data_ref = av_buffer_ref(unit->data_ref); > > if (!frame->data_ref) > > Ping reviewers to help to apply. > > The review history can be found here: > https://patchwork.ffmpeg.org/project/ffmpeg/patch/CH3PR11MB793797554CD > B411074364733B1742@CH3PR11MB7937.namprd11.prod.outlook.com/ @Ronald (rsbultje@gmail.com), @Andreas (andreas.rheinhardt@outlook.com) Could you please help to apply these 2 fixes? [FFmpeg-devel,v1] avcodec/cbs_vp8: Improve the bitstream position check - Patchwork https://patchwork.ffmpeg.org/project/ffmpeg/patch/DS7PR11MB7949CF2C01F31B4B8597EC61B17A2@DS7PR11MB7949.namprd11.prod.outlook.com/ [FFmpeg-devel,v1] avcodec/cbs_vp8: Use little endian in fixed() - Patchwork https://patchwork.ffmpeg.org/project/ffmpeg/patch/DS7PR11MB79499AF0B5FB03FBF1876EFCB17A2@DS7PR11MB7949.namprd11.prod.outlook.com/ > > > -- > > 2.25.1 > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
Dai, Jianhui J: > The VP8 compressed header may not be byte-aligned due to boolean > coding. Use bitwise comparison to prevent the potential overread. > > Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com> > --- > libavcodec/cbs_vp8.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c > index 065156c248..13acad3724 100644 > --- a/libavcodec/cbs_vp8.c > +++ b/libavcodec/cbs_vp8.c > @@ -327,9 +327,10 @@ static int cbs_vp8_read_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > + // Position may not be byte-aligned after compressed header; using bits > + // count comparison for accuracy. > pos = get_bits_count(&gbc); > - pos /= 8; > - av_assert0(pos <= unit->data_size); > + av_assert0(pos <= unit->data_size * 8); (pos + 7U) / 8 seems better to avoid potential overflow issues (not an issue atm, but if we ever were to use e.g. 64bit for bitcount of the GetBit API, then the multiplication on the right could overflow a 32bit size_t). > > frame->data_ref = av_buffer_ref(unit->data_ref); > if (!frame->data_ref)
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Andreas Rheinhardt > Sent: Monday, March 18, 2024 7:35 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the > bitstream position check > > Dai, Jianhui J: > > The VP8 compressed header may not be byte-aligned due to boolean > > coding. Use bitwise comparison to prevent the potential overread. > > > > Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com> > > --- > > libavcodec/cbs_vp8.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index > > 065156c248..13acad3724 100644 > > --- a/libavcodec/cbs_vp8.c > > +++ b/libavcodec/cbs_vp8.c > > @@ -327,9 +327,10 @@ static int cbs_vp8_read_unit(CodedBitstreamContext > *ctx, > > if (err < 0) > > return err; > > > > + // Position may not be byte-aligned after compressed header; using bits > > + // count comparison for accuracy. > > pos = get_bits_count(&gbc); > > - pos /= 8; > > - av_assert0(pos <= unit->data_size); > > + av_assert0(pos <= unit->data_size * 8); > > (pos + 7U) / 8 seems better to avoid potential overflow issues (not an issue atm, > but if we ever were to use e.g. 64bit for bitcount of the GetBit API, then the > multiplication on the right could overflow a 32bit size_t). Thanks. Fixed it in PATCH v2. Please take a look. > > > > > frame->data_ref = av_buffer_ref(unit->data_ref); > > if (!frame->data_ref) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index 065156c248..13acad3724 100644 --- a/libavcodec/cbs_vp8.c +++ b/libavcodec/cbs_vp8.c @@ -327,9 +327,10 @@ static int cbs_vp8_read_unit(CodedBitstreamContext *ctx, if (err < 0) return err; + // Position may not be byte-aligned after compressed header; using bits + // count comparison for accuracy. pos = get_bits_count(&gbc); - pos /= 8; - av_assert0(pos <= unit->data_size); + av_assert0(pos <= unit->data_size * 8); frame->data_ref = av_buffer_ref(unit->data_ref); if (!frame->data_ref)
The VP8 compressed header may not be byte-aligned due to boolean coding. Use bitwise comparison to prevent the potential overread. Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com> --- libavcodec/cbs_vp8.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)