Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -45,14 +45,17 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool); -static bool isCurrentArgSVal(SVal V, CheckerContext &C) { +static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C) { SymbolRef Sym = V.getAsSymbol(); if (!Sym) - return false; + return nullptr; const auto *VR = dyn_cast_or_null(Sym->getOriginRegion()); - return VR && VR->hasStackParametersStorage() && - VR->getStackFrame()->inTopFrame(); + if (VR && VR->hasStackParametersStorage() && + VR->getStackFrame()->inTopFrame()) + return cast(VR->getDecl()); + + return nullptr; } static bool isInMIGCall(const LocationContext *LC) { @@ -86,8 +89,18 @@ // TODO: Unhardcode "1". SVal Arg = Call.getArgSVal(1); - if (isCurrentArgSVal(Arg, C)) - C.addTransition(C.getState()->set(true)); + const ParmVarDecl *PVD = getOriginParam(Arg, C); + if (!PVD) + return; + + const NoteTag *T = C.getNoteTag([PVD]() -> std::string { + SmallString<64> Str; + llvm::raw_svector_ostream OS(Str); + OS << "Deallocating object passed through parameter '" << PVD->getName() + << '\''; + return OS.str(); + }); + C.addTransition(C.getState()->set(true), T); } void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { @@ -129,6 +142,8 @@ "deallocate it again", N); + R->addRange(RS->getSourceRange()); + bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); C.emitReport(std::move(R)); } Index: clang/test/Analysis/mig.cpp =================================================================== --- clang/test/Analysis/mig.cpp +++ clang/test/Analysis/mig.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG -verify %s +// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\ +// RUN: -analyzer-output=text -verify %s + // XNU APIs. @@ -16,9 +18,11 @@ MIG_SERVER_ROUTINE kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) { - vm_deallocate(port, address, size); - if (size > 10) { + vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}} + if (size > 10) { // expected-note{{Assuming 'size' is > 10}} + // expected-note@-1{{Taking true branch}} return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}} + // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}} } return KERN_SUCCESS; } @@ -28,3 +32,15 @@ kern_return_t no_crash(mach_port_name_t port, vm_address_t address, vm_size_t size) { vm_deallocate(port, address, size); } + +// When releasing two parameters, add a note for both of them. +// Also when returning a variable, explain why do we think that it contains +// a non-success code. +MIG_SERVER_ROUTINE +kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_address_t addr2, vm_size_t size) { + kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}} + vm_deallocate(port, addr1, size); // expected-note{{Deallocating object passed through parameter 'addr1'}} + vm_deallocate(port, addr2, size); // expected-note{{Deallocating object passed through parameter 'addr2'}} + return ret; // expected-warning{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}} + // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}} +}