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 <lionaneesh@gmail.com>, and
inspired by patch in Chromium by Chris Evans <cevans@chromium.org>.

diff --git a/configure b/configure
index 974e75f91b529eb461a06bd6ca08f461506f7e70..22542afbe7c275ae1ef166c498d9c51dadd1e24f 100755 (executable)
--- 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 684cc992fee08d7e0e11dc6c4bfd929c7e7fbb2e..33ddb4b89f2d77fe66599c022860af8c9a49de59 100644 (file)
--- 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 00e985d230ec1dbeea1fbee4085af430e8dc469d..8854e35d936dea5aa1cf7bda1ad2c87f03436d9e 100644 (file)
--- a/libavcodec/wmavoice.c
+++ b/libavcodec/wmavoice.c
@@ -25,6 +25,8 @@
  * @author Ronald S. Bultje <rsbultje@gmail.com>
  */
 
+#define UNCHECKED_BITSTREAM_READER 1
+
 #include <math.h>
 #include "avcodec.h"
 #include "get_bits.h"