Fix integer overflow in RKNPU implicit bias allocation

Merged
GO
4mo ago
+32-20

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.