Måns Rullgård <[email protected]> writes:

> "Ronald S. Bultje" <[email protected]> writes:
>
>> Hi,
>>
>> 2011/12/15 Måns Rullgård <[email protected]>
>>
>>> "Ronald S. Bultje" <[email protected]> writes:
>>>
>>> f> Hi,
>>> >
>>> > 2011/12/15 Måns Rullgård <[email protected]>
>>> >
>>> >> >> @@ -230,7 +263,12 @@ static inline int get_bits_count(const
>>> >> GetBitContext
>>> >> >> *s) {
>>> >> >> >  static inline void skip_bits_long(GetBitContext *s, int n){
>>> >> >> >      OPEN_READER(re, s);
>>> >> >> >      re_bit_count += n;
>>> >> >> > +#if UNCHECKED_BITSTREAM_READER
>>> >> >> >      re_buffer_ptr += re_bit_count>>5;
>>> >> >> > +#else
>>> >> >> > +    re_buffer_ptr = (re_bit_count >> 5) < (const uint32_t *)
>>> >> s->buffer_end - re_buffer_ptr ?
>>> >> >> > +                    re_buffer_ptr + (re_bit_count >> 5) : (const
>>> >> uint32_t *) s->buffer_end;
>>> >> >> > +#endif
>>> >> >>
>>> >> >> This is again guaranteed to fall within the padding region, so
>>> whatever
>>> >> >> variant gives better code is OK to use here.
>>> >> >
>>> >> > gcc messes up and does 4 more instructions. Fortunately it's not
>>> terribly
>>> >> > important, but I'll stick to this:
>>> >> >
>>> >> > [..]
>>> >> >       re_buffer_ptr += re_bit_count>>5;
>>> >> > + #if !UNCHECKED_BITSTREAM_READER
>>> >> > +     re_buffer_ptr = FFMIN(re_buffer_ptr, s->buffer_end);
>>> >> > + #endif
>>> >>
>>> >> Gah, I'm stupid.  This is skip_bits_long(), which can be called with any
>>> >> value.  It must protect against overflow.
>>> >
>>> > No you're actually right, but differently; re_bit_count is never >31.
>>>
>>> The &= 31 is after the addition, so it can have any value.  If that were
>>> not the case, skip_bits_long() would not work at all.
>>>
>>
>> re_bit_count is the bit offset within the uint32_t. It's never >31, also
>> not before the addition. So for re_bit_count < 31 and n any value, it
>> should mostly work.
>
> 31 + INT_MAX overflows.

Since this is comparing pointers, overflowing the padding is sufficient
for it to become unsafe (if the (padded) buffer ends near the top of the
address space).

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to