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
