* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/cbs_av1: Avoid shift overflow
2024-05-09 20:38 ` [FFmpeg-devel] [FFmpeg-cvslog] avcodec/cbs_av1: Avoid shift overflow Mark Thompson
@ 2024-05-10 21:33 ` Michael Niedermayer
0 siblings, 0 replies; 2+ messages in thread
From: Michael Niedermayer @ 2024-05-10 21:33 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2398 bytes --]
On Thu, May 09, 2024 at 09:38:14PM +0100, Mark Thompson wrote:
> On 09/05/2024 16:10, Michael Niedermayer wrote:
> > ffmpeg | branch: master | Michael Niedermayer <michael@niedermayer.cc> | Wed May 1 21:44:33 2024 +0200| [d7924a4f60f2088de1e6790345caba929eb97030] | committer: Michael Niedermayer
> >
> > avcodec/cbs_av1: Avoid shift overflow
> >
> > Fixes: CID1465488 Unintentional integer overflow
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >
> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=d7924a4f60f2088de1e6790345caba929eb97030
> > ---
> >
> > libavcodec/cbs_av1.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> > index 1d9ac5ab44..fb82996022 100644
> > --- a/libavcodec/cbs_av1.c
> > +++ b/libavcodec/cbs_av1.c
> > @@ -301,7 +301,7 @@ static int cbs_av1_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
> > return AVERROR(ENOSPC);
> >
> > if (len > 0)
> > - put_bits(pbc, len, (1 << len) - 1 - (value != range_max));
> > + put_bits(pbc, len, (1U << len) - 1 - (value != range_max));
> >
> > CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
> >
> What syntax element can call this with range_max - range_min == 31? (Do you have a stream?)
coverity is a tool doing static analysis. There are no streams.
cbs_av1_write_increment() checks its inputs at the begin of the function
av_assert0(range_min <= range_max && range_max - range_min < 32);
if (value < range_min || value > range_max) ...
"error"
If we assume these are correct then the function is supposed to support
range_max = range_min + 31
otherwise the assert really should be range_max - range_min < 31
both value = range_max and value = range_max - 1 will overflow
1 << len but not 1U << len
Do you suggest that the range in the assert() is wrong ?
The patch/commit intended to fix the discrepancy of the asserts range
and what the function actually supports. Iam not sure if an input stream
can trigger this.
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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] 2+ messages in thread