[PATCH v4] Bug 14526 - MallocChecker: Add awareness of kernel malloc() flags
Meyer, Conrad
conrad.meyer at isilon.com
Fri Mar 21 11:05:45 PDT 2014
Add M_ZERO awareness to malloc() static analysis in Clang for FreeBSD,
NetBSD, and OpenBSD in a similar fashion to O_CREAT for open(2).
This should reduce the number of false positives when running static
analysis on BSD kernels.
Additionally, add kmalloc() (Linux kernel malloc()) and treat __GFP_ZERO
like M_ZERO on Linux.
Future work involves a better (symbolic) method of checking for named
flags without hardcoding values.
Add Analysis/malloc-three-arg.c tests for M_ZERO-alike malloc, and
Analysis/kmalloc-linux.c tests for the kmalloc() and __GFP_ZERO
additions.
Sponsored by: EMC/Isilon storage division
Signed-off-by: Conrad Meyer <conrad.meyer at isilon.com>
---
Based on newish LLVM, Clang. Something more recently broke my build so I just
rebased to sources from Tuesday. Shouldn't be too stale.
LLVM+Clang builds, and the new tests pass.
Per review, renamed M_ZERO and MallocZero to KernelZeroFlagVal and
performKernelMalloc(). Also, dropped RetVal and used None for the aborted
returns in performKernelMalloc().
I think I missed the mark earlier on the question:
> Are there ever going to be systems that have both kmalloc and
> malloc-with-flags?
I don't think there will be a system where both types of malloc are equally
important. For now I think it's safe to assume that one of the flag values is
more important than the other, if both exist simultaneously (__GFP_ZERO and
M_ZERO).
Thanks.
---
lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 114 ++++++++++++++++++++++++-
lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp | 1 +
test/Analysis/kmalloc-linux.c | 58 +++++++++++++
test/Analysis/malloc-three-arg.c | 58 +++++++++++++
4 files changed, 227 insertions(+), 4 deletions(-)
create mode 100644 test/Analysis/kmalloc-linux.c
create mode 100644 test/Analysis/malloc-three-arg.c
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index f64c392..7ec777d 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -14,10 +14,11 @@
#include "ClangSACheckers.h"
#include "InterCheckerAPI.h"
#include "clang/AST/Attr.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.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"
@@ -155,11 +156,12 @@ class MallocChecker : public Checker<check::DeadSymbols,
check::Location,
eval::Assume>
{
public:
MallocChecker() : II_malloc(0), II_free(0), II_realloc(0), II_calloc(0),
- II_valloc(0), II_reallocf(0), II_strndup(0), II_strdup(0) {}
+ II_valloc(0), II_reallocf(0), II_strndup(0), II_strdup(0),
+ II_kmalloc(0) {}
/// In pessimistic mode, the checker assumes that it does not know which
/// functions might free the memory.
enum CheckKind {
CK_MallocPessimistic,
@@ -205,11 +207,13 @@ private:
mutable std::unique_ptr<BugType> BT_UseFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_BadFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
mutable IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc,
- *II_valloc, *II_reallocf, *II_strndup, *II_strdup;
+ *II_valloc, *II_reallocf, *II_strndup, *II_strdup,
+ *II_kmalloc;
+ mutable Optional<uint64_t> KernelZeroFlagVal;
void initIdentifierInfo(ASTContext &C) const;
/// \brief Determine family of a deallocation expression.
AllocationFamily getAllocationFamily(CheckerContext &C, const Stmt *S) const;
@@ -251,10 +255,16 @@ private:
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
SVal SizeEx, SVal Init,
ProgramStateRef State,
AllocationFamily Family = AF_Malloc);
+ // Check if this malloc() for special flags. At present that means M_ZERO or
+ // __GFP_ZERO (in which case, treat it like calloc).
+ llvm::Optional<ProgramStateRef>
+ performKernelMalloc(const CallExpr *CE, CheckerContext &C,
+ const ProgramStateRef &State) const;
+
/// Update the RefState to reflect the new memory allocation.
static ProgramStateRef
MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State,
AllocationFamily Family = AF_Malloc);
@@ -480,10 +490,11 @@ void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const {
II_reallocf = &Ctx.Idents.get("reallocf");
II_calloc = &Ctx.Idents.get("calloc");
II_valloc = &Ctx.Idents.get("valloc");
II_strdup = &Ctx.Idents.get("strdup");
II_strndup = &Ctx.Idents.get("strndup");
+ II_kmalloc = &Ctx.Idents.get("kmalloc");
}
bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {
if (isFreeFunction(FD, C))
return true;
@@ -506,11 +517,11 @@ bool MallocChecker::isAllocationFunction(const FunctionDecl *FD,
IdentifierInfo *FunI = FD->getIdentifier();
initIdentifierInfo(C);
if (FunI == II_malloc || FunI == II_realloc ||
FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc ||
- FunI == II_strdup || FunI == II_strndup)
+ FunI == II_strdup || FunI == II_strndup || FunI == II_kmalloc)
return true;
}
if (ChecksEnabled[CK_MallocOptimistic] && FD->hasAttrs())
for (const auto *I : FD->specific_attrs<OwnershipAttr>())
@@ -570,10 +581,85 @@ bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD,
// One of the standard new/new[]/delete/delete[] non-placement operators.
return true;
}
+llvm::Optional<ProgramStateRef> MallocChecker::performKernelMalloc(
+ const CallExpr *CE, CheckerContext &C, const ProgramStateRef &State) const {
+ // 3-argument malloc(), as commonly used in {Free,Net,Open}BSD Kernels:
+ //
+ // void *malloc(unsigned long size, struct malloc_type *mtp, int flags);
+ //
+ // One of the possible flags is M_ZERO, which means 'give me back an
+ // allocation which is already zeroed', like calloc.
+
+ // 2-argument kmalloc(), as used in the Linux kernel:
+ //
+ // void *kmalloc(size_t size, gfp_t flags);
+ //
+ // Has the similar flag value __GFP_ZERO.
+
+ // This logic is largely cloned from O_CREAT in UnixAPIChecker, maybe some
+ // code could be shared.
+
+ llvm::Triple::OSType os = C.getASTContext().getTargetInfo().getTriple().getOS();
+
+ if (!KernelZeroFlagVal.hasValue()) {
+ if (os == llvm::Triple::FreeBSD)
+ KernelZeroFlagVal = 0x0100;
+ else if (os == llvm::Triple::NetBSD)
+ KernelZeroFlagVal = 0x0002;
+ else if (os == llvm::Triple::OpenBSD)
+ KernelZeroFlagVal = 0x0008;
+ else if (os == llvm::Triple::Linux)
+ // __GFP_ZERO
+ KernelZeroFlagVal = 0x8000;
+ else
+ // FIXME: We need a more general way of getting the M_ZERO value.
+ // See also: O_CREAT in UnixAPIChecker.cpp.
+
+ // Fall back to normal malloc behavior on platforms where we don't
+ // know M_ZERO.
+ return None;
+ }
+
+ // We treat the last argument as the flags argument, and callers fall-back to
+ // normal malloc on a None return. This works for the FreeBSD kernel malloc
+ // as well as Linux kmalloc.
+ if (CE->getNumArgs() < 2)
+ return None;
+
+ const Expr *FlagsEx = CE->getArg(CE->getNumArgs() - 1);
+ const SVal V = State->getSVal(FlagsEx, C.getLocationContext());
+ if (!V.getAs<NonLoc>()) {
+ // The case where 'V' can be a location can only be due to a bad header,
+ // so in this case bail out.
+ return None;
+ }
+
+ NonLoc Flags = V.castAs<NonLoc>();
+ NonLoc ZeroFlag = C.getSValBuilder()
+ .makeIntVal(KernelZeroFlagVal.getValue(), FlagsEx->getType())
+ .castAs<NonLoc>();
+ SVal MaskedFlagsUC = C.getSValBuilder().evalBinOpNN(State, BO_And,
+ Flags, ZeroFlag,
+ FlagsEx->getType());
+ if (MaskedFlagsUC.isUnknownOrUndef())
+ return None;
+ DefinedSVal MaskedFlags = MaskedFlagsUC.castAs<DefinedSVal>();
+
+ // Check if maskedFlags is non-zero.
+ ProgramStateRef TrueState, FalseState;
+ std::tie(TrueState, FalseState) = State->assume(MaskedFlags);
+
+ // If M_ZERO is set, treat this like calloc (initialized).
+ if (TrueState && !FalseState)
+ return CallocMem(C, CE);
+
+ return None;
+}
+
void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (C.wasInlined)
return;
const FunctionDecl *FD = C.getCalleeDecl(CE);
@@ -585,11 +671,31 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (FD->getKind() == Decl::Function) {
initIdentifierInfo(C.getASTContext());
IdentifierInfo *FunI = FD->getIdentifier();
- if (FunI == II_malloc || FunI == II_valloc) {
+ if (FunI == II_malloc) {
+ if (CE->getNumArgs() < 1)
+ return;
+ if (CE->getNumArgs() < 3) {
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ } else if (CE->getNumArgs() == 3) {
+ llvm::Optional<ProgramStateRef> MaybeState =
+ performKernelMalloc(CE, C, State);
+ if (MaybeState.hasValue())
+ State = MaybeState.getValue();
+ else
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ }
+ } else if (FunI == II_kmalloc) {
+ llvm::Optional<ProgramStateRef> MaybeState =
+ performKernelMalloc(CE, C, State);
+ if (MaybeState.hasValue())
+ State = MaybeState.getValue();
+ else
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ } else if (FunI == II_valloc) {
if (CE->getNumArgs() < 1)
return;
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
} else if (FunI == II_realloc) {
State = ReallocMem(C, CE, false);
diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 8869654..aa7e25bc 100644
--- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -78,10 +78,11 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
Val_O_CREAT = 0x0200;
else {
// FIXME: We need a more general way of getting the O_CREAT value.
// We could possibly grovel through the preprocessor state, but
// that would require passing the Preprocessor object to the ExprEngine.
+ // See also: MallocChecker.cpp / M_ZERO.
return;
}
}
// Look at the 'oflags' argument for the O_CREAT flag.
diff --git a/test/Analysis/kmalloc-linux.c b/test/Analysis/kmalloc-linux.c
new file mode 100644
index 0000000..87c1107
--- /dev/null
+++ b/test/Analysis/kmalloc-linux.c
@@ -0,0 +1,58 @@
+// RUN: %clang -target x86_64-unknown-linux --analyze %s
+
+#include "Inputs/system-header-simulator.h"
+
+#define __GFP_ZERO 0x8000
+#define NULL ((void *)0)
+
+void *kmalloc(size_t, int);
+
+struct test {
+};
+
+void foo(struct test *);
+
+void test_zeroed() {
+ struct test **list, *t;
+ int i;
+
+ list = kmalloc(sizeof(*list) * 10, __GFP_ZERO);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < 10; i++) {
+ t = list[i];
+ foo(t);
+ }
+ free(list); // no-warning
+}
+
+void test_nonzero() {
+ struct test **list, *t;
+ int i;
+
+ list = kmalloc(sizeof(*list) * 10, 0);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < 10; i++) {
+ t = list[i]; // expected-warning{{undefined}}
+ foo(t);
+ }
+ free(list);
+}
+
+void test_indeterminate(int flags) {
+ struct test **list, *t;
+ int i;
+
+ list = kmalloc(sizeof(*list) * 10, flags);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < 10; i++) {
+ t = list[i]; // expected-warning{{undefined}}
+ foo(t);
+ }
+ free(list);
+}
diff --git a/test/Analysis/malloc-three-arg.c b/test/Analysis/malloc-three-arg.c
new file mode 100644
index 0000000..01b08ae
--- /dev/null
+++ b/test/Analysis/malloc-three-arg.c
@@ -0,0 +1,58 @@
+// RUN: %clang -target x86_64-unknown-freebsd --analyze %s
+
+#include "Inputs/system-header-simulator.h"
+
+#define M_ZERO 0x0100
+#define NULL ((void *)0)
+
+void *malloc(size_t, void *, int);
+
+struct test {
+};
+
+void foo(struct test *);
+
+void test_zeroed() {
+ struct test **list, *t;
+ int i;
+
+ list = malloc(sizeof(*list) * 10, NULL, M_ZERO);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < 10; i++) {
+ t = list[i];
+ foo(t);
+ }
+ free(list); // no-warning
+}
+
+void test_nonzero() {
+ struct test **list, *t;
+ int i;
+
+ list = malloc(sizeof(*list) * 10, NULL, 0);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < 10; i++) {
+ t = list[i]; // expected-warning{{undefined}}
+ foo(t);
+ }
+ free(list);
+}
+
+void test_indeterminate(int flags) {
+ struct test **list, *t;
+ int i;
+
+ list = malloc(sizeof(*list) * 10, NULL, flags);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < 10; i++) {
+ t = list[i]; // expected-warning{{undefined}}
+ foo(t);
+ }
+ free(list);
+}
--
1.8.5.3
More information about the cfe-commits
mailing list