[FFmpeg-cvslog] avcodec/put_bits: Make skip_put_bits() less dangerous
Andreas Rheinhardt
git at videolan.org
Sat Aug 8 02:34:31 EEST 2020
ffmpeg | branch: master | Andreas Rheinhardt <andreas.rheinhardt at gmail.com> | Fri Jul 31 08:32:55 2020 +0200| [06fef1e9f11a45df809e7cc4522f7174f48a664f] | committer: Andreas Rheinhardt
avcodec/put_bits: Make skip_put_bits() less dangerous
Before c63c303a1f2b58677d480505ec93a90f77dd25b5 (the commit which
introduced a typedef for the type of the buffer of a PutBitContext)
skip_put_bits() was as follows:
static inline void skip_put_bits(PutBitContext *s, int n)
{
s->bit_left -= n;
s->buf_ptr -= 4 * (s->bit_left >> 5);
s->bit_left &= 31;
}
If s->bit_left was negative after the first subtraction, then the next
line will divide this by 32 with rounding towards -inf and multiply by
four; the result will be negative, of course.
The aforementioned commit changed this to:
static inline void skip_put_bits(PutBitContext *s, int n)
{
s->bit_left -= n;
s->buf_ptr -= sizeof(BitBuf) * ((unsigned)s->bit_left / BUF_BITS);
s->bit_left &= (BUF_BITS - 1);
}
Casting s->bit_left to unsigned meant that the rounding is still towards
-inf; yet the right side is now always positive (it transformed the
arithmetic shift into a logical shift), so that s->buf_ptr will always
be decremented (by about UINT_MAX / 8 unless n is huge) which leads to
segfaults on further usage and is already undefined pointer arithmetic
before that. This can be reproduced with the mpeg4 encoder with the
AV_CODEC_FLAG2_NO_OUTPUT flag set.
Furthermore, the earlier version as well as the new version share
another bug: s->bit_left will be in the range of 0..(BUF_BITS - 1)
afterwards, although the assumption throughout the other PutBitContext
functions is that it is in the range of 1..BUF_BITS. This might lead to
a shift by BUF_BITS in little-endian mode. This has been fixed, too.
The new version is furthermore able to skip zero bits, too.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=06fef1e9f11a45df809e7cc4522f7174f48a664f
---
libavcodec/put_bits.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
index ddd97906b2..3ba9549948 100644
--- a/libavcodec/put_bits.h
+++ b/libavcodec/put_bits.h
@@ -364,13 +364,13 @@ static inline void skip_put_bytes(PutBitContext *s, int n)
/**
* Skip the given number of bits.
* Must only be used if the actual values in the bitstream do not matter.
- * If n is 0 the behavior is undefined.
+ * If n is < 0 the behavior is undefined.
*/
static inline void skip_put_bits(PutBitContext *s, int n)
{
- s->bit_left -= n;
- s->buf_ptr -= sizeof(BitBuf) * ((unsigned)s->bit_left / BUF_BITS);
- s->bit_left &= (BUF_BITS - 1);
+ unsigned bits = BUF_BITS - s->bit_left + n;
+ s->buf_ptr += sizeof(BitBuf) * (bits / BUF_BITS);
+ s->bit_left = BUF_BITS - (bits & (BUF_BITS - 1));
}
/**
More information about the ffmpeg-cvslog
mailing list