onnxruntime
d857f77a - Fix integer overflow in RKNPU implicit bias allocation (#29249)

Commit
24 days ago
Fix integer overflow in RKNPU implicit bias allocation (#29249) ### Description The RKNPU execution provider's ONNX converter creates implicit (all-zero) bias buffers when a `Conv` / `Gemm` / `QLinearConv` node omits its bias input. The buffer size was computed as `sizeof(T) * dim` (where `dim` derives from a model weight's shape) with no overflow check, and the raw allocation was tracked in a manually-freed `void*` list. This PR hardens the converter: - **Dimension validation:** ONNX `int64_t` dimensions are validated (via `ORT_ENFORCE` in a new `ToRknpuDim` helper) before being narrowed to the RKNPU `uint32_t` shape representation, rejecting negative or out-of-range values. This covers all four ingestion points (`HandleInitializer`, `GetInputOfOnnxModel`, `GetShape`, `GetSupportedNodes`). - **Overflow-checked allocation:** all four implicit-bias sites (`AddLayerConvImpl`, `AddLayerQLinearConvImpl`, `AddLayerDepthwiseConvImpl`, `AddLayerFC`) go through a shared `AllocZeroedBias` helper that computes the byte count with `SafeInt<size_t>` (throws on overflow) and returns a zero-initialized `std::make_unique<uint8_t[]>` buffer. - **RAII ownership:** `free_list_` is now `std::vector<std::unique_ptr<uint8_t[]>>`, so the bias buffers are freed automatically and `Clear()` no longer walks/`free()`s raw pointers. ### Motivation and Context A malicious ONNX model can provide dimensions that are unsafe for the RKNPU converter's 32-bit shape representation or for byte-size allocation arithmetic: - ONNX stores dimensions as `int64_t`, while the RKNPU converter/DDK uses `uint32_t` shape values. Silently narrowing a large or negative `int64_t` value can produce a misleading `uint32_t` dimension. - Even after a dimension is represented as `uint32_t`, the original `sizeof(T) * dim` could overflow `size_t` on 32-bit RKNPU targets. For example, `dim = 0x40000400` makes `sizeof(float) * dim` wrap to **4096 bytes** while the created tensor still advertises `dim` elements, which would corrupt the heap when the bias is consumed by the driver. - The original code also passed the `malloc` result to `memset` without a null check. The fix uses `SafeInt<size_t>` (the ORT-standard idiom for memory-size arithmetic), validates ONNX dimensions before they enter the RKNPU `uint32_t` shape model, and replaces the manual `malloc`/`free` list with zero-initialized, RAII-owned `std::make_unique<uint8_t[]>` buffers. **Validation:** the RKNPU EP requires the Rockchip DDK (`RKNPU_DDK_PATH`) and an ARM target, so it does not build on a typical x64 dev box and has no GPU-free CI leg. The change was validated with `clang-format`, `git diff --check`, and a standalone test against ORT's `SafeInt.hpp`, confirming the guard throws on the overflowing dimensions (`0x40000400`, `0xFFFFFFFF`) while preserving normal and zero-dimension behavior. **Testing:** No unit test is included because the RKNPU EP is not compiled in any CI leg (it requires the proprietary Rockchip DDK and an ARM target), and the `ToRknpuDim` / `AllocZeroedBias` helpers have internal (file-`static`) linkage, so they are not reachable from the gtest suites. The overflow/validation logic was instead exercised with a standalone test against ORT's `SafeInt.hpp` as noted above. --------- Co-authored-by: Gopalakrishnan Nallasamy <gopalakrishnan.nallasamy@microsoft.com>
Parents
Loading