From: Ronald S. Bultje Date: Fri, 16 Dec 2011 21:42:04 +0000 (+0000) Subject: get_bits: introduce safe bitreading to prevent overreads. X-Git-Url: https://git.sesse.net/?a=commitdiff_plain;h=8cfbbd928cc94b4de6ad0a937cb818e999c7d75d;p=ffmpeg get_bits: introduce safe bitreading to prevent overreads. When turned on, H264/CAVLC gets ~15% (CVPCMNL1_SVA_C.264) slower for ultra-high-bitrate files, or ~2.5% (CVFI1_SVA_C.264) for lower-bitrate files. Other codecs are affected to a lesser extent because they are less optimized; e.g., VC-1 slows down by less than 1% (all on x86). The patch generated 3 extra instructions (cmp, cmovae and mov) per call to get_bits(). The performance penalty on ARM is within the error margin for most files, up to 4% in extreme cases such as CVPCMNL1_SVA_C.264. Based on work (for GCI) by Aneesh Dogra , and inspired by patch in Chromium by Chris Evans . --- diff --git a/configure b/configure index 974e75f91b5..22542afbe7c 100755 --- a/configure +++ b/configure @@ -113,6 +113,9 @@ Configuration options: --disable-dxva2 disable DXVA2 code --enable-runtime-cpudetect detect cpu capabilities at runtime (bigger binary) --enable-hardcoded-tables use hardcoded tables instead of runtime generation + --disable-safe-bitstream-reader + disable buffer boundary checking in bitreaders + (faster, but may crash) --enable-memalign-hack emulate memalign, interferes with memory debuggers --disable-everything disable all components listed below --disable-encoder=NAME disable encoder NAME @@ -976,6 +979,7 @@ CONFIG_LIST=" rdft rtpdec runtime_cpudetect + safe_bitstream_reader shared sinewin small @@ -1679,6 +1683,7 @@ enable doc enable fastdiv enable network enable optimizations +enable safe_bitstream_reader enable static enable swscale_alpha diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h index 684cc992fee..33ddb4b89f2 100644 --- a/libavcodec/get_bits.h +++ b/libavcodec/get_bits.h @@ -35,12 +35,32 @@ #include "libavutil/log.h" #include "mathops.h" +/* + * Safe bitstream reading: + * optionally, the get_bits API can check to ensure that we + * don't read past input buffer boundaries. This is protected + * with CONFIG_SAFE_BITSTREAM_READER at the global level, and + * then below that with UNCHECKED_BITSTREAM_READER at the per- + * decoder level. This means that decoders that check internally + * can "#define UNCHECKED_BITSTREAM_READER 1" to disable + * overread checks. + * Boundary checking causes a minor performance penalty so for + * applications that won't want/need this, it can be disabled + * globally using "#define CONFIG_SAFE_BITSTREAM_READER 0". + */ +#ifndef UNCHECKED_BITSTREAM_READER +#define UNCHECKED_BITSTREAM_READER !CONFIG_SAFE_BITSTREAM_READER +#endif + /* bit input */ /* buffer, buffer_end and size_in_bits must be present and used by every reader */ typedef struct GetBitContext { const uint8_t *buffer, *buffer_end; int index; int size_in_bits; +#if !UNCHECKED_BITSTREAM_READER + int size_in_bits_plus8; +#endif } GetBitContext; #define VLC_TYPE int16_t @@ -137,7 +157,12 @@ for examples see get_bits, show_bits, skip_bits, get_vlc # endif // FIXME name? +#if UNCHECKED_BITSTREAM_READER # define SKIP_COUNTER(name, gb, num) name##_index += (num) +#else +# define SKIP_COUNTER(name, gb, num) \ + name##_index = FFMIN((gb)->size_in_bits_plus8, name##_index + (num)) +#endif # define SKIP_BITS(name, gb, num) do { \ SKIP_CACHE(name, gb, num); \ @@ -164,7 +189,11 @@ static inline int get_bits_count(const GetBitContext *s){ } static inline void skip_bits_long(GetBitContext *s, int n){ +#if UNCHECKED_BITSTREAM_READER s->index += n; +#else + s->index += av_clip(n, -s->index, s->size_in_bits_plus8 - s->index); +#endif } /** @@ -237,7 +266,10 @@ static inline unsigned int get_bits1(GetBitContext *s){ result <<= index & 7; result >>= 8 - 1; #endif - index++; +#if !UNCHECKED_BITSTREAM_READER + if (s->index < s->size_in_bits_plus8) +#endif + index++; s->index = index; return result; @@ -314,6 +346,9 @@ static inline void init_get_bits(GetBitContext *s, s->buffer = buffer; s->size_in_bits = bit_size; +#if !UNCHECKED_BITSTREAM_READER + s->size_in_bits_plus8 = bit_size + 8; +#endif s->buffer_end = buffer + buffer_size; s->index = 0; } diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c index 00e985d230e..8854e35d936 100644 --- a/libavcodec/wmavoice.c +++ b/libavcodec/wmavoice.c @@ -25,6 +25,8 @@ * @author Ronald S. Bultje */ +#define UNCHECKED_BITSTREAM_READER 1 + #include #include "avcodec.h" #include "get_bits.h"