Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 4-byte alignment check on pc in branching instructions #102

Merged
merged 3 commits into from
Jan 15, 2025
Merged
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
5 changes: 5 additions & 0 deletions rvgo/fast/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,11 @@ func (inst *InstrumentedState) riscvStep() (outErr error) {
// So it's really 13 bits with a hardcoded 0 bit.
pc = add64(pc, imm)
}

if pc&3 != 0 { // quick target alignment check
mininny marked this conversation as resolved.
Show resolved Hide resolved
refcell marked this conversation as resolved.
Show resolved Hide resolved
revertWithCode(riscv.ErrNotAlignedAddr, fmt.Errorf("pc %d not aligned with 4 bytes", pc))
}

// not like the other opcodes: nothing to write to rd register, and PC has already changed
setPC(pc)
case 0x13: // 001_0011: immediate arithmetic and logic
Expand Down
5 changes: 5 additions & 0 deletions rvgo/slow/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,11 @@ func Step(calldata []byte, po PreimageOracle) (stateHash common.Hash, outErr err
// So it's really 13 bits with a hardcoded 0 bit.
pc = add64(pc, imm)
}

if and64(pc, toU64(3)) != (U64{}) { // quick target alignment check
revertWithCode(riscv.ErrNotAlignedAddr, fmt.Errorf("pc %d not aligned with 4 bytes", pc))
}

// not like the other opcodes: nothing to write to rd register, and PC has already changed
setPC(pc)
case 0x13: // 001_0011: immediate arithmetic and logic
Expand Down
6 changes: 6 additions & 0 deletions rvsol/src/RISCV.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,12 @@ contract RISCV is IBigStepper {
// So it's really 13 bits with a hardcoded 0 bit.
_pc := add64(_pc, imm)
}

if and64(_pc, toU64(3)) {
// quick target alignment check
revertWithCode(0xbad10ad0) // target not aligned with 4 bytes
}

// not like the other opcodes: nothing to write to rd register, and PC has already changed
setPC(_pc)
}
Expand Down
10 changes: 5 additions & 5 deletions rvsol/test/RISCV.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2034,7 +2034,7 @@ contract RISCV_Test is CommonTest {
function test_beq_succeeds() public {
uint16 imm = 0x19cd;
uint32 insn = encodeBType(0x63, 0, 23, 20, imm); // beq x23, x20, offset
(State memory state, bytes memory proof) = constructRISCVState(0x139a, insn);
(State memory state, bytes memory proof) = constructRISCVState(0x139c, insn);
state.registers[23] = 0x2152;
state.registers[20] = 0x2152;
bytes memory encodedState = encodeState(state);
Expand All @@ -2060,7 +2060,7 @@ contract RISCV_Test is CommonTest {
}

function test_bne_succeeds() public {
uint16 imm = 0x1d7e;
uint16 imm = 0x1d7c;
uint32 insn = encodeBType(0x63, 1, 20, 26, imm); // bne x20, x26, offset
(State memory state, bytes memory proof) = constructRISCVState(0x1afc, insn);
state.registers[20] = 0x14b6;
Expand Down Expand Up @@ -2144,9 +2144,9 @@ contract RISCV_Test is CommonTest {
}

function test_bltu_succeeds() public {
uint16 imm = 0x171d;
uint16 imm = 0x171c;
uint32 insn = encodeBType(0x63, 6, 13, 22, imm); // bltu x13, x22, offset
(State memory state, bytes memory proof) = constructRISCVState(0x2e3a, insn);
(State memory state, bytes memory proof) = constructRISCVState(0x2e3c, insn);
state.registers[13] = 0xa0cc;
state.registers[22] = 0xffffffff_ffff795c;
bytes memory encodedState = encodeState(state);
Expand Down Expand Up @@ -2174,7 +2174,7 @@ contract RISCV_Test is CommonTest {
function test_bgeu_succeeds() public {
uint16 imm = 0x14b5;
uint32 insn = encodeBType(0x63, 7, 7, 16, imm); // bgeu x7, x16, offset
(State memory state, bytes memory proof) = constructRISCVState(0x296a, insn);
(State memory state, bytes memory proof) = constructRISCVState(0x296c, insn);
state.registers[7] = 0xffffffff_ffff35e5;
state.registers[16] = 0x7c3c;
bytes memory encodedState = encodeState(state);
Expand Down
Loading