From f64c1dd26387b6d47555a53ed1828d3bbc236632 Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Fri, 13 Mar 2026 09:16:58 -0700 Subject: [PATCH 1/3] Fix gsl::narrow crash in rewrite_extern_constant_load for large values BPF MOV imm has a 32-bit immediate field that is sign-extended to 64 bits. When rewrite_extern_constant_load() rewrites a LDDW+LDX pair into a MOV imm, it called gsl::narrow(narrowed_value) which throws narrowing_error if the resolved 64-bit constant exceeds INT32_MAX (e.g., 0x80000000 or larger). Fix: check whether the value fits in a sign-extended int32 before rewriting. If it doesn't fit, return false so the caller falls back to the original LDDW+LDX instruction sequence. Add regression test covering boundary values. Signed-off-by: Michael Agun --- src/io/elf_extern_resolve.cpp | 12 ++++++- src/test/test_elf_loader.cpp | 64 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/io/elf_extern_resolve.cpp b/src/io/elf_extern_resolve.cpp index 3fac718fc..76d459073 100644 --- a/src/io/elf_extern_resolve.cpp +++ b/src/io/elf_extern_resolve.cpp @@ -1,5 +1,7 @@ // Copyright (c) Prevail Verifier contributors. // SPDX-License-Identifier: MIT +#include +#include #include #include #include @@ -83,6 +85,14 @@ bool rewrite_extern_constant_load(std::vector& instructions, const siz narrowed_value = static_cast((static_cast(narrowed_value << shift)) >> shift); } + // BPF MOV imm has a 32-bit immediate field that is sign-extended to 64 bits + // by the runtime. Bail out if the value cannot be represented this way; + // the caller will fall back to the original LDDW+LDX instruction sequence. + const auto signed_value = static_cast(narrowed_value); + if (signed_value > std::numeric_limits::max() || signed_value < std::numeric_limits::min()) { + return false; + } + // Use mov-imm to materialize the resolved constant in the destination register of // the load, and neutralize the preceding LDDW pair. const uint8_t mov_opcode = width == 8 || mode == INST_MODE_MEMSX @@ -91,7 +101,7 @@ bool rewrite_extern_constant_load(std::vector& instructions, const siz load_inst.opcode = mov_opcode; load_inst.src = 0; load_inst.offset = 0; - load_inst.imm = gsl::narrow(narrowed_value); + load_inst.imm = static_cast(narrowed_value); lo_inst.get() = make_mov_reg_nop(lo_inst.get().dst); hi_inst.get() = make_mov_reg_nop(hi_inst.get().dst); diff --git a/src/test/test_elf_loader.cpp b/src/test/test_elf_loader.cpp index a8baffaa6..b1754945d 100644 --- a/src/test/test_elf_loader.cpp +++ b/src/test/test_elf_loader.cpp @@ -510,6 +510,70 @@ TEST_CASE("ELF loader ignores non-function .ksyms entries", "[elf]") { REQUIRE(progs.size() == 1); } +// Regression test: rewrite_extern_constant_load must not crash when the resolved value +// exceeds INT32_MAX and the load instruction is 8-byte width (LDX DW). +// BPF MOV imm only has a 32-bit immediate; values that don't fit must cause a +// graceful bail-out (return false), not a gsl::narrowing_error exception. +TEST_CASE("rewrite_extern_constant_load bails out on values exceeding int32 range", "[elf][hardening]") { + // Build a 3-instruction sequence: LDDW pair + LDX DW r2, [r1+0]. + // This is the pattern clang emits for: extern uint64_t LINUX_KERNEL_VERSION; + auto make_instructions = []() { + std::vector insts(3); + insts[0].opcode = INST_OP_LDDW_IMM; + insts[0].dst = 1; + insts[0].src = 0; + insts[0].offset = 0; + insts[0].imm = 0; + insts[1] = {}; + // LDX DW = INST_CLS_LDX | INST_MODE_MEM | INST_SIZE_DW = 0x79 + insts[2].opcode = static_cast(INST_CLS_LDX | INST_MODE_MEM | INST_SIZE_DW); + insts[2].dst = 2; + insts[2].src = 1; + insts[2].offset = 0; + insts[2].imm = 0; + return insts; + }; + + SECTION("small value fits in int32 — rewrite succeeds") { + auto insts = make_instructions(); + REQUIRE(rewrite_extern_constant_load(insts, 0, 42)); + } + + SECTION("INT32_MAX fits — rewrite succeeds") { + auto insts = make_instructions(); + REQUIRE(rewrite_extern_constant_load(insts, 0, 0x7FFFFFFF)); + } + + SECTION("0x80000000 exceeds int32 — returns false without throwing") { + auto insts = make_instructions(); + CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0x80000000ULL)); + // Verify the function returns false (bail out, don't rewrite). + auto insts2 = make_instructions(); + CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0x80000000ULL)); + } + + SECTION("0x100000000 exceeds int32 — returns false without throwing") { + auto insts = make_instructions(); + CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0x100000000ULL)); + auto insts2 = make_instructions(); + CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0x100000000ULL)); + } + + SECTION("0xFFFFFFFF exceeds int32 as uint64 — returns false without throwing") { + auto insts = make_instructions(); + CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0xFFFFFFFFULL)); + auto insts2 = make_instructions(); + CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0xFFFFFFFFULL)); + } + + SECTION("large 64-bit value — returns false without throwing") { + auto insts = make_instructions(); + CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0xDEADBEEFCAFEBABEULL)); + auto insts2 = make_instructions(); + CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0xDEADBEEFCAFEBABEULL)); + } +} + // Regression test: read_elf(istream, path) must work when path is not a real file. // The load_elf function uses file_size(path) for section-bounds validation, which // fails for non-file paths like "memory". The fix falls back to stream size. From 3aa4fabf2c9e6c878534648184591330d799860c Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Fri, 13 Mar 2026 12:57:38 -0700 Subject: [PATCH 2/3] feedback: sign-extension round-trip check and assert non-mutation Signed-off-by: Michael Agun --- src/io/elf_extern_resolve.cpp | 12 +++++------ src/test/test_elf_loader.cpp | 38 ++++++++++++++++------------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/io/elf_extern_resolve.cpp b/src/io/elf_extern_resolve.cpp index 76d459073..295aa68fd 100644 --- a/src/io/elf_extern_resolve.cpp +++ b/src/io/elf_extern_resolve.cpp @@ -1,7 +1,6 @@ // Copyright (c) Prevail Verifier contributors. // SPDX-License-Identifier: MIT #include -#include #include #include #include @@ -86,10 +85,11 @@ bool rewrite_extern_constant_load(std::vector& instructions, const siz } // BPF MOV imm has a 32-bit immediate field that is sign-extended to 64 bits - // by the runtime. Bail out if the value cannot be represented this way; - // the caller will fall back to the original LDDW+LDX instruction sequence. - const auto signed_value = static_cast(narrowed_value); - if (signed_value > std::numeric_limits::max() || signed_value < std::numeric_limits::min()) { + // by the runtime. Bail out if the value cannot survive the int32 → int64 + // sign-extension round-trip; the caller will fall back to the original + // LDDW+LDX instruction sequence. + const auto truncated = static_cast(narrowed_value); + if (static_cast(static_cast(truncated)) != narrowed_value) { return false; } @@ -101,7 +101,7 @@ bool rewrite_extern_constant_load(std::vector& instructions, const siz load_inst.opcode = mov_opcode; load_inst.src = 0; load_inst.offset = 0; - load_inst.imm = static_cast(narrowed_value); + load_inst.imm = truncated; lo_inst.get() = make_mov_reg_nop(lo_inst.get().dst); hi_inst.get() = make_mov_reg_nop(hi_inst.get().dst); diff --git a/src/test/test_elf_loader.cpp b/src/test/test_elf_loader.cpp index b1754945d..d17574f82 100644 --- a/src/test/test_elf_loader.cpp +++ b/src/test/test_elf_loader.cpp @@ -534,6 +534,15 @@ TEST_CASE("rewrite_extern_constant_load bails out on values exceeding int32 rang return insts; }; + // Verify bail-out returns false, doesn't throw, and leaves instructions unmodified. + auto check_bailout_preserves_program = [&](const uint64_t value) { + const auto original = make_instructions(); + auto insts = original; + CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, value)); + CHECK_FALSE(rewrite_extern_constant_load(insts, 0, value)); + CHECK(insts == original); + }; + SECTION("small value fits in int32 — rewrite succeeds") { auto insts = make_instructions(); REQUIRE(rewrite_extern_constant_load(insts, 0, 42)); @@ -544,33 +553,20 @@ TEST_CASE("rewrite_extern_constant_load bails out on values exceeding int32 rang REQUIRE(rewrite_extern_constant_load(insts, 0, 0x7FFFFFFF)); } - SECTION("0x80000000 exceeds int32 — returns false without throwing") { - auto insts = make_instructions(); - CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0x80000000ULL)); - // Verify the function returns false (bail out, don't rewrite). - auto insts2 = make_instructions(); - CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0x80000000ULL)); + SECTION("0x80000000 exceeds int32 — bails out without mutation") { + check_bailout_preserves_program(0x80000000ULL); } - SECTION("0x100000000 exceeds int32 — returns false without throwing") { - auto insts = make_instructions(); - CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0x100000000ULL)); - auto insts2 = make_instructions(); - CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0x100000000ULL)); + SECTION("0x100000000 exceeds int32 — bails out without mutation") { + check_bailout_preserves_program(0x100000000ULL); } - SECTION("0xFFFFFFFF exceeds int32 as uint64 — returns false without throwing") { - auto insts = make_instructions(); - CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0xFFFFFFFFULL)); - auto insts2 = make_instructions(); - CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0xFFFFFFFFULL)); + SECTION("0xFFFFFFFF exceeds int32 as uint64 — bails out without mutation") { + check_bailout_preserves_program(0xFFFFFFFFULL); } - SECTION("large 64-bit value — returns false without throwing") { - auto insts = make_instructions(); - CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, 0xDEADBEEFCAFEBABEULL)); - auto insts2 = make_instructions(); - CHECK_FALSE(rewrite_extern_constant_load(insts2, 0, 0xDEADBEEFCAFEBABEULL)); + SECTION("large 64-bit value — bails out without mutation") { + check_bailout_preserves_program(0xDEADBEEFCAFEBABEULL); } } From 2823dab0624503d80e0c049ee6b6b3e469d23648 Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Fri, 13 Mar 2026 15:36:17 -0700 Subject: [PATCH 3/3] review: single invocation in bail-out helper, add -1 sign-extension success case Signed-off-by: Michael Agun --- src/test/test_elf_loader.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/test_elf_loader.cpp b/src/test/test_elf_loader.cpp index d17574f82..e145f7e93 100644 --- a/src/test/test_elf_loader.cpp +++ b/src/test/test_elf_loader.cpp @@ -538,8 +538,9 @@ TEST_CASE("rewrite_extern_constant_load bails out on values exceeding int32 rang auto check_bailout_preserves_program = [&](const uint64_t value) { const auto original = make_instructions(); auto insts = original; - CHECK_NOTHROW(rewrite_extern_constant_load(insts, 0, value)); - CHECK_FALSE(rewrite_extern_constant_load(insts, 0, value)); + bool result = true; + CHECK_NOTHROW(result = rewrite_extern_constant_load(insts, 0, value)); + CHECK_FALSE(result); CHECK(insts == original); }; @@ -553,6 +554,11 @@ TEST_CASE("rewrite_extern_constant_load bails out on values exceeding int32 rang REQUIRE(rewrite_extern_constant_load(insts, 0, 0x7FFFFFFF)); } + SECTION("-1 (0xFFFFFFFFFFFFFFFF) fits via sign-extension — rewrite succeeds") { + auto insts = make_instructions(); + REQUIRE(rewrite_extern_constant_load(insts, 0, 0xFFFFFFFFFFFFFFFFULL)); + } + SECTION("0x80000000 exceeds int32 — bails out without mutation") { check_bailout_preserves_program(0x80000000ULL); }