ygribov created this revision.
ygribov added reviewers: zaks.anna, dcoughlin, jordan_rose, krememek.
ygribov added a subscriber: cfe-commits.
ygribov set the repository for this revision to rL LLVM.
Hi all,
This checker verifies that vfork is used safely. Vforked process shared stack
with parent process so it's range of actions is significantly limited (can't
write variables, can't call functions not in whitelist, etc.).
The patch grew out of complicated 2-day debugging of production SW caused by
well-known vfork bug in xtables (see
http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
Is something like this interesting for upstream?
Repository:
rL LLVM
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-1.c
test/Analysis/vfork-2.c
Index: test/Analysis/vfork-2.c
===================================================================
--- /dev/null
+++ test/Analysis/vfork-2.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.Vfork %s -verify
+
+#include "Inputs/system-header-simulator.h"
+
+void foo() {
+ // See "libxtables: move some code to avoid cautions in vfork man page"
+ if (vfork() == 0) {
+ execl("prog", "arg1", 0);
+ exit(1); // expected-warning{{}}
+ }
+}
+
Index: test/Analysis/vfork-1.c
===================================================================
--- /dev/null
+++ test/Analysis/vfork-1.c
@@ -0,0 +1,104 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions,alpha.unix.Vfork %s -verify
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Check that child process is checked
+int f1(int x) {
+ pid_t pid = vfork();
+ if (pid != 0)
+ return 0;
+
+ switch (x) {
+ case 0:
+ // writing pid is ok
+ pid = 1;
+ // calling whitelisted routines is ok
+ execl("", "", 0);
+ _exit(1);
+ break;
+ case 1:
+ // writing variables is prohibited
+ x = 0; // expected-warning{{}}
+ break;
+ case 2:
+ // calling functions is prohibited
+ foo(); // expected-warning{{}}
+ break;
+ default:
+ // returning from function is prohibited
+ return 0;
+ }
+
+ while(1);
+} // expected-warning{{}}
+
+// Same but without explicit pid
+int f2(int x) {
+ pid_t pid = vfork();
+
+ switch (x) {
+ case 0:
+ // writing pid is ok
+ pid = 1;
+ // calling whitelisted routines is ok
+ execl("", "", 0);
+ _exit(1);
+ break;
+ case 1:
+ // writing variables is prohibited
+ x = 0; // expected-warning{{}}
+ break;
+ case 2:
+ // calling functions is prohibited
+ foo(); // expected-warning{{}}
+ break;
+ default:
+ // returning from function is prohibited
+ return 0;
+ }
+
+ while(1);
+} // expected-warning{{}}
+
+// check that parent process can do anything
+int f3(int x) {
+ if (vfork() == 0)
+ _exit(1);
+ x = 0;
+ foo();
+ return 0;
+}
+
+// unbound pids are special so test them separately
+void f4(int x) {
+ switch (x) {
+ case 0:
+ vfork();
+ x = 0; // expected-warning{{}}
+ break;
+
+ case 1:
+ {
+ char args[2];
+ switch (vfork()) {
+ case 0:
+ args[0] = 0; // expected-warning{{}}
+ exit(1);
+ }
+ break;
+ }
+
+ case 2:
+ {
+ pid_t pid;
+ if ((pid = vfork()) == 0)
+ while(1);
+ break;
+ }
+ }
+ while(1);
+}
+
+
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,260 @@
+//===- 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 chroot checker, which checks improper use of chroot.
+//
+//===----------------------------------------------------------------------===//
+
+#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;
+
+namespace {
+
+// This checker checks prohibited constructs in vforked process.
+// The state transition:
+// PARENT ---(vfork() == 0)--> CHILD
+// |
+// --(*p = ...)--> bug
+// |
+// --foo()--> bug
+// |
+// --return--> bug
+class VforkChecker : public Checker<check::PreCall, check::PostCall,
+ check::Bind, check::EndFunction> {
+ mutable const IdentifierInfo *II_vfork;
+ mutable SmallVector<const IdentifierInfo *, 10> VforkWhitelist;
+
+ // This bug refers to dangerous code in vforked child.
+ mutable std::unique_ptr<BuiltinBug> BT_BadChildCode;
+
+ bool isChildProcess(const ProgramStateRef State) const;
+
+ bool isInWhitelist(const IdentifierInfo *II, CheckerContext &C) const;
+
+ void reportBug(CheckerContext &C) const;
+
+ const VarDecl *getAssignedVariable(const CallEvent &Call,
+ CheckerContext &C) const;
+
+ bool isVforkCall(const Decl *D, CheckerContext &C) const;
+
+public:
+ VforkChecker() : II_vfork(0) {}
+
+ // these are used to track vfork state
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
+
+ // these constructs are prohibited in vforked child
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkEndFunction(CheckerContext &C) const;
+};
+
+} // end anonymous namespace
+
+// holds region of variable that is assigned with vfork's return value (the only
+// region child is allowed to write)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(VforkLhs, const void *)
+#define VFORK_LHS_NONE ((void *)(uintptr_t)1)
+
+bool VforkChecker::isChildProcess(const ProgramStateRef State) const {
+ return State->get<VforkLhs>();
+}
+
+bool VforkChecker::isInWhitelist(const IdentifierInfo *II,
+ CheckerContext &C) const {
+ if (VforkWhitelist.empty()) {
+ // according to vfork(3)
+ const char *ids[] = {
+ "_exit",
+ "_Exit",
+ "execl",
+ "execlp",
+ "execle",
+ "execv",
+ "execvp",
+ "execvpe",
+ 0,
+ };
+
+ ASTContext &Ctx = C.getASTContext();
+ for (const char **id = ids; *id; ++id)
+ VforkWhitelist.push_back(&Ctx.Idents.get(*id));
+ }
+
+ for (SmallVectorImpl<const IdentifierInfo *>::iterator
+ I = VforkWhitelist.begin(), E = VforkWhitelist.end();
+ I != E; ++I) {
+ if (*I == II)
+ return true;
+ }
+
+ return false;
+}
+
+void VforkChecker::reportBug(CheckerContext &C) const {
+ if (!BT_BadChildCode)
+ BT_BadChildCode.reset(
+ new BuiltinBug(this, "Dangerous construct in vforked process",
+ "Prohibited construct after successful "
+ "vfork"));
+ ExplodedNode *N = C.generateSink(C.getState(), C.getPredecessor());
+ auto Report = llvm::make_unique<BugReport>(*BT_BadChildCode,
+ BT_BadChildCode->getDescription(),
+ N);
+ C.emitReport(std::move(Report));
+}
+
+// return LHS in LHS = vfork() statement
+const VarDecl *VforkChecker::getAssignedVariable(const CallEvent &Call,
+ CheckerContext &C) const {
+ const Expr *CE = Call.getOriginExpr();
+
+ const ParentMap &PM = C.getLocationContext()->getParentMap();
+ const Stmt *P = PM.getParentIgnoreParenCasts(CE);
+
+ // see if it's a variable initialization
+ do {
+ const DeclStmt *PD = dyn_cast_or_null<DeclStmt>(P);
+ if (!PD)
+ break;
+
+ if (!PD->isSingleDecl())
+ break;
+
+ const VarDecl *D = dyn_cast_or_null<VarDecl>(PD->getSingleDecl());
+ if (!D)
+ break;
+
+ return D;
+ } while (0);
+
+ // see if it's an ordinary assignment
+ do {
+ const BinaryOperator *Assign = dyn_cast_or_null<BinaryOperator>(P);
+ if (!Assign || !Assign->isAssignmentOp())
+ break;
+
+ const DeclRefExpr *DE = dyn_cast_or_null<DeclRefExpr>(Assign->getLHS());
+ if (!DE)
+ break;
+
+ const VarDecl *D = dyn_cast_or_null<VarDecl>(DE->getDecl());
+ if (!D)
+ break;
+
+ return D;
+ } while (0);
+
+ // otherwise no lhs
+ return (const VarDecl *)VFORK_LHS_NONE;
+}
+
+bool VforkChecker::isVforkCall(const Decl *D, CheckerContext &C) const {
+ const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
+ if (!FD || !C.isCLibraryFunction(FD))
+ return false;
+
+ if (!II_vfork) {
+ ASTContext &Ctx = C.getASTContext();
+ II_vfork = &Ctx.Idents.get("vfork");
+ }
+
+ return FD->getIdentifier() == II_vfork;
+}
+
+// track calls to vforked process
+void VforkChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ // we can't call vfork in child so don't bother
+ ProgramStateRef State = C.getState();
+ if (isChildProcess(State))
+ return;
+
+ if (!isVforkCall(Call.getDecl(), C))
+ return;
+
+ // return value of vfork
+ SVal VforkRetVal = Call.getReturnValue();
+ SymbolRef Sym = VforkRetVal.getAsSymbol();
+ Optional<DefinedOrUnknownSVal> DVal =
+ VforkRetVal.getAs<DefinedOrUnknownSVal>();
+ if (!Sym || !DVal)
+ return;
+
+ // provide child with assigned pid variable
+ const VarDecl *LhsDecl = getAssignedVariable(Call, C);
+ MemRegionManager &M = C.getStoreManager().getRegionManager();
+ const MemRegion *LhsDeclReg =
+ LhsDecl == VFORK_LHS_NONE
+ ? (const MemRegion *)VFORK_LHS_NONE
+ : M.getVarRegion(LhsDecl, C.getLocationContext());
+
+ // parent gets nonzero
+ ProgramStateRef ParentState, ChildState;
+ std::tie(ParentState, ChildState) = C.getState()->assume(*DVal);
+ C.addTransition(ParentState);
+ ChildState = ChildState->set<VforkLhs>(LhsDeclReg);
+ C.addTransition(ChildState);
+}
+
+// prohibit calls to non-whitelist functions in vfork
+void VforkChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (!isChildProcess(State))
+ return;
+
+ if (isInWhitelist(Call.getCalleeIdentifier(), C))
+ return;
+
+ reportBug(C);
+}
+
+// prohibit writes in child
+void VforkChecker::checkBind(SVal L, SVal V, const Stmt *S,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (!isChildProcess(State))
+ return;
+
+ const MemRegion *VforkLhsReg =
+ static_cast<const MemRegion *>(State->get<VforkLhs>());
+ const MemRegion *MR = L.getAsRegion();
+
+ // child is allowed to modify only vfork's lhs
+ if (!MR || MR == VforkLhsReg)
+ return;
+
+ reportBug(C);
+}
+
+// prohibit return from function in vforked process
+void VforkChecker::checkEndFunction(CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (!isChildProcess(State))
+ return;
+
+ reportBug(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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
