NoQ updated this revision to Diff 187502. NoQ added a comment. Rebase.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58368/new/ https://reviews.llvm.org/D58368 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp clang/test/Analysis/mig.mm
Index: clang/test/Analysis/mig.mm =================================================================== --- clang/test/Analysis/mig.mm +++ clang/test/Analysis/mig.mm @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\ -// RUN: -fblocks -verify %s +// RUN: -analyzer-output=text -fblocks -verify %s // XNU APIs. @@ -20,9 +20,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; } @@ -42,6 +44,18 @@ 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}} +} + // Check that we work on Objective-C messages and blocks. @interface I - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size; @@ -51,8 +65,9 @@ - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size MIG_SERVER_ROUTINE { - vm_deallocate(port, address, size); + vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}} 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}} } @end @@ -60,8 +75,9 @@ kern_return_t (^block)(mach_port_name_t, vm_address_t, vm_size_t) = ^MIG_SERVER_ROUTINE (mach_port_name_t port, vm_address_t address, vm_size_t size) { - vm_deallocate(port, address, size); + vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}} 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}} }; } Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -46,14 +46,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<VarRegion>(Sym->getOriginRegion()); - return VR && VR->hasStackParametersStorage() && - VR->getStackFrame()->inTopFrame(); + if (VR && VR->hasStackParametersStorage() && + VR->getStackFrame()->inTopFrame()) + return cast<ParmVarDecl>(VR->getDecl()); + + return nullptr; } static bool isInMIGCall(CheckerContext &C) { @@ -96,8 +99,18 @@ // TODO: Unhardcode "1". SVal Arg = Call.getArgSVal(1); - if (isCurrentArgSVal(Arg, C)) - C.addTransition(C.getState()->set<ReleasedParameter>(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<ReleasedParameter>(true), T); } void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { @@ -139,6 +152,8 @@ "deallocate it again", N); + R->addRange(RS->getSourceRange()); + bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); C.emitReport(std::move(R)); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits