ygribov updated this revision to Diff 38740.
ygribov added a comment.
Updated after Anna's review.
http://reviews.llvm.org/D14014
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/VforkChecker.cpp
test/Analysis/Inputs/system-header-simulator.h
test/Analysis/vfork.c
Index: test/Analysis/vfork.c
===================================================================
--- /dev/null
+++ test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+ pid_t pid = vfork();
+ if (pid != 0)
+ return 0;
+
+ switch (x) {
+ case 0:
+ // Ensure that modifying pid is ok.
+ pid = 1; // no-warning
+ // Ensure that calling whitelisted routines is ok.
+ execl("", "", 0); // no-warning
+ _exit(1); // no-warning
+ break;
+ case 1:
+ // Ensure that writing variables is prohibited.
+ x = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+ break;
+ case 2:
+ // Ensure that calling functions is prohibited.
+ foo(); // expected-warning{{Calling functions (except exec*() or _exit()) is prohibited after a successful vfork}}
+ break;
+ default:
+ // Ensure that returning from function is prohibited.
+ return 0;
+ }
+
+ while(1);
+} // expected-warning{{Returning from a function is prohibited after a successful vfork}}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+ pid_t pid = vfork();
+
+ switch (x) {
+ case 0:
+ // Ensure that writing pid is ok.
+ pid = 1; // no-warning
+ // Ensure that calling whitelisted routines is ok.
+ execl("", "", 0); // no-warning
+ _exit(1); // no-warning
+ break;
+ case 1:
+ // Ensure that writing variables is prohibited.
+ x = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+ break;
+ case 2:
+ // Ensure that calling functions is prohibited.
+ foo(); // expected-warning{{Calling functions (except exec*() or _exit()) is prohibited after a successful vfork}}
+ break;
+ default:
+ // Ensure that returning from function is prohibited.
+ return 0;
+ }
+
+ while(1);
+} // expected-warning{{Returning from a function is prohibited after a successful vfork}}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+ if (vfork() == 0)
+ _exit(1);
+ x = 0; // no-warning
+ foo(); // no-warning
+ return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+ switch (x) {
+ case 0:
+ vfork();
+ x = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+ break;
+
+ case 1:
+ {
+ char args[2];
+ switch (vfork()) {
+ case 0:
+ args[0] = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+ exit(1);
+ }
+ break;
+ }
+
+ case 2:
+ {
+ pid_t pid;
+ if ((pid = vfork()) == 0)
+ while(1); // no-warning
+ break;
+ }
+ }
+ while(1);
+} //no-warning
+
+
+void f5() {
+ // See "libxtables: move some code to avoid cautions in vfork man page"
+ // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+ if (vfork() == 0) {
+ execl("prog", "arg1", 0); // no-warning
+ exit(1); // expected-warning{{Calling functions (except exec*() or _exit()) is prohibited after a successful vfork}}
+ }
+}
+
Index: test/Analysis/Inputs/system-header-simulator.h
===================================================================
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -86,3 +86,13 @@
char * p;
} SomeStruct;
void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -0,0 +1,239 @@
+//===- VforkChecker.cpp -------- Vfork usage checks --------------*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines vfork checker which checks for dangerous uses of vfork.
+// Vforked process shares memory (including stack) with parent so it's
+// range of actions is significantly limited: can't write variables,
+// can't call functions not in whitelist, etc. For more details, see
+// http://man7.org/linux/man-pages/man2/vfork.2.html
+//
+// This checker checks for prohibited constructs in vforked process.
+// The state transition diagram:
+// PARENT ---(vfork() == 0)--> CHILD
+// |
+// --(*p = ...)--> bug
+// |
+// --foo()--> bug
+// |
+// --return--> bug
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+
+using namespace clang;
+using namespace ento;
+
+// Pattern matches to extract lhs in `lhs = vfork()' statement.
+static const VarDecl *GetAssignedVariable(const CallEvent &Call,
+ CheckerContext &C) {
+ const Expr *CE = Call.getOriginExpr();
+
+ const ParentMap &PM = C.getLocationContext()->getParentMap();
+ const Stmt *P = PM.getParentIgnoreParenCasts(CE);
+
+ // Check for ordinary assignment.
+ do {
+ auto Assign = dyn_cast_or_null<BinaryOperator>(P);
+ if (!Assign || !Assign->isAssignmentOp())
+ break;
+
+ auto DE = dyn_cast_or_null<DeclRefExpr>(Assign->getLHS());
+ if (!DE)
+ break;
+
+ if (auto D = dyn_cast_or_null<VarDecl>(DE->getDecl()))
+ return D;
+ } while (0);
+
+ // Check for variable initialization.
+ auto PD = dyn_cast_or_null<DeclStmt>(P);
+ if (PD && PD->isSingleDecl())
+ if (auto D = dyn_cast_or_null<VarDecl>(PD->getSingleDecl()))
+ return D;
+
+ // Otherwise no lhs.
+ return 0;
+}
+
+namespace {
+
+class VforkChecker : public Checker<check::PreCall, check::PostCall,
+ check::Bind, check::EndFunction> {
+ mutable std::unique_ptr<BuiltinBug> BT;
+ mutable llvm::SmallSet<const IdentifierInfo *, 10> VforkWhitelist;
+ mutable const IdentifierInfo *II_vfork;
+
+ static bool isChildProcess(const ProgramStateRef State);
+
+ bool isVforkCall(const Decl *D, CheckerContext &C) const;
+ bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext &C) const;
+
+ void reportBug(const char *What, CheckerContext &C) const;
+
+public:
+ VforkChecker() : II_vfork(0) {}
+
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
+ void checkEndFunction(CheckerContext &C) const;
+};
+
+} // end anonymous namespace
+
+// This trait holds region of variable that is assigned with vfork's
+// return value (this is the only region child is allowed to write).
+// VFORK_RESULT_INVALID means that we are in parent process.
+// VFORK_RESULT_NONE means that vfork's return value hasn't been assigned.
+// Other values point to valid regions.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(VforkResultRegion, const void *)
+#define VFORK_RESULT_INVALID 0
+#define VFORK_RESULT_NONE ((void *)(uintptr_t)1)
+
+bool VforkChecker::isChildProcess(const ProgramStateRef State) {
+ return State->get<VforkResultRegion>() != VFORK_RESULT_INVALID;
+}
+
+bool VforkChecker::isVforkCall(const Decl *D, CheckerContext &C) const {
+ auto FD = dyn_cast_or_null<FunctionDecl>(D);
+ if (!FD || !C.isCLibraryFunction(FD))
+ return false;
+
+ if (!II_vfork) {
+ ASTContext &AC = C.getASTContext();
+ II_vfork = &AC.Idents.get("vfork");
+ }
+
+ return FD->getIdentifier() == II_vfork;
+}
+
+// Returns true iff ok to call function after successful vfork.
+bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
+ CheckerContext &C) const {
+ if (VforkWhitelist.empty()) {
+ // According to manpage.
+ const char *ids[] = {
+ "_exit",
+ "_Exit",
+ "execl",
+ "execlp",
+ "execle",
+ "execv",
+ "execvp",
+ "execvpe",
+ 0,
+ };
+
+ ASTContext &AC = C.getASTContext();
+ for (const char **id = ids; *id; ++id)
+ VforkWhitelist.insert(&AC.Idents.get(*id));
+ }
+
+ return VforkWhitelist.count(II);
+}
+
+void VforkChecker::reportBug(const char *What, CheckerContext &C) const {
+ if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
+ if (!BT)
+ BT.reset(new BuiltinBug(this,
+ "Dangerous construct in a vforked process"));
+
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+
+ os << What << " is prohibited after a successful vfork";
+
+ auto Report = llvm::make_unique<BugReport>(*BT, os.str(), N);
+ // TODO: mark vfork call in BugReportVisitor
+ C.emitReport(std::move(Report));
+ }
+}
+
+// Detect calls to vfork and split execution appropriately.
+void VforkChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ // We can't call vfork in child so don't bother
+ // (corresponding warning has already been emitted in checkPreCall).
+ ProgramStateRef State = C.getState();
+ if (isChildProcess(State))
+ return;
+
+ if (!isVforkCall(Call.getDecl(), C))
+ return;
+
+ // Get return value of vfork.
+ SVal VforkRetVal = Call.getReturnValue();
+ Optional<DefinedOrUnknownSVal> DVal =
+ VforkRetVal.getAs<DefinedOrUnknownSVal>();
+ if (!DVal)
+ return;
+
+ // Find assigned memory region.
+ const VarDecl *LhsDecl = GetAssignedVariable(Call, C);
+ MemRegionManager &M = C.getStoreManager().getRegionManager();
+ const MemRegion *LhsDeclReg =
+ LhsDecl
+ ? M.getVarRegion(LhsDecl, C.getLocationContext())
+ : (const MemRegion *)VFORK_RESULT_NONE;
+
+ // Parent branch gets nonzero return value (according to manpage).
+ ProgramStateRef ParentState, ChildState;
+ std::tie(ParentState, ChildState) = C.getState()->assume(*DVal);
+ C.addTransition(ParentState);
+ ChildState = ChildState->set<VforkResultRegion>(LhsDeclReg);
+ C.addTransition(ChildState);
+}
+
+// Prohibit calls to non-whitelist functions in child process.
+void VforkChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (isChildProcess(State)
+ && !isCallWhitelisted(Call.getCalleeIdentifier(), C))
+ reportBug("Calling functions (except exec*() or _exit())", C);
+}
+
+// Prohibit writes in child process (except for vfork's lhs).
+void VforkChecker::checkBind(SVal L, SVal V, const Stmt *S,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (!isChildProcess(State))
+ return;
+
+ const MemRegion *VforkLhs =
+ static_cast<const MemRegion *>(State->get<VforkResultRegion>());
+ const MemRegion *MR = L.getAsRegion();
+
+ // Child is allowed to modify only vfork's lhs.
+ if (!MR || MR == VforkLhs)
+ return;
+
+ reportBug("Assigning variables (except return value of vfork)", C);
+}
+
+// Prohibit return from function in child process.
+void VforkChecker::checkEndFunction(CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (isChildProcess(State))
+ reportBug("Returning from a function", C);
+}
+
+void ento::registerVforkChecker(CheckerManager &mgr) {
+ mgr.registerChecker<VforkChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -356,6 +356,10 @@
HelpText<"Check for misuses of stream APIs">,
DescFile<"SimpleStreamChecker.cpp">;
+def VforkChecker : Checker<"Vfork">,
+ HelpText<"Check for proper usage of vfork">,
+ DescFile<"VforkChecker.cpp">;
+
} // end "alpha.unix"
let ParentPackage = CString in {
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -76,6 +76,7 @@
UndefinedAssignmentChecker.cpp
UnixAPIChecker.cpp
UnreachableCodeChecker.cpp
+ VforkChecker.cpp
VLASizeChecker.cpp
VirtualCallChecker.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
