Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/io/elf_extern_resolve.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Prevail Verifier contributors.
// SPDX-License-Identifier: MIT
#include <cstdint>
#include <optional>
#include <string_view>
#include <vector>
Expand Down Expand Up @@ -83,6 +84,15 @@ bool rewrite_extern_constant_load(std::vector<EbpfInst>& instructions, const siz
narrowed_value = static_cast<uint64_t>((static_cast<int64_t>(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 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<int32_t>(narrowed_value);
if (static_cast<uint64_t>(static_cast<int64_t>(truncated)) != narrowed_value) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add an appropriate abstraction to num_safety.hpp?

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
Expand All @@ -91,7 +101,7 @@ bool rewrite_extern_constant_load(std::vector<EbpfInst>& instructions, const siz
load_inst.opcode = mov_opcode;
load_inst.src = 0;
load_inst.offset = 0;
load_inst.imm = gsl::narrow<int32_t>(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);
Expand Down
60 changes: 60 additions & 0 deletions src/test/test_elf_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,66 @@ 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<EbpfInst> 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<uint8_t>(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;
};

// 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));
}

SECTION("INT32_MAX fits — rewrite succeeds") {
auto insts = make_instructions();
REQUIRE(rewrite_extern_constant_load(insts, 0, 0x7FFFFFFF));
}

SECTION("0x80000000 exceeds int32 — bails out without mutation") {
check_bailout_preserves_program(0x80000000ULL);
}

SECTION("0x100000000 exceeds int32 — bails out without mutation") {
check_bailout_preserves_program(0x100000000ULL);
}

SECTION("0xFFFFFFFF exceeds int32 as uint64 — bails out without mutation") {
check_bailout_preserves_program(0xFFFFFFFFULL);
}

SECTION("large 64-bit value — bails out without mutation") {
check_bailout_preserves_program(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.
Expand Down
Loading