From 30b2c593c9040e841f44e14d19b507c70b5db650 Mon Sep 17 00:00:00 2001 From: Arman Hasanzadeh Date: Thu, 14 Aug 2025 08:36:16 -0700 Subject: [PATCH] Add fbounds-safety annotations for `VP8LBitWriter`. Reasoning: The struct `VP8LBitWriter` in `src/utils/bit_writer_utils.h` uses `buf`, `cur`, and `end` pointers to manage a dynamic buffer. Pointer arithmetic on `buf` and `cur` caused bounds safety errors. Initial attempts using `WEBP_ENDED_BY(end)` for both `buf` and `cur` failed due to compiler limitations (one field cannot be the upper bound for multiple other fields). Using `WEBP_INDEXABLE` for `buf` and `cur` resolved the arithmetic errors but caused ABI incompatibility issues, as `VP8LBitWriter` is used in multiple compilation units with different settings. The final approach annotates `buf` with `WEBP_ENDED_BY(end)` and `cur` with `WEBP_UNSAFE_INDEXABLE`. This resolves the arithmetic errors for both pointers without changing the ABI. `WEBP_UNSAFE_INDEXABLE` is used for `cur` as a workaround for the `WEBP_ENDED_BY` limitation and ABI constraints. Additionally, the `VP8LBitWriterResize` function in `src/utils/bit_writer_utils.c` was modified: - The assignments to `bw->buf`, `bw->end`, and `bw->cur` (lines 231-233) were reordered and updated to use the local `allocated_buf` variable on the right-hand side. This satisfies the consecutive assignment requirement imposed by `WEBP_ENDED_BY(end)` on `bw->buf`. - The local variable `allocated_buf` (line 207) was annotated with `WEBP_BIDI_INDEXABLE`. - The allocation for `allocated_buf` (line 222) now uses `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` to create a safe, bounded pointer from the result of `WebPSafeMalloc`, fixing potential type mismatches when `suppress_fbounds_errors=yes`. Bug: 432511821 Change-Id: I603a6a7d3ff3bf2a8edf9749a9898ea227c32982 --- src/utils/bit_writer_utils.c | 9 +++++---- src/utils/bit_writer_utils.h | 10 +++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/utils/bit_writer_utils.c b/src/utils/bit_writer_utils.c index bd2355a1..243918a6 100644 --- a/src/utils/bit_writer_utils.c +++ b/src/utils/bit_writer_utils.c @@ -204,7 +204,7 @@ void VP8BitWriterWipeOut(VP8BitWriter* const bw) { // Returns 1 on success. static int VP8LBitWriterResize(VP8LBitWriter* const bw, size_t extra_size) { - uint8_t* allocated_buf; + uint8_t* WEBP_BIDI_INDEXABLE allocated_buf; size_t allocated_size; const size_t max_bytes = bw->end - bw->buf; const size_t current_size = bw->cur - bw->buf; @@ -219,7 +219,8 @@ static int VP8LBitWriterResize(VP8LBitWriter* const bw, size_t extra_size) { if (allocated_size < size_required) allocated_size = size_required; // make allocated size multiple of 1k allocated_size = (((allocated_size >> 10) + 1) << 10); - allocated_buf = (uint8_t*)WebPSafeMalloc(1ULL, allocated_size); + allocated_buf = (uint8_t*)WEBP_UNSAFE_FORGE_BIDI_INDEXABLE( + void*, WebPSafeMalloc(1ULL, allocated_size), allocated_size); if (allocated_buf == NULL) { bw->error = 1; return 0; @@ -229,8 +230,8 @@ static int VP8LBitWriterResize(VP8LBitWriter* const bw, size_t extra_size) { } WebPSafeFree(bw->buf); bw->buf = allocated_buf; - bw->cur = bw->buf + current_size; - bw->end = bw->buf + allocated_size; + bw->end = allocated_buf + allocated_size; + bw->cur = allocated_buf + current_size; return 1; } diff --git a/src/utils/bit_writer_utils.h b/src/utils/bit_writer_utils.h index d0cdd01d..a75db893 100644 --- a/src/utils/bit_writer_utils.h +++ b/src/utils/bit_writer_utils.h @@ -92,11 +92,11 @@ typedef uint16_t vp8l_wtype_t; #endif typedef struct { - vp8l_atype_t bits; // bit accumulator - int used; // number of bits used in accumulator - uint8_t* buf; // start of buffer - uint8_t* cur; // current write position - uint8_t* end; // end of buffer + vp8l_atype_t bits; // bit accumulator + int used; // number of bits used in accumulator + uint8_t* WEBP_ENDED_BY(end) buf; // start of buffer + uint8_t* WEBP_UNSAFE_INDEXABLE cur; // current write position + uint8_t* end; // end of buffer // After all bits are written (VP8LBitWriterFinish()), the caller must observe // the state of 'error'. A value of 1 indicates that a memory allocation