HRX: hipModuleLoadData segfaults on a truncated code object where ROCm returns an error

HRX: `hipModuleLoadData` segfaults on a truncated code object where ROCm returns an error

Feeding libhrx's libamdhip64.so a code object that is truncated but still starts with a
valid ELF magic crashes the process. Upstream ROCm's libamdhip64.so returns a clean error
for the identical input. This is a memory-safety bug (out-of-bounds read) reachable from a
public HIP entry point with a caller-supplied buffer.

Repro

#include <string.h>
#include <hip/hip_runtime.h>

/* any valid AMDGPU code object, embedded as a byte array */
extern const unsigned char kcode[];

int main(void) {
  hipModule_t m = NULL;
  unsigned char buf[64];
  memcpy(buf, kcode, 64);                 /* valid ELF magic, truncated body */
  hipModuleLoadData(&m, buf);             /* HRX: SIGSEGV */
  return 0;
}

Backend	`hipModuleLoadData` on the 64-byte buffer
upstream ROCm `libamdhip64.so`	returns 209 (`hipErrorNoBinaryForGpu`)
HRX `libamdhip64.so`	SIGSEGV, exit 139

The full, valid code object loads fine on both (HRX returns 0). Only the truncated case
diverges.

Analysis

hipModuleLoadDataEx hands the loader a zero-length span:

// libhrx/src/binding/hip/api.c:14907-14910
hipError_t result = iree_hip_module_load_data_span(
    iree_make_const_byte_span(image, 0), module);   // length hardcoded to 0

and the loader's range check is conditionally disabled when the length is zero:

// libhrx/src/binding/common/fat_binary.c  (hrx_fat_elf_checked_range)
if (IREE_UNLIKELY(elf.data_length != 0 && end > elf.data_length)) {
  return iree_make_status(IREE_STATUS_INVALID_ARGUMENT,
                          "ELF file range exceeds the available data");
}

so hrx_fat_elf_read_section_header then performs an unbounded memcpy from an offset taken
straight from the untrusted header:

// libhrx/src/binding/common/fat_binary.c:686-688
memcpy(out_section,
       view->data.data + view->section_table_offset + relative_offset,
       sizeof(*out_section));

Call chain at fault:

hrx_fat_elf_read_section_header   fat_binary.c:687
hrx_fat_measure_elf               fat_binary.c:700
iree_hal_streaming_fat_binary_describe_amdgpu_elf   fat_binary.c:843
hrx_fat_extract_concatenated_elves                  fat_binary.c:1393
iree_hal_streaming_module_create_from_memory_impl   module.c:666
iree_hip_module_load_data_span                      api.c:14749
hipModuleLoadDataEx                                 api.c:14910

Note the sibling path already does this correctly — hipLibraryLoadData passes a real
length, so it is bounds-safe:

// libhrx/src/binding/hip/library.c:310-312
result = iree_hip_module_load_data_span(
    iree_make_const_byte_span(library->source_data,
                              library->source_data_length),   // real length
    &module_handle);

So this is an inconsistency inside libhrx, not an API limitation.

Suggested fix

The HIP signature is const void* image with no size (matching upstream), so the parameter
cannot change. Either:

validate e_shoff / e_shentsize / e_shnum / e_phoff / e_phentsize / e_phnum
against an extent derived from the ELF itself before any read, so data_length == 0 still
bounds the section/program tables; or
treat data_length == 0 as a hard failure in the fat-binary path rather than as "unbounded".

The key point is that data_length == 0 must not disable bounds checking.

Relation to engine work

Not hit by the engine's own loads (GGUF/HSACO paths supply correct extents), but any tool that
hands hipModuleLoadData a buffer it did not fully verify can crash the process instead of
getting a diagnostic — including code-object caches with partial writes.

Follow-up: a correct length is already passed on a sibling path in-tree

While analysing the surrounding code I found that the fix pattern already exists in this
repository, on the HIP library API rather than the module API.

hipLibraryLoadData stores the caller's byte count and later passes a real length:

// libhrx/src/binding/hip/library.c:310-312
result = iree_hip_module_load_data_span(
    iree_make_const_byte_span(library->source_data,
                              library->source_data_length),   // real length
    &module_handle);

whereas the entry point in this report hardcodes zero:

// libhrx/src/binding/hip/api.c:14907-14910
hipError_t result = iree_hip_module_load_data_span(
    iree_make_const_byte_span(image, 0), module);   // length hardcoded to 0

So the two paths through the same loader differ only in whether the extent is supplied — and
that single difference is what disables the bounds check:

// libhrx/src/binding/common/fat_binary.c  (hrx_fat_elf_checked_range)
if (IREE_UNLIKELY(elf.data_length != 0 && end > elf.data_length)) { ... }

With data_length == 0 the guard is skipped entirely, and hrx_fat_elf_read_section_header then
memcpys from an offset taken from the untrusted ELF header.

Why this matters for the fix

hipModuleLoadData's signature (const void* image, matching upstream HIP) genuinely cannot
carry a size, so the loader must derive the extent from the code object itself before trusting
e_shoff/e_shentsize/e_shnum/e_phoff/e_phentsize/e_phnum. The library path shows the
loader handles a supplied extent correctly — the gap is specifically that data_length == 0
is treated as "unbounded" rather than "derive and validate".

A minimal change consistent with the existing design: when data_length == 0, validate the ELF
header fields against an extent computed from the header (e.g. the maximum of the section and
program table ends) before any read, instead of skipping validation. The existing
hrx_fat_elf_checked_range helper already does the right thing once given a non-zero length.

Fixed by 1bit-MONSTER/hrx-system#2 (branch 409-hipmoduleloaddata-oob off 1bit/main).

Two corrections to the issue text, both verified on ryzen (RX 9070 XT, ROCm 7.16):

The "upstream returns hipErrorNoBinaryForGpu/209" claim is false. The identical 64-byte probe against the stock ROCm runtime (~/sdk-gfx1201-devel/lib/libamdhip64.so.7.16.26332-0000000, confirmed loaded via ldd) also dies with SIGSEGV, exit 139. So the fix is hardening beyond upstream rather than restoring upstream parity. The stated root cause (a zero data_length disabling hrx_fat_elf_checked_range) is correct and is fixed — a length-less span now gets a bounded extent derived from the ELF header, proven mapped before use, and a zero-length span is a hard error in the range checker.
Residual, filed separately as HRX: crafted ELF header with small e_shoff still crashes inside the AMDGPU HAL executable loader #420 and deliberately not fixed here: crafted 64-byte headers with small e_shoff values (0x100, 0x1000) still fault after the fix, but in a different place — iree_hal_amdgpu_executable_create_from_raw_hsaco() trusts offsets inside the span it is handed. Different defect, different PR.

Verified after the fix: the truncated 64-byte header returns a HIP error and exits 0; a complete 248448-byte Kernels.so-000-gfx1201.hsaco still returns 0; hip_module_execution_test still 3/3. Raw evidence in the campaign workspace (team/evidence/409-*).

Fix landed (fork), with two corrections to the report

Fixed on 1bit-MONSTER/hrx-system 1bit/main — PR 1bit-MONSTER/hrx-system#2, branch 409-hipmoduleloaddata-oob.

The root cause in the report is right and is what the patch fixes: hipModuleLoadDataEx passed a zero length, hrx_fat_elf_checked_range() skipped its check whenever the length was zero, and hrx_fat_elf_read_section_header() then memcpy'd from an offset taken out of the caller's header. A zero-length span is now a hard error, a length-less span gets a bounded extent derived from the ELF header's own table geometry (checked arithmetic, 1 GiB policy cap), and every page of that extent is proven mapped (mincore) before it is read.

probe	before	after
64-byte ELF, `e_shoff=0x7fffffff0000`	SIGSEGV, exit 139	`-> 1 (invalid argument)`, exit 0
complete 248448-byte gfx1201 hsaco	`-> 0`	`-> 0`
64-byte header truncated from the real hsaco	exit 139	error 200, exit 0
`e_shoff=0x40000000` / `0x30000000`	exit 139	error, exit 0

Correction 1 — upstream ROCm does not return 209 either

The report's table says upstream libamdhip64.so returns hipErrorNoBinaryForGpu (209) for the 64-byte buffer. Tested here against stock ROCm 7.16 (~/sdk-gfx1201-devel/lib/libamdhip64.so.7.16.26332-0000000, confirmed loaded with ldd): the identical probe segfaults there too, exit 139. So this patch is hardening beyond upstream, not a return to upstream parity. The defect is real; that one comparison in the report is not.

Correction 2 — a second, distinct crash remains (filed, not fixed here)

Crafted 64-byte headers with small e_shoff (0x100, 0x1000) still fault after this patch, in a different place: iree_hal_amdgpu_executable_create_from_raw_hsaco() trusts offsets inside the span it is handed. The HAL receives an explicit length and can validate against it, so this is a separate hardening item: filed as #420. One defect per PR.

What this does not do

The fix lives on hrx-system 1bit/main. The engine pins third_party/hrx-system at 98d05d94, held by config/hrx-system-hold.config, so engine users do not have it yet — see the hold's own note (long-context attention miscompile after 98d05d94, plus the kernel-corpus multiple_of rename). The shipping route is a separate decision from this fix.

Evidence: team/evidence/409-evidence.md and the raw files beside it (409-before-after.txt, 409-sweep.txt, 409-hal-residual-backtrace.txt, repro source, build script). Regression: hip_module_execution_test 3/3 before and after.

HRX: pin hrx-system to the held-pin backport carrying #409, #410 and #…

Shipped: the engine now pins the fix

The hipModuleLoadData out-of-bounds read is fixed and in engine builds: PR #423 moved third_party/hrx-system to 1bit/held-98d05d94-3fixes (3319e5361, merged as 7d44c99), and the engine-level acceptance at that pin passed.

Route, for the record: the fix landed on 1bit/main (PR #2), but the engine holds hrx-system at 98d05d94 (ROCm/hrx-system#1535). Rather than block a memory-safety fix on a 1688-commit core move, the owner chose a backport branch off the same held commit carrying only the three fixes plus a +61/−0 api.h forward-port. The held AMD core is untouched, so the long-context question stays exactly where it was.

Verified at the pinned line (gfx1151, -DONEBIT_HRX=ON build exit 0):

probe	result
full code object	`ret = 0`
64-byte truncation with valid ELF magic	`ret = 200`, process exit 0, no SIGSEGV (was SIGSEGV / exit 139)

tools/check_pins.py passes (98d05d94 → 3319e5361 is ahead 4, behind 0), and tests/hrx_long_prompt.py was run against a build at this exact pin as the validated-pin record demands (7082-token prompt: expected 7341, got '7341').

Still true from the fix comment: the issue's "upstream ROCm returns 209" is wrong — stock ROCm 7.16 also segfaults on that probe, so this is hardening beyond upstream. The separate small-e_shoff crash inside the AMDGPU HAL loader remains open as #420.

Closing as completed: the reported defect is fixed, merged and pinned.

Navigation Menu

Repository navigation

HRX: hipModuleLoadData segfaults on a truncated code object where ROCm returns an error #409