Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -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 @@ 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> 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 @@ if (!Sym) return nullptr; - const auto *VR = dyn_cast_or_null(Sym->getOriginRegion()); - if (VR && VR->hasStackParametersStorage() && - VR->getStackFrame()->inTopFrame()) - return cast(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(MR); + if (VR && VR->hasStackParametersStorage() && + VR->getStackFrame()->inTopFrame()) + return cast(VR->getDecl()); + + const SymbolicRegion *SR = MR->getSymbolicBase(); + if (!SR) + return nullptr; + + Sym = SR->getSymbol(); + } return nullptr; } @@ -133,6 +169,12 @@ if (D->hasAttr()) return true; + // See if there's an annotated method in the superclass. + if (const auto *MD = dyn_cast(D)) + for (const auto *OMD: MD->overridden_methods()) + if (OMD->hasAttr()) + return true; + return false; } @@ -140,14 +182,15 @@ if (!isInMIGCall(C)) return; - if (!Call.isGlobalCFunction()) + auto I = std::find_if(Deallocators.begin(), Deallocators.end(), + [&](const std::pair &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 @@ C.addTransition(C.getState()->set(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 @@ return; SVal V = C.getSVal(RS); - if (!State->isNonNull(V).isConstrainedTrue()) + if (mayBeSuccess(V, C)) return; ExplodedNode *N = C.generateErrorNode(); Index: cfe/trunk/test/Analysis/mig.mm =================================================================== --- cfe/trunk/test/Analysis/mig.mm +++ cfe/trunk/test/Analysis/mig.mm @@ -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 @@ 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@-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@-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@-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@-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@-1{{MIG callback fails with error after deallocating argument value}} + } +};