search

microsoft / cheriot-ibex

cheriot-ibex is a RTL implementation of CHERIoT ISA based on LowRISC's Ibex core.
Apache License 2.0
73 stars 14 forks source link

CJALR with instr[14:12] != 0 is not illegal #43

Closed mndstrmr closed 3 weeks ago

mndstrmr commented 1 month ago

The decoder for CJALR will not raise an illegal instruction exception when instr[14:12] != 0, but it will also not set cheri_jalr_en:

https://github.com/microsoft/cheriot-ibex/blob/3f0ec8600cb7621c4c952951d211b3515116ea27/rtl/ibex_decoder.sv#L343-L367

When a CJALR instruction is run with instr[14:12] != 0, the instruction that runs becomes one which stores the next instruction PC to the register file, but does not branch, since it runs into the old logic for JALR. This is therefore not a security issue, but is a bug.

Instead illegal_insn should be set when instr[14:12] != 0 (see the original code for JALR, which does the same).

marnovandermaas commented 1 month ago

I discussed this with Louis-Emile and agree with this conclusion. I think the funct3 not equal to zero behaviour should be the same as for non-CHERI RISC-V which sets illegal_insn when this is the case: https://github.com/lowRISC/ibex/blob/master/rtl/ibex_decoder.sv#L267-L269

kliuMsft commented 1 month ago

I agree, this is bug indeed. Will fix, thanks.

kliuMsft commented 4 weeks ago

Commit cf1a0f7 should fix the isse, please verify, thanks.

mndstrmr commented 3 weeks ago

This proves, thanks.