NoQ updated this revision to Diff 187503. NoQ added a comment. Rebase.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58392/new/ https://reviews.llvm.org/D58392 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 @@ -56,6 +56,31 @@ // 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}} } +// Make sure we find the bug when the object is destroyed within an +// automatic destructor. +MIG_SERVER_ROUTINE +kern_return_t test_vm_deallocate_in_automatic_dtor(mach_port_name_t port, vm_address_t address, vm_size_t size) { + struct WillDeallocate { + mach_port_name_t port; + vm_address_t address; + vm_size_t size; + ~WillDeallocate() { + vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}} + } + } will_deallocate{port, address, size}; + + if (size > 10) { + // expected-note@-1{{Assuming 'size' is > 10}} + // expected-note@-2{{Taking true branch}} + return KERN_ERROR; + // expected-note@-1{{Calling '~WillDeallocate'}} + // expected-note@-2{{Returning from '~WillDeallocate'}} + // expected-warning@-3{{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@-4 {{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; +} + // 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; Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -32,15 +32,30 @@ using namespace ento; namespace { -class MIGChecker : public Checker<check::PostCall, check::PreStmt<ReturnStmt>> { +class MIGChecker : public Checker<check::PostCall, check::PreStmt<ReturnStmt>, + check::EndFunction> { BugType BT{this, "Use-after-free (MIG calling convention violation)", categories::MemoryError}; CallDescription vm_deallocate { "vm_deallocate", 3 }; + void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const; + public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; - void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; + + // HACK: We're making two attempts to find the bug: checkEndFunction + // should normally be enough but it fails when the return value is a literal + // that never gets put into the Environment and ends of function with multiple + // returns get agglutinated across returns, preventing us from obtaining + // the return value. The problem is similar to https://reviews.llvm.org/D25326 + // but now we step into it in the top-level function. + void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { + checkReturnAux(RS, C); + } + void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const { + checkReturnAux(RS, C); + } }; } // end anonymous namespace @@ -113,7 +128,7 @@ C.addTransition(C.getState()->set<ReleasedParameter>(true), T); } -void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { +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 // that follows the MIG calling convention. Therefore we're safe to believe
Index: clang/test/Analysis/mig.mm =================================================================== --- clang/test/Analysis/mig.mm +++ clang/test/Analysis/mig.mm @@ -56,6 +56,31 @@ // 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}} } +// Make sure we find the bug when the object is destroyed within an +// automatic destructor. +MIG_SERVER_ROUTINE +kern_return_t test_vm_deallocate_in_automatic_dtor(mach_port_name_t port, vm_address_t address, vm_size_t size) { + struct WillDeallocate { + mach_port_name_t port; + vm_address_t address; + vm_size_t size; + ~WillDeallocate() { + vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}} + } + } will_deallocate{port, address, size}; + + if (size > 10) { + // expected-note@-1{{Assuming 'size' is > 10}} + // expected-note@-2{{Taking true branch}} + return KERN_ERROR; + // expected-note@-1{{Calling '~WillDeallocate'}} + // expected-note@-2{{Returning from '~WillDeallocate'}} + // expected-warning@-3{{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@-4 {{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; +} + // 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; Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -32,15 +32,30 @@ using namespace ento; namespace { -class MIGChecker : public Checker<check::PostCall, check::PreStmt<ReturnStmt>> { +class MIGChecker : public Checker<check::PostCall, check::PreStmt<ReturnStmt>, + check::EndFunction> { BugType BT{this, "Use-after-free (MIG calling convention violation)", categories::MemoryError}; CallDescription vm_deallocate { "vm_deallocate", 3 }; + void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const; + public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; - void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; + + // HACK: We're making two attempts to find the bug: checkEndFunction + // should normally be enough but it fails when the return value is a literal + // that never gets put into the Environment and ends of function with multiple + // returns get agglutinated across returns, preventing us from obtaining + // the return value. The problem is similar to https://reviews.llvm.org/D25326 + // but now we step into it in the top-level function. + void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { + checkReturnAux(RS, C); + } + void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const { + checkReturnAux(RS, C); + } }; } // end anonymous namespace @@ -113,7 +128,7 @@ C.addTransition(C.getState()->set<ReleasedParameter>(true), T); } -void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { +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 // that follows the MIG calling convention. Therefore we're safe to believe
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits