[FFmpeg-devel] [PATCH 1/2] lavc/bitstream: avoid UB in bits_{read, peek}_signed(0)

* [FFmpeg-devel] [PATCH 1/2] lavc/bitstream: avoid UB in bits_{read, peek}_signed(0)
@ 2023-01-15  9:59 Anton Khirnov
  2023-01-15  9:59 ` [FFmpeg-devel] [PATCH 2/2] lavc/tests/bitstream: test bits_*_signed_nz and bits_peek_signed* Anton Khirnov
  0 siblings, 1 reply; 2+ messages in thread
From: Anton Khirnov @ 2023-01-15  9:59 UTC (permalink / raw)
  To: ffmpeg-devel

bits_*_signed(0) will currently invoke an undefined shift by
8 * sizeof(int).

Add bits_*_signed_nz() that only works for n>0, analogous to
bits_read_nz(). Add an explicit check for n=0 in bits_*_signed().

Found-by: James Almer
---
 libavcodec/bitstream.h          |  4 ++++
 libavcodec/bitstream_template.h | 36 +++++++++++++++++++++++++++++++--
 libavcodec/get_bits.h           |  2 +-
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/libavcodec/bitstream.h b/libavcodec/bitstream.h
index ef7d8d55c2..b60f0c296d 100644
--- a/libavcodec/bitstream.h
+++ b/libavcodec/bitstream.h
@@ -89,9 +89,11 @@
 # define bits_read_63       bits_read_63_le
 # define bits_read_64       bits_read_64_le
 # define bits_read_signed   bits_read_signed_le
+# define bits_read_signed_nz bits_read_signed_nz_le
 # define bits_peek_nz       bits_peek_nz_le
 # define bits_peek          bits_peek_le
 # define bits_peek_signed   bits_peek_signed_le
+# define bits_peek_signed_nz bits_peek_signed_nz_le
 # define bits_skip          bits_skip_le
 # define bits_seek          bits_seek_le
 # define bits_align         bits_align_le
@@ -115,9 +117,11 @@
 # define bits_read_63       bits_read_63_be
 # define bits_read_64       bits_read_64_be
 # define bits_read_signed   bits_read_signed_be
+# define bits_read_signed_nz bits_read_signed_nz_be
 # define bits_peek_nz       bits_peek_nz_be
 # define bits_peek          bits_peek_be
 # define bits_peek_signed   bits_peek_signed_be
+# define bits_peek_signed_nz bits_peek_signed_nz_be
 # define bits_skip          bits_skip_be
 # define bits_seek          bits_seek_be
 # define bits_align         bits_align_be
diff --git a/libavcodec/bitstream_template.h b/libavcodec/bitstream_template.h
index f2c14fc4d3..30bea84add 100644
--- a/libavcodec/bitstream_template.h
+++ b/libavcodec/bitstream_template.h
@@ -290,13 +290,29 @@ static inline uint64_t BS_FUNC(read_64)(BSCTX *bc, unsigned int n)
     return BS_FUNC(read_63)(bc, n);
 }
 
+/**
+ * Return n bits from the buffer as a signed integer, n has to be in the 1-32
+ * range. May be faster than bits_read_signed() when n is not a compile-time
+ * constant and is known to be non-zero;
+ */
+static inline int32_t BS_FUNC(read_signed_nz)(BSCTX *bc, unsigned int n)
+{
+    av_assert2(n > 0 && n <= 32);
+    return sign_extend(BS_FUNC(read_nz)(bc, n), n);
+}
+
 /**
  * Return n bits from the buffer as a signed integer.
  * n has to be in the 0-32 range.
  */
 static inline int32_t BS_FUNC(read_signed)(BSCTX *bc, unsigned int n)
 {
-    return sign_extend(BS_FUNC(read)(bc, n), n);
+    av_assert2(n <= 32);
+
+    if (!n)
+        return 0;
+
+    return BS_FUNC(read_signed_nz)(bc, n);
 }
 
 /**
@@ -327,6 +343,17 @@ static inline uint32_t BS_FUNC(peek)(BSCTX *bc, unsigned int n)
     return BS_FUNC(peek_nz)(bc, n);
 }
 
+/**
+ * Return n bits from the buffer as a signed integer, do not change the buffer
+ * state. n has to be in the 1-32 range. May be faster than bits_peek_signed()
+ * when n is not a compile-time constant and is known to be non-zero;
+ */
+static inline int BS_FUNC(peek_signed_nz)(BSCTX *bc, unsigned int n)
+{
+    av_assert2(n > 0 && n <= 32);
+    return sign_extend(BS_FUNC(peek_nz)(bc, n), n);
+}
+
 /**
  * Return n bits from the buffer as a signed integer,
  * do not change the buffer state.
@@ -334,7 +361,12 @@ static inline uint32_t BS_FUNC(peek)(BSCTX *bc, unsigned int n)
  */
 static inline int BS_FUNC(peek_signed)(BSCTX *bc, unsigned int n)
 {
-    return sign_extend(BS_FUNC(peek)(bc, n), n);
+    av_assert2(n <= 32);
+
+    if (!n)
+        return 0;
+
+    return BS_FUNC(peek_signed_nz)(bc, n);
 }
 
 /**
diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index 1aa327a4ac..65dc080ddb 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -85,7 +85,7 @@ typedef BitstreamContext GetBitContext;
 #define get_bits1           bits_read_bit
 #define get_bits64          bits_read_64
 #define get_xbits           bits_read_xbits
-#define get_sbits           bits_read_signed
+#define get_sbits           bits_read_signed_nz
 #define get_sbits_long      bits_read_signed
 #define show_bits           bits_peek
 #define show_bits_long      bits_peek
-- 
2.35.1

_______________________________________________
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