Skip to content

Commit 33411d5

Browse files
authored
[ARM] Fix CMSE S->NS calls when CONTROL_S.SFPA==0 (CVE-2024-7883) (llvm#114433)
When doing a call from CMSE secure state to non-secure state for v8-M.main, we use the VLLDM and VLSTM instructions to save, clear and restore the FP registers around the call. These instructions both check the CONTROL_S.SFPA bit, and if it is clear (meaning the current contents of the FP registers are not secret) they execute as no-ops. This causes a problem when CONTROL_S.SFPA==0 before the call, which happens if there are no floating-point instructions executed between entry to secure state and the call. If this is the case, then the VLSTM instruction will do nothing, leaving the save area in the stack uninitialised. If the called function returns a value in floating-point registers, the call sequence includes an instruction to copy the return value from a floating-point register to a GPR, which must be before the VLLDM instruction. This copy sets CONTROL_S.SFPA, meaning that the VLLDM will fully execute, and load the uninitialised stack memory into the FP registers. This causes two problems: * The FP register file is clobbered, including all of the callee-saved registers, which might contain live values. * The stack region might contain secret values, which will be leaked to non-secure state through the floating-point registers if/when we return to non-secure state. The fix is to insert a `vmov s0, s0` instruction before the VLSTM instruction, to ensure that CONTROL_S.SFPA is set for both the VLLDM and VLSTM instruction. CVE: https://www.cve.org/cverecord?id=CVE-2024-7883 Security bulletin: https://developer.arm.com/Arm%20Security%20Center/Cortex-M%20Security%20Extensions%20Vulnerability
1 parent da083e3 commit 33411d5

File tree

3 files changed

+172
-33
lines changed

3 files changed

+172
-33
lines changed

llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,6 +1426,7 @@ void ARMExpandPseudo::CMSESaveClearFPRegsV8(
14261426
// Use ScratchRegs to store the fp regs
14271427
std::vector<std::tuple<unsigned, unsigned, unsigned>> ClearedFPRegs;
14281428
std::vector<unsigned> NonclearedFPRegs;
1429+
bool ReturnsFPReg = false;
14291430
for (const MachineOperand &Op : MBBI->operands()) {
14301431
if (Op.isReg() && Op.isUse()) {
14311432
Register Reg = Op.getReg();
@@ -1460,14 +1461,51 @@ void ARMExpandPseudo::CMSESaveClearFPRegsV8(
14601461
NonclearedFPRegs.push_back(Reg);
14611462
}
14621463
}
1464+
} else if (Op.isReg() && Op.isDef()) {
1465+
Register Reg = Op.getReg();
1466+
if (ARM::SPRRegClass.contains(Reg) || ARM::DPRRegClass.contains(Reg) ||
1467+
ARM::QPRRegClass.contains(Reg))
1468+
ReturnsFPReg = true;
14631469
}
14641470
}
14651471

1466-
bool passesFPReg = (!NonclearedFPRegs.empty() || !ClearedFPRegs.empty());
1472+
bool PassesFPReg = (!NonclearedFPRegs.empty() || !ClearedFPRegs.empty());
14671473

1468-
if (passesFPReg)
1474+
if (PassesFPReg || ReturnsFPReg)
14691475
assert(STI->hasFPRegs() && "Subtarget needs fpregs");
14701476

1477+
// CVE-2024-7883
1478+
//
1479+
// The VLLDM/VLSTM instructions set up lazy state preservation, but they
1480+
// execute as NOPs if the FP register file is not considered to contain
1481+
// secure data, represented by the CONTROL_S.SFPA bit. This means that the
1482+
// state of CONTROL_S.SFPA must be the same when these two instructions are
1483+
// executed. That might not be the case if we haven't used any FP
1484+
// instructions before the VLSTM, so CONTROL_S.SFPA is clear, but do have one
1485+
// before the VLLDM, which sets it..
1486+
//
1487+
// If we can't prove that SFPA will be the same for the VLSTM and VLLDM, we
1488+
// execute a "vmov s0, s0" instruction before the VLSTM to ensure that
1489+
// CONTROL_S.SFPA is set for both.
1490+
//
1491+
// That can only happen for callees which take no FP arguments (or we'd have
1492+
// inserted a VMOV above) and which return values in FP regs (so that we need
1493+
// to use a VMOV to back-up the return value before the VLLDM). It also can't
1494+
// happen if the call is dominated by other existing floating-point
1495+
// instructions, but we don't currently check for that case.
1496+
//
1497+
// These conditions mean that we only emit this instruction when using the
1498+
// hard-float ABI, which means we can assume that FP instructions are
1499+
// available, and don't need to make it conditional like we do for the
1500+
// CVE-2021-35465 workaround.
1501+
if (ReturnsFPReg && !PassesFPReg) {
1502+
bool S0Dead = !LiveRegs.contains(ARM::S0);
1503+
BuildMI(MBB, MBBI, DL, TII->get(ARM::VMOVS))
1504+
.addReg(ARM::S0, RegState::Define | getDeadRegState(S0Dead))
1505+
.addReg(ARM::S0, getUndefRegState(S0Dead))
1506+
.add(predOps(ARMCC::AL));
1507+
}
1508+
14711509
// Lazy store all fp registers to the stack.
14721510
// This executes as NOP in the absence of floating-point support.
14731511
MachineInstrBuilder VLSTM =
@@ -1528,7 +1566,7 @@ void ARMExpandPseudo::CMSESaveClearFPRegsV8(
15281566
}
15291567
// restore FPSCR from stack and clear bits 0-4, 7, 28-31
15301568
// The other bits are program global according to the AAPCS
1531-
if (passesFPReg) {
1569+
if (PassesFPReg) {
15321570
BuildMI(MBB, MBBI, DL, TII->get(ARM::tLDRspi), SpareReg)
15331571
.addReg(ARM::SP)
15341572
.addImm(0x10)

0 commit comments

Comments
 (0)