Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check
@ 2024-01-25  0:54 Dai, Jianhui J
  2024-02-23  0:43 ` Dai, Jianhui J
  2024-03-18 11:34 ` Andreas Rheinhardt
  0 siblings, 2 replies; 5+ messages in thread
From: Dai, Jianhui J @ 2024-01-25  0:54 UTC (permalink / raw)
  To: ffmpeg-devel

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)
-- 
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check
  2024-01-25  0:54 [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check Dai, Jianhui J
@ 2024-02-23  0:43 ` Dai, Jianhui J
  2024-03-05  5:33   ` Dai, Jianhui J
  2024-03-18 11:34 ` Andreas Rheinhardt
  1 sibling, 1 reply; 5+ messages in thread
From: Dai, Jianhui J @ 2024-02-23  0:43 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check
  2024-02-23  0:43 ` Dai, Jianhui J
@ 2024-03-05  5:33   ` Dai, Jianhui J
  0 siblings, 0 replies; 5+ messages in thread
From: Dai, Jianhui J @ 2024-03-05  5:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check
  2024-01-25  0:54 [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check Dai, Jianhui J
  2024-02-23  0:43 ` Dai, Jianhui J
@ 2024-03-18 11:34 ` Andreas Rheinhardt
  2024-03-19  6:39   ` Dai, Jianhui J
  1 sibling, 1 reply; 5+ messages in thread
From: Andreas Rheinhardt @ 2024-03-18 11:34 UTC (permalink / raw)
  To: ffmpeg-devel

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)

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check
  2024-03-18 11:34 ` Andreas Rheinhardt
@ 2024-03-19  6:39   ` Dai, Jianhui J
  0 siblings, 0 replies; 5+ messages in thread
From: Dai, Jianhui J @ 2024-03-19  6:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-19  6:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-25  0:54 [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check Dai, Jianhui J
2024-02-23  0:43 ` Dai, Jianhui J
2024-03-05  5:33   ` Dai, Jianhui J
2024-03-18 11:34 ` Andreas Rheinhardt
2024-03-19  6:39   ` Dai, Jianhui J

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git