Skip to content

Commit 78ff617

Browse files
pratlucasvhscampos
andauthored
[ARM] CMSE security mitigation on function arguments and returned values (llvm#89944)
The ABI mandates two things related to function calls: - Function arguments must be sign- or zero-extended to the register size by the caller. - Return values must be sign- or zero-extended to the register size by the callee. As consequence, callees can assume that function arguments have been extended and so can callers with regards to return values. Here lies the problem: Nonsecure code might deliberately ignore this mandate with the intent of attempting an exploit. It might try to pass values that lie outside the expected type's value range in order to trigger undefined behaviour, e.g. out of bounds access. With the mitigation implemented, Secure code always performs extension of values passed by Nonsecure code. This addresses the vulnerability described in CVE-2024-0151. Patches by Victor Campos. --------- Co-authored-by: Victor Campos <[email protected]>
1 parent d594d9f commit 78ff617

File tree

4 files changed

+953
-15
lines changed

4 files changed

+953
-15
lines changed

llvm/lib/Target/ARM/ARMISelLowering.cpp

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,17 @@ static const MCPhysReg GPRArgRegs[] = {
156156
ARM::R0, ARM::R1, ARM::R2, ARM::R3
157157
};
158158

159+
static SDValue handleCMSEValue(const SDValue &Value, const ISD::InputArg &Arg,
160+
SelectionDAG &DAG, const SDLoc &DL) {
161+
assert(Arg.ArgVT.isScalarInteger());
162+
assert(Arg.ArgVT.bitsLT(MVT::i32));
163+
SDValue Trunc = DAG.getNode(ISD::TRUNCATE, DL, Arg.ArgVT, Value);
164+
SDValue Ext =
165+
DAG.getNode(Arg.Flags.isSExt() ? ISD::SIGN_EXTEND : ISD::ZERO_EXTEND, DL,
166+
MVT::i32, Trunc);
167+
return Ext;
168+
}
169+
159170
void ARMTargetLowering::addTypeForNEON(MVT VT, MVT PromotedLdStVT) {
160171
if (VT != PromotedLdStVT) {
161172
setOperationAction(ISD::LOAD, VT, Promote);
@@ -2193,7 +2204,7 @@ SDValue ARMTargetLowering::LowerCallResult(
21932204
SDValue Chain, SDValue InGlue, CallingConv::ID CallConv, bool isVarArg,
21942205
const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl,
21952206
SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals, bool isThisReturn,
2196-
SDValue ThisVal) const {
2207+
SDValue ThisVal, bool isCmseNSCall) const {
21972208
// Assign locations to each value returned by this call.
21982209
SmallVector<CCValAssign, 16> RVLocs;
21992210
CCState CCInfo(CallConv, isVarArg, DAG.getMachineFunction(), RVLocs,
@@ -2271,6 +2282,15 @@ SDValue ARMTargetLowering::LowerCallResult(
22712282
(VA.getValVT() == MVT::f16 || VA.getValVT() == MVT::bf16))
22722283
Val = MoveToHPR(dl, DAG, VA.getLocVT(), VA.getValVT(), Val);
22732284

2285+
// On CMSE Non-secure Calls, call results (returned values) whose bitwidth
2286+
// is less than 32 bits must be sign- or zero-extended after the call for
2287+
// security reasons. Although the ABI mandates an extension done by the
2288+
// callee, the latter cannot be trusted to follow the rules of the ABI.
2289+
const ISD::InputArg &Arg = Ins[VA.getValNo()];
2290+
if (isCmseNSCall && Arg.ArgVT.isScalarInteger() &&
2291+
VA.getLocVT().isScalarInteger() && Arg.ArgVT.bitsLT(MVT::i32))
2292+
Val = handleCMSEValue(Val, Arg, DAG, dl);
2293+
22742294
InVals.push_back(Val);
22752295
}
22762296

@@ -2882,7 +2902,7 @@ ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
28822902
// return.
28832903
return LowerCallResult(Chain, InGlue, CallConv, isVarArg, Ins, dl, DAG,
28842904
InVals, isThisReturn,
2885-
isThisReturn ? OutVals[0] : SDValue());
2905+
isThisReturn ? OutVals[0] : SDValue(), isCmseNSCall);
28862906
}
28872907

28882908
/// HandleByVal - Every parameter *after* a byval parameter is passed
@@ -4485,8 +4505,6 @@ SDValue ARMTargetLowering::LowerFormalArguments(
44854505
*DAG.getContext());
44864506
CCInfo.AnalyzeFormalArguments(Ins, CCAssignFnForCall(CallConv, isVarArg));
44874507

4488-
SmallVector<SDValue, 16> ArgValues;
4489-
SDValue ArgValue;
44904508
Function::const_arg_iterator CurOrigArg = MF.getFunction().arg_begin();
44914509
unsigned CurArgIdx = 0;
44924510

@@ -4541,6 +4559,7 @@ SDValue ARMTargetLowering::LowerFormalArguments(
45414559
// Arguments stored in registers.
45424560
if (VA.isRegLoc()) {
45434561
EVT RegVT = VA.getLocVT();
4562+
SDValue ArgValue;
45444563

45454564
if (VA.needsCustom() && VA.getLocVT() == MVT::v2f64) {
45464565
// f64 and vector types are split up into multiple registers or
@@ -4604,16 +4623,6 @@ SDValue ARMTargetLowering::LowerFormalArguments(
46044623
case CCValAssign::BCvt:
46054624
ArgValue = DAG.getNode(ISD::BITCAST, dl, VA.getValVT(), ArgValue);
46064625
break;
4607-
case CCValAssign::SExt:
4608-
ArgValue = DAG.getNode(ISD::AssertSext, dl, RegVT, ArgValue,
4609-
DAG.getValueType(VA.getValVT()));
4610-
ArgValue = DAG.getNode(ISD::TRUNCATE, dl, VA.getValVT(), ArgValue);
4611-
break;
4612-
case CCValAssign::ZExt:
4613-
ArgValue = DAG.getNode(ISD::AssertZext, dl, RegVT, ArgValue,
4614-
DAG.getValueType(VA.getValVT()));
4615-
ArgValue = DAG.getNode(ISD::TRUNCATE, dl, VA.getValVT(), ArgValue);
4616-
break;
46174626
}
46184627

46194628
// f16 arguments have their size extended to 4 bytes and passed as if they
@@ -4623,6 +4632,15 @@ SDValue ARMTargetLowering::LowerFormalArguments(
46234632
(VA.getValVT() == MVT::f16 || VA.getValVT() == MVT::bf16))
46244633
ArgValue = MoveToHPR(dl, DAG, VA.getLocVT(), VA.getValVT(), ArgValue);
46254634

4635+
// On CMSE Entry Functions, formal integer arguments whose bitwidth is
4636+
// less than 32 bits must be sign- or zero-extended in the callee for
4637+
// security reasons. Although the ABI mandates an extension done by the
4638+
// caller, the latter cannot be trusted to follow the rules of the ABI.
4639+
const ISD::InputArg &Arg = Ins[VA.getValNo()];
4640+
if (AFI->isCmseNSEntryFunction() && Arg.ArgVT.isScalarInteger() &&
4641+
RegVT.isScalarInteger() && Arg.ArgVT.bitsLT(MVT::i32))
4642+
ArgValue = handleCMSEValue(ArgValue, Arg, DAG, dl);
4643+
46264644
InVals.push_back(ArgValue);
46274645
} else { // VA.isRegLoc()
46284646
// Only arguments passed on the stack should make it here.

llvm/lib/Target/ARM/ARMISelLowering.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ class VectorType;
894894
const SmallVectorImpl<ISD::InputArg> &Ins,
895895
const SDLoc &dl, SelectionDAG &DAG,
896896
SmallVectorImpl<SDValue> &InVals, bool isThisReturn,
897-
SDValue ThisVal) const;
897+
SDValue ThisVal, bool isCmseNSCall) const;
898898

899899
bool supportSplitCSR(MachineFunction *MF) const override {
900900
return MF->getFunction().getCallingConv() == CallingConv::CXX_FAST_TLS &&

0 commit comments

Comments
 (0)