r354643 - [analyzer] MIGChecker: Add support for more APIs.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 21 16:15:14 PST 2019
Author: dergachev
Date: Thu Feb 21 16:15:14 2019
New Revision: 354643
URL: http://llvm.org/viewvc/llvm-project?rev=354643&view=rev
Log:
[analyzer] MIGChecker: Add support for more APIs.
Add more "consuming" functions. For now only vm_deallocate() was supported.
Add a non-zero value that isn't an error; this value is -305 ("MIG_NO_REPLY")
and it's fine to deallocate data when you are returning this error.
Make sure that the mig_server_routine annotation is inherited.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/D58397
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
cfe/trunk/test/Analysis/mig.mm
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp?rev=354643&r1=354642&r2=354643&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp Thu Feb 21 16:15:14 2019
@@ -8,13 +8,14 @@
//
// This file defines MIGChecker, a Mach Interface Generator calling convention
// checker. Namely, in MIG callback implementation the following rules apply:
-// - When a server routine returns KERN_SUCCESS, it must take ownership of
-// resources (and eventually release them).
-// - Additionally, when returning KERN_SUCCESS, all out-parameters must be
+// - When a server routine returns an error code that represents success, it
+// must take ownership of resources passed to it (and eventually release
+// them).
+// - Additionally, when returning success, all out-parameters must be
// initialized.
-// - When it returns anything except KERN_SUCCESS it must not take ownership,
-// because the message and its descriptors will be destroyed by the server
-// function.
+// - When it returns any other error code, it must not take ownership,
+// because the message and its out-of-line parameters will be destroyed
+// by the client that called the function.
// For now we only check the last rule, as its violations lead to dangerous
// use-after-free exploits.
//
@@ -37,7 +38,29 @@ class MIGChecker : public Checker<check:
BugType BT{this, "Use-after-free (MIG calling convention violation)",
categories::MemoryError};
- CallDescription vm_deallocate { "vm_deallocate", 3 };
+ // The checker knows that an out-of-line object is deallocated if it is
+ // passed as an argument to one of these functions. If this object is
+ // additionally an argument of a MIG routine, the checker keeps track of that
+ // information and issues a warning when an error is returned from the
+ // respective routine.
+ std::vector<std::pair<CallDescription, unsigned>> Deallocators = {
+#define CALL(required_args, deallocated_arg, ...) \
+ {{{__VA_ARGS__}, required_args}, deallocated_arg}
+ // E.g., if the checker sees a C function 'vm_deallocate' that is
+ // defined on class 'IOUserClient' that has exactly 3 parameters, it knows
+ // that argument #1 (starting from 0, i.e. the second argument) is going
+ // to be consumed in the sense of the MIG consume-on-success convention.
+ CALL(3, 1, "vm_deallocate"),
+ CALL(3, 1, "mach_vm_deallocate"),
+ CALL(2, 0, "mig_deallocate"),
+ CALL(2, 1, "mach_port_deallocate"),
+ // E.g., if the checker sees a method 'releaseAsyncReference64()' that is
+ // defined on class 'IOUserClient' that takes exactly 1 argument, it knows
+ // that the argument is going to be consumed in the sense of the MIG
+ // consume-on-success convention.
+ CALL(1, 0, "IOUserClient", "releaseAsyncReference64"),
+#undef CALL
+ };
void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const;
@@ -100,10 +123,23 @@ static const ParmVarDecl *getOriginParam
if (!Sym)
return nullptr;
- const auto *VR = dyn_cast_or_null<VarRegion>(Sym->getOriginRegion());
- if (VR && VR->hasStackParametersStorage() &&
- VR->getStackFrame()->inTopFrame())
- return cast<ParmVarDecl>(VR->getDecl());
+ // If we optimistically assume that the MIG routine never re-uses the storage
+ // that was passed to it as arguments when it invalidates it (but at most when
+ // it assigns to parameter variables directly), this procedure correctly
+ // determines if the value was loaded from the transitive closure of MIG
+ // routine arguments in the heap.
+ while (const MemRegion *MR = Sym->getOriginRegion()) {
+ const auto *VR = dyn_cast<VarRegion>(MR);
+ if (VR && VR->hasStackParametersStorage() &&
+ VR->getStackFrame()->inTopFrame())
+ return cast<ParmVarDecl>(VR->getDecl());
+
+ const SymbolicRegion *SR = MR->getSymbolicBase();
+ if (!SR)
+ return nullptr;
+
+ Sym = SR->getSymbol();
+ }
return nullptr;
}
@@ -133,6 +169,12 @@ static bool isInMIGCall(CheckerContext &
if (D->hasAttr<MIGServerRoutineAttr>())
return true;
+ // See if there's an annotated method in the superclass.
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+ for (const auto *OMD: MD->overridden_methods())
+ if (OMD->hasAttr<MIGServerRoutineAttr>())
+ return true;
+
return false;
}
@@ -140,14 +182,15 @@ void MIGChecker::checkPostCall(const Cal
if (!isInMIGCall(C))
return;
- if (!Call.isGlobalCFunction())
+ auto I = std::find_if(Deallocators.begin(), Deallocators.end(),
+ [&](const std::pair<CallDescription, unsigned> &Item) {
+ return Call.isCalled(Item.first);
+ });
+ if (I == Deallocators.end())
return;
- if (!Call.isCalled(vm_deallocate))
- return;
-
- // TODO: Unhardcode "1".
- SVal Arg = Call.getArgSVal(1);
+ unsigned ArgIdx = I->second;
+ SVal Arg = Call.getArgSVal(ArgIdx);
const ParmVarDecl *PVD = getOriginParam(Arg, C);
if (!PVD)
return;
@@ -155,6 +198,27 @@ void MIGChecker::checkPostCall(const Cal
C.addTransition(C.getState()->set<ReleasedParameter>(PVD));
}
+// Returns true if V can potentially represent a "successful" kern_return_t.
+static bool mayBeSuccess(SVal V, CheckerContext &C) {
+ ProgramStateRef State = C.getState();
+
+ // Can V represent KERN_SUCCESS?
+ if (!State->isNull(V).isConstrainedFalse())
+ return true;
+
+ SValBuilder &SVB = C.getSValBuilder();
+ ASTContext &ACtx = C.getASTContext();
+
+ // Can V represent MIG_NO_REPLY?
+ static const int MigNoReply = -305;
+ V = SVB.evalEQ(C.getState(), V, SVB.makeIntVal(MigNoReply, ACtx.IntTy));
+ if (!State->isNull(V).isConstrainedTrue())
+ return true;
+
+ // If none of the above, it's definitely an error.
+ return false;
+}
+
void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const {
// It is very unlikely that a MIG callback will be called from anywhere
// within the project under analysis and the caller isn't itself a routine
@@ -180,7 +244,7 @@ void MIGChecker::checkReturnAux(const Re
return;
SVal V = C.getSVal(RS);
- if (!State->isNonNull(V).isConstrainedTrue())
+ if (mayBeSuccess(V, C))
return;
ExplodedNode *N = C.generateErrorNode();
Modified: cfe/trunk/test/Analysis/mig.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/mig.mm?rev=354643&r1=354642&r2=354643&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/mig.mm (original)
+++ cfe/trunk/test/Analysis/mig.mm Thu Feb 21 16:15:14 2019
@@ -1,20 +1,55 @@
// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
// RUN: -analyzer-output=text -fblocks -verify %s
+typedef unsigned uint32_t;
+
// XNU APIs.
typedef int kern_return_t;
#define KERN_SUCCESS 0
#define KERN_ERROR 1
+#define MIG_NO_REPLY (-305)
typedef unsigned mach_port_name_t;
typedef unsigned vm_address_t;
typedef unsigned vm_size_t;
+typedef void *ipc_space_t;
+typedef unsigned long io_user_reference_t;
kern_return_t vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
+kern_return_t mach_vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
+void mig_deallocate(vm_address_t, vm_size_t);
+kern_return_t mach_port_deallocate(ipc_space_t, mach_port_name_t);
#define MIG_SERVER_ROUTINE __attribute__((mig_server_routine))
+// IOKit wrappers.
+
+class OSObject;
+typedef kern_return_t IOReturn;
+#define kIOReturnError 1
+
+enum {
+ kOSAsyncRef64Count = 8,
+};
+
+typedef io_user_reference_t OSAsyncReference64[kOSAsyncRef64Count];
+
+struct IOExternalMethodArguments {
+ io_user_reference_t *asyncReference;
+};
+
+struct IOExternalMethodDispatch {};
+
+class IOUserClient {
+public:
+ static IOReturn releaseAsyncReference64(OSAsyncReference64);
+
+ MIG_SERVER_ROUTINE
+ virtual IOReturn externalMethod(uint32_t selector, IOExternalMethodArguments *arguments,
+ IOExternalMethodDispatch *dispatch = 0, OSObject *target = 0, void *reference = 0);
+};
+
// Tests.
@@ -123,3 +158,52 @@ void test_block_with_weird_return_type()
return Empty{}; // no-crash
};
}
+
+// Test various APIs.
+MIG_SERVER_ROUTINE
+kern_return_t test_mach_vm_deallocate(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+ mach_vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
+ return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value}}
+}
+
+MIG_SERVER_ROUTINE
+kern_return_t test_mach_port_deallocate(ipc_space_t space,
+ mach_port_name_t port) {
+ mach_port_deallocate(space, port); // expected-note{{Value passed through parameter 'port' is deallocated}}
+ return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value}}
+}
+
+MIG_SERVER_ROUTINE
+kern_return_t test_mig_deallocate(vm_address_t address, vm_size_t size) {
+ mig_deallocate(address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
+ return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value}}
+}
+
+// Let's try the C++11 attribute spelling syntax as well.
+[[clang::mig_server_routine]]
+IOReturn test_releaseAsyncReference64(IOExternalMethodArguments *arguments) {
+ IOUserClient::releaseAsyncReference64(arguments->asyncReference); // expected-note{{Value passed through parameter 'arguments' is deallocated}}
+ return kIOReturnError; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value}}
+}
+
+MIG_SERVER_ROUTINE
+kern_return_t test_no_reply(ipc_space_t space, mach_port_name_t port) {
+ mach_port_deallocate(space, port);
+ return MIG_NO_REPLY; // no-warning
+}
+
+class MyClient: public IOUserClient {
+ // The MIG_SERVER_ROUTINE annotation is intentionally skipped.
+ // It should be picked up from the superclass.
+ IOReturn externalMethod(uint32_t selector, IOExternalMethodArguments *arguments,
+ IOExternalMethodDispatch *dispatch = 0, OSObject *target = 0, void *reference = 0) override {
+
+ releaseAsyncReference64(arguments->asyncReference); // expected-note{{Value passed through parameter 'arguments' is deallocated}}
+ return kIOReturnError; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value}}
+ }
+};
More information about the cfe-commits
mailing list