From: Chris Lattner Date: Tue, 20 Jul 2010 20:19:24 +0000 (+0000) Subject: implement rdar://5739832 - operator new should check for overflow in multiply, X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6c552c1d5f47fbba00e6268d96a26ad026f2da2a;p=clang implement rdar://5739832 - operator new should check for overflow in multiply, causing clang to compile this code into something that correctly throws a length error, fixing a potential integer overflow security attack: void *test(long N) { return new int[N]; } int main() { test(1L << 62); } We do this even when exceptions are disabled, because it is better for the code to abort than for the attack to succeed. This is heavily based on a patch that Fariborz wrote. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@108915 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index fa5ac8fb14..d6a34562e5 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -1315,10 +1315,9 @@ llvm::BasicBlock *CodeGenFunction::getTrapBB() { // If we are not optimzing, don't collapse all calls to trap in the function // to the same call, that way, in the debugger they can see which operation - // did in fact fail. If we are optimizing, we collpase all call to trap down + // did in fact fail. If we are optimizing, we collapse all calls to trap down // to just one per function to save on codesize. - if (GCO.OptimizationLevel - && TrapBB) + if (GCO.OptimizationLevel && TrapBB) return TrapBB; llvm::BasicBlock *Cont = 0; diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index f9c4b30d0b..cc34b3d75c 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -13,6 +13,7 @@ #include "CodeGenFunction.h" #include "CGObjCRuntime.h" +#include "llvm/Intrinsics.h" using namespace clang; using namespace CodeGen; @@ -435,11 +436,26 @@ static llvm::Value *EmitCXXNewAllocSize(ASTContext &Context, // Emit the array size expression. NumElements = CGF.EmitScalarExpr(E->getArraySize()); - // Multiply with the type size. - llvm::Value *V = - CGF.Builder.CreateMul(NumElements, - llvm::ConstantInt::get(SizeTy, - TypeSize.getQuantity())); + // Multiply with the type size. This multiply can overflow, e.g. in: + // new double[n] + // where n is 2^30 on a 32-bit machine or 2^62 on a 64-bit machine. Because + // of this, we need to detect the overflow and ensure that an exception is + // called by invoking std::__throw_length_error. + llvm::Value *UMulF = CGF.CGM.getIntrinsic(llvm::Intrinsic::umul_with_overflow, + &SizeTy, 1); + llvm::Value *MulRes = CGF.Builder.CreateCall2(UMulF, NumElements, + llvm::ConstantInt::get(SizeTy, + TypeSize.getQuantity())); + // Branch on the overflow bit to the overflow block, which is lazily created. + llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(MulRes, 1); + + llvm::BasicBlock *NormalBB = CGF.createBasicBlock("no_overflow"); + + CGF.Builder.CreateCondBr(DidOverflow, CGF.getThrowLengthErrorBB(), NormalBB); + CGF.EmitBlock(NormalBB); + + // Get the normal result of the multiplication. + llvm::Value *V = CGF.Builder.CreateExtractValue(MulRes, 0); // And add the cookie padding if necessary. if (!CookiePadding.isZero()) diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp index eb6c4361be..5e5e2a49cc 100644 --- a/lib/CodeGen/CodeGenFunction.cpp +++ b/lib/CodeGen/CodeGenFunction.cpp @@ -36,7 +36,7 @@ CodeGenFunction::CodeGenFunction(CodeGenModule &cgm) DidCallStackSave(false), UnreachableBlock(0), CXXThisDecl(0), CXXThisValue(0), CXXVTTDecl(0), CXXVTTValue(0), ConditionalBranchLevel(0), TerminateLandingPad(0), TerminateHandler(0), - TrapBB(0) { + TrapBB(0), ThrowLengthErrorBB(0) { // Get some frequently used types. LLVMPointerWidth = Target.getPointerWidth(0); @@ -155,6 +155,13 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) { Builder.ClearInsertionPoint(); } + // If someone called operator new[] and needs a throw_length_error block, emit + // it at the end of the function. + if (ThrowLengthErrorBB) { + EmitBlock(ThrowLengthErrorBB); + Builder.ClearInsertionPoint(); + } + // Remove the AllocaInsertPt instruction, which is just a convenience for us. llvm::Instruction *Ptr = AllocaInsertPt; AllocaInsertPt = 0; @@ -178,6 +185,37 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) { EmitDeclMetadata(); } +/// getThrowLengthErrorBB - Create a basic block that will call +/// std::__throw_length_error to throw a std::length_error exception. +llvm::BasicBlock *CodeGenFunction::getThrowLengthErrorBB() { + if (ThrowLengthErrorBB) return ThrowLengthErrorBB; + + llvm::IRBuilder<>::InsertPoint SavedIP = Builder.saveIP(); + + ThrowLengthErrorBB = createBasicBlock("throw_length_error"); + Builder.SetInsertPoint(ThrowLengthErrorBB); + + // Call to void std::__throw_length_error("length_error"); + const llvm::Type *ResultType = Builder.getVoidTy(); + const llvm::Type *PtrToInt8Ty = Builder.getInt8PtrTy(); + std::vector ArgTys(1, PtrToInt8Ty); + llvm::Constant *Fn = + CGM.CreateRuntimeFunction(llvm::FunctionType::get(ResultType, ArgTys, false), + "_ZSt20__throw_length_errorPKc"); + + llvm::Value *C = CGM.GetAddrOfConstantCString("length_error"); + C = Builder.CreateStructGEP(C, 0, "arraydecay"); + llvm::CallInst *TheCall = Builder.CreateCall(Fn, C); + TheCall->setDoesNotReturn(); + + Builder.CreateUnreachable(); + + + Builder.restoreIP(SavedIP); + return ThrowLengthErrorBB; +} + + /// ShouldInstrumentFunction - Return true if the current function should be /// instrumented with __cyg_profile_func_* calls bool CodeGenFunction::ShouldInstrumentFunction() { diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 778604b217..1a117cab96 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -656,7 +656,7 @@ private: llvm::BasicBlock *TerminateLandingPad; llvm::BasicBlock *TerminateHandler; - llvm::BasicBlock *TrapBB; + llvm::BasicBlock *TrapBB, *ThrowLengthErrorBB; public: CodeGenFunction(CodeGenModule &cgm); @@ -1542,7 +1542,11 @@ public: /// getTrapBB - Create a basic block that will call the trap intrinsic. We'll /// generate a branch around the created basic block as necessary. - llvm::BasicBlock* getTrapBB(); + llvm::BasicBlock *getTrapBB(); + + /// getThrowLengthErrorBB - Create a basic block that will call + /// std::__throw_length_error to throw a std::length_error exception. + llvm::BasicBlock *getThrowLengthErrorBB(); /// EmitCallArg - Emit a single call argument. RValue EmitCallArg(const Expr *E, QualType ArgType); diff --git a/test/CodeGenCXX/operator-new.cpp b/test/CodeGenCXX/operator-new.cpp index f718faebef..f5cb2fb6c5 100644 --- a/test/CodeGenCXX/operator-new.cpp +++ b/test/CodeGenCXX/operator-new.cpp @@ -11,7 +11,21 @@ public: }; void f1() { - // CHECK-SANE: declare noalias i8* @_Znwj( - // CHECK-SANENOT: declare i8* @_Znwj( + // SANE: declare noalias i8* @_Znwj( + // SANENOT: declare i8* @_Znwj( new teste(); } + + +// rdar://5739832 - operator new should check for overflow in multiply. +void *f2(long N) { + return new int[N]; + +// SANE: call{{.*}}@llvm.umul.with.overflow +// SANE: extractvalue +// SANE: br i1{{.*}}, label %throw_length_error, label %no_overflow + +// SANE: throw_length_error: +// SANE: call void @_ZSt20__throw_length_errorPKc +// SANE: unreachable +}