ygribov removed rL LLVM as the repository for this revision.
ygribov updated this revision to Diff 38423.
ygribov added a comment.

Updated based on 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{{}}
+    break;
+  case 2:
+    // Ensure that calling functions is prohibited.
+    foo(); // expected-warning{{}}
+    break;
+  default:
+    // Ensure that returning from function is prohibited.
+    return 0;
+  }
+
+  while(1);
+} // expected-warning{{}}
+
+// 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{{}}
+    break;
+  case 2:
+    // Ensure that calling functions is prohibited.
+    foo(); // expected-warning{{}}
+    break;
+  default:
+    // Ensure that returning from function is prohibited.
+    return 0;
+  }
+
+  while(1);
+} // expected-warning{{}}
+
+// 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{{}}
+    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); // 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{{}}
+  }
+}
+
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,238 @@
+//===- 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.
+//
+//===----------------------------------------------------------------------===//
+
+#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;
+
+// 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
+
+// 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 vforked process"));
+
+    SmallString<256> buf;
+    llvm::raw_svector_ostream os(buf);
+
+    os << What << " is prohibited after successful vfork";
+
+    auto Report = llvm::make_unique<BugReport>(*BT, os.str(), N);
+    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(3) or _exit(2))", 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 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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to