From 59dae3a5083a007866ea3a5dfdc821ef0380f077 Mon Sep 17 00:00:00 2001 From: Arman Hasanzadeh Date: Fri, 15 Aug 2025 08:39:12 -0700 Subject: [PATCH] Add fbounds-safety annotations for `VP8BitReader`. Reasoning: Analysis of initialization (`VP8BitReaderSetBuffer`) and usage sites (`src/utils/bit_reader_utils.c:59-61`, `src/utils/bit_reader_inl_utils.h:87`, `src/utils/bit_reader_utils.c:102`) showed: - `buf` is the current read pointer, iterating towards `buf_end`. - `buf_end` points one past the end of valid data (`start + size`). - `buf_max` points to the last position for a safe read of `lbit_t`. - All three pointers relate to the same buffer and are adjusted together in `VP8RemapBitReader`. Annotations applied: - `buf` is annotated `__ended_by(buf_end)` as it iterates up to `buf_end`. - Self-assignments `br->buf_end = br->buf_end;` are added where `br->buf` is modified to satisfy the compiler check for `__ended_by` when the pointer is updated but the end bound remains the same. - `buf_max` is annotated `__unsafe_indexable` because its arithmetic in `VP8RemapBitReader` couldn't be easily expressed with other annotations without ABI changes, and it's mainly adjusted during setup/remapping. - The `start` parameters in `VP8BitReaderSetBuffer` (`src/utils/bit_reader_utils.h:116`, `src/utils/bit_reader_utils.c:34`) `VP8InitBitReader` (`src/utils/bit_reader_utils.h:113`, and `src/utils/bit_reader_utils.c:45`) are annotated `__counted_by(size)` because they receive a buffer start and size, and `start` using `size` within `VP8BitReaderSetBuffer`. This resolved errors arithmetic is performed on within the function and a subsequent warning when calling it from `VP8InitBitReader`. Bug: 432511821 Change-Id: Ica436d9764b5bd1f7dcf1b54a280b6f60ecef4df --- src/utils/bit_reader_inl_utils.h | 1 + src/utils/bit_reader_utils.c | 7 +++++-- src/utils/bit_reader_utils.h | 15 +++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/utils/bit_reader_inl_utils.h b/src/utils/bit_reader_inl_utils.h index 2db0d825..36369304 100644 --- a/src/utils/bit_reader_inl_utils.h +++ b/src/utils/bit_reader_inl_utils.h @@ -85,6 +85,7 @@ static WEBP_UBSAN_IGNORE_UNDEF WEBP_INLINE void VP8LoadNewBytes( WEBP_UNSAFE_MEMCPY(&in_bits, br->buf, sizeof(in_bits)); #endif br->buf += BITS >> 3; + WEBP_SELF_ASSIGN(br->buf_end); #if !defined(WORDS_BIGENDIAN) #if (BITS > 32) bits = BSwap64(in_bits); diff --git a/src/utils/bit_reader_utils.c b/src/utils/bit_reader_utils.c index 5b252414..33921344 100644 --- a/src/utils/bit_reader_utils.c +++ b/src/utils/bit_reader_utils.c @@ -31,7 +31,8 @@ WEBP_ASSUME_UNSAFE_INDEXABLE_ABI //------------------------------------------------------------------------------ // VP8BitReader -void VP8BitReaderSetBuffer(VP8BitReader* const br, const uint8_t* const start, +void VP8BitReaderSetBuffer(VP8BitReader* const br, + const uint8_t* const WEBP_COUNTED_BY(size) start, size_t size) { assert(start != NULL); br->buf = start; @@ -40,7 +41,8 @@ void VP8BitReaderSetBuffer(VP8BitReader* const br, const uint8_t* const start, (size >= sizeof(lbit_t)) ? start + size - sizeof(lbit_t) + 1 : start; } -void VP8InitBitReader(VP8BitReader* const br, const uint8_t* const start, +void VP8InitBitReader(VP8BitReader* const br, + const uint8_t* const WEBP_COUNTED_BY(size) start, size_t size) { assert(br != NULL); assert(start != NULL); @@ -87,6 +89,7 @@ void VP8LoadFinalBytes(VP8BitReader* const br) { if (br->buf < br->buf_end) { br->bits += 8; br->value = (bit_t)(*br->buf++) | (br->value << 8); + WEBP_SELF_ASSIGN(br->buf_end); } else if (!br->eof) { br->value <<= 8; br->bits += 8; diff --git a/src/utils/bit_reader_utils.h b/src/utils/bit_reader_utils.h index efb217a5..d4126ae1 100644 --- a/src/utils/bit_reader_utils.h +++ b/src/utils/bit_reader_utils.h @@ -103,17 +103,20 @@ struct VP8BitReader { range_t range; // current range minus 1. In [127, 254] interval. int bits; // number of valid bits left // read buffer - const uint8_t* buf; // next byte to be read - const uint8_t* buf_end; // end of read buffer - const uint8_t* buf_max; // max packed-read position on buffer - int eof; // true if input is exhausted + const uint8_t* WEBP_ENDED_BY(buf_end) buf; // next byte to be read + const uint8_t* buf_end; // end of read buffer + // max packed-read position on buffer + const uint8_t* WEBP_UNSAFE_INDEXABLE buf_max; + int eof; // true if input is exhausted }; // Initialize the bit reader and the boolean decoder. -void VP8InitBitReader(VP8BitReader* const br, const uint8_t* const start, +void VP8InitBitReader(VP8BitReader* const br, + const uint8_t* const WEBP_COUNTED_BY(size) start, size_t size); // Sets the working read buffer. -void VP8BitReaderSetBuffer(VP8BitReader* const br, const uint8_t* const start, +void VP8BitReaderSetBuffer(VP8BitReader* const br, + const uint8_t* const WEBP_COUNTED_BY(size) start, size_t size); // Update internal pointers to displace the byte buffer by the