yawanng updated this revision to Diff 107557.
yawanng added a comment.
Refactor the check, add a base class for it, which can facilitate all other
similar checks. Basically, all checks in the same category will have only one
or two lines code by inheriting the base class. If this looks good, I will
modify all other similar ones. Thank you :-)
https://reviews.llvm.org/D35372
Files:
clang-tidy/android/AndroidTidyModule.cpp
clang-tidy/android/CMakeLists.txt
clang-tidy/android/CloexecCheck.cpp
clang-tidy/android/CloexecCheck.h
clang-tidy/android/CloexecMemfdCreateCheck.cpp
clang-tidy/android/CloexecMemfdCreateCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/android-cloexec-memfd-create.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/android-cloexec-memfd-create.cpp
Index: test/clang-tidy/android-cloexec-memfd-create.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/android-cloexec-memfd-create.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s android-cloexec-memfd-create %t
+
+#define MFD_ALLOW_SEALING 1
+#define __O_CLOEXEC 3
+#define MFD_CLOEXEC __O_CLOEXEC
+#define TEMP_FAILURE_RETRY(exp) \
+ ({ \
+ int _rc; \
+ do { \
+ _rc = (exp); \
+ } while (_rc == -1); \
+ })
+#define NULL 0
+
+extern "C" int memfd_create(const char *name, unsigned int flags);
+
+void a() {
+ memfd_create(NULL, MFD_ALLOW_SEALING);
+ // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 'memfd_create' should use MFD_CLOEXEC where possible [android-cloexec-memfd-create]
+ // CHECK-FIXES: memfd_create(NULL, MFD_ALLOW_SEALING | MFD_CLOEXEC)
+ TEMP_FAILURE_RETRY(memfd_create(NULL, MFD_ALLOW_SEALING));
+ // CHECK-MESSAGES: :[[@LINE-1]]:58: warning: 'memfd_create'
+ // CHECK-FIXES: TEMP_FAILURE_RETRY(memfd_create(NULL, MFD_ALLOW_SEALING | MFD_CLOEXEC))
+}
+
+void f() {
+ memfd_create(NULL, 3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'memfd_create'
+ // CHECK-FIXES: memfd_create(NULL, 3 | MFD_CLOEXEC)
+ TEMP_FAILURE_RETRY(memfd_create(NULL, 3));
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: 'memfd_create'
+ // CHECK-FIXES: TEMP_FAILURE_RETRY(memfd_create(NULL, 3 | MFD_CLOEXEC))
+
+ int flag = 3;
+ memfd_create(NULL, flag);
+ TEMP_FAILURE_RETRY(memfd_create(NULL, flag));
+}
+
+namespace i {
+int memfd_create(const char *name, unsigned int flags);
+
+void d() {
+ memfd_create(NULL, MFD_ALLOW_SEALING);
+ TEMP_FAILURE_RETRY(memfd_create(NULL, MFD_ALLOW_SEALING));
+}
+
+} // namespace i
+
+void e() {
+ memfd_create(NULL, MFD_CLOEXEC);
+ TEMP_FAILURE_RETRY(memfd_create(NULL, MFD_CLOEXEC));
+ memfd_create(NULL, MFD_ALLOW_SEALING | MFD_CLOEXEC);
+ TEMP_FAILURE_RETRY(memfd_create(NULL, MFD_ALLOW_SEALING | MFD_CLOEXEC));
+}
+
+class G {
+public:
+ int memfd_create(const char *name, unsigned int flags);
+ void d() {
+ memfd_create(NULL, MFD_ALLOW_SEALING);
+ TEMP_FAILURE_RETRY(memfd_create(NULL, MFD_ALLOW_SEALING));
+ }
+};
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
.. toctree::
android-cloexec-creat
android-cloexec-fopen
+ android-cloexec-memfd-create
android-cloexec-open
android-cloexec-socket
boost-use-to-string
Index: docs/clang-tidy/checks/android-cloexec-memfd-create.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/android-cloexec-memfd-create.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - android-cloexec-memfd-create
+
+android-cloexec-memfd-create
+============================
+
+``memfd_create()`` should include ``MFD_CLOEXEC`` in its type argument to avoid
+the file descriptor leakage. Without this flag, an opened sensitive file would
+remain open across a fork+exec to a lower-privileged SELinux domain.
+
+Examples:
+
+.. code-block:: c++
+
+ memfd_create(name, MFD_ALLOW_SEALING);
+
+ // becomes
+
+ memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,12 @@
Checks if the required mode ``e`` exists in the mode argument of ``fopen()``.
+- New `android-cloexec-memfd_create
+ <http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-memfd_create.html>`_ check
+
+ Checks if the required file flag ``MFD_CLOEXEC`` is present in the argument of
+ ``memfd_create()``.
+
- New `android-cloexec-socket
<http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-socket.html>`_ check
Index: clang-tidy/android/CloexecMemfdCreateCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/android/CloexecMemfdCreateCheck.h
@@ -0,0 +1,35 @@
+//===--- CloexecMemfdCreateCheck.h - clang-tidy-----------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_MEMFD_CREATE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_MEMFD_CREATE_H
+
+#include "CloexecCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+/// Finds code that uses memfd_create() without using the MFD_CLOEXEC flag.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-memfd-create.html
+class CloexecMemfdCreateCheck : public CloexecCheck {
+public:
+ CloexecMemfdCreateCheck(StringRef Name, ClangTidyContext *Context)
+ : CloexecCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_MEMFD_CREATE_H
Index: clang-tidy/android/CloexecMemfdCreateCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/android/CloexecMemfdCreateCheck.cpp
@@ -0,0 +1,29 @@
+//===--- CloexecMemfdCreateCheck.cpp - clang-tidy--------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CloexecMemfdCreateCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+void CloexecMemfdCreateCheck::registerMatchers(MatchFinder *Finder) {
+ doRegisterMatchers(Finder, returnsInteger(), hasName("memfd_create"),
+ hasCharPointerTypeParameter(0), hasIntegerParameter(1));
+}
+
+void CloexecMemfdCreateCheck::check(const MatchFinder::MatchResult &Result) {
+ doMacroFlagInsertion(Result, "MFD_CLOEXEC", 1);
+}
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/android/CloexecCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/android/CloexecCheck.h
@@ -0,0 +1,105 @@
+//===--- CloexecCheck.h - clang-tidy-----------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_H
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+/// The base class for all close-on-exec checks.
+class CloexecCheck : public ClangTidyCheck {
+public:
+ CloexecCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+ // Register matchers, we assume all the checked APIs are C functions.
+ template <typename... Types>
+ void doRegisterMatchers(ast_matchers::MatchFinder *Finder, Types... Params) {
+ auto FuncDecl = std::bind(ast_matchers::functionDecl,
+ ast_matchers::isExternC(), Params...);
+ Finder->addMatcher(ast_matchers::callExpr(
+ ast_matchers::callee(FuncDecl().bind("funcDecl")))
+ .bind("func"),
+ this);
+ }
+
+ // This issue has three types.
+ // Type1 is to insert the necessary macro flag in the flag argument.
+ void
+ doMacroFlagInsertion(const ast_matchers::MatchFinder::MatchResult &Result,
+ const StringRef Flag, const int ArgPos);
+
+ // Type2 is to replace the API to another function that has required the
+ // ability.
+ void doFuncReplacement(const ast_matchers::MatchFinder::MatchResult &Result,
+ StringRef Msg, StringRef FixMsg);
+
+ // Type3 is also to add a flag to the corresponding argument, but this time,
+ // the flag is some string rather than a macro.
+ void
+ doStringFlagInsertion(const ast_matchers::MatchFinder::MatchResult &Result,
+ const StringRef Mode, const int ArgPos);
+
+ // Helper function to get the spelling of a particular argument.
+ StringRef getArgSpelling(const ast_matchers::MatchFinder::MatchResult &Result,
+ int n);
+
+ // Helper function to form the correct string mode for Type3.
+ StringRef buildFixMsgForStringFlag(const Expr *Arg, const SourceManager &SM,
+ const LangOptions &LangOpts,
+ StringRef Mode);
+
+ inline ast_matchers::internal::Matcher<FunctionDecl>
+ hasIntegerParameter(int n) {
+ return ast_matchers::hasParameter(
+ n, ast_matchers::hasType(ast_matchers::isInteger()));
+ }
+
+ inline ast_matchers::internal::Matcher<FunctionDecl>
+ hasCharPointerTypeParameter(int n) {
+ return ast_matchers::hasParameter(
+ n, ast_matchers::hasType(ast_matchers::pointerType(
+ ast_matchers::pointee(ast_matchers::isAnyCharacter()))));
+ }
+
+ inline ast_matchers::internal::Matcher<FunctionDecl>
+ hasSockAddrPointerTypeParameter(int n) {
+ return ast_matchers::hasParameter(
+ n,
+ ast_matchers::hasType(ast_matchers::pointsTo(ast_matchers::recordDecl(
+ ast_matchers::isStruct(), ast_matchers::hasName("sockaddr")))));
+ }
+
+ inline ast_matchers::internal::Matcher<FunctionDecl>
+ hasSockLenPointerTypeParameter(int n) {
+ return ast_matchers::hasParameter(
+ n, ast_matchers::hasType(ast_matchers::pointsTo(
+ ast_matchers::namedDecl(ast_matchers::hasName("socklen_t")))));
+ }
+
+ inline ast_matchers::internal::Matcher<FunctionDecl>
+ hasMODETTypeParameter(int n) {
+ return ast_matchers::hasParameter(
+ n, ast_matchers::hasType(
+ ast_matchers::namedDecl(ast_matchers::hasName("mode_t"))));
+ }
+
+ inline ast_matchers::internal::Matcher<FunctionDecl> returnsInteger() {
+ return ast_matchers::returns(ast_matchers::isInteger());
+ }
+};
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_H
Index: clang-tidy/android/CloexecCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/android/CloexecCheck.cpp
@@ -0,0 +1,99 @@
+//===--- CloexecCheck.cpp - clang-tidy-------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CloexecCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+void CloexecCheck::doMacroFlagInsertion(const MatchFinder::MatchResult &Result,
+ const StringRef Flag,
+ const int ArgPos) {
+ const auto MatchedCall = Result.Nodes.getNodeAs<CallExpr>("func");
+ const auto FlagArg = MatchedCall->getArg(ArgPos);
+ const auto FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
+ SourceManager &SM = *Result.SourceManager;
+
+ if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM,
+ Result.Context->getLangOpts(), Flag))
+ return;
+
+ SourceLocation EndLoc =
+ Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
+ Result.Context->getLangOpts());
+
+ diag(EndLoc, "%0 should use %1 where possible")
+ << FD << Flag
+ << FixItHint::CreateInsertion(EndLoc, (Twine(" | ") + Flag).str());
+}
+
+StringRef CloexecCheck::getArgSpelling(const MatchFinder::MatchResult &Result,
+ int n) {
+ const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("func");
+ const SourceManager &SM = *Result.SourceManager;
+ return Lexer::getSourceText(
+ CharSourceRange::getTokenRange(MatchedCall->getArg(n)->getSourceRange()),
+ SM, Result.Context->getLangOpts());
+}
+
+void CloexecCheck::doFuncReplacement(const MatchFinder::MatchResult &Result,
+ StringRef Msg, StringRef FixMsg) {
+ const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("func");
+ diag(MatchedCall->getLocStart(), Msg)
+ << FixItHint::CreateReplacement(MatchedCall->getSourceRange(), FixMsg);
+}
+
+void CloexecCheck::doStringFlagInsertion(
+ const ast_matchers::MatchFinder::MatchResult &Result, const StringRef Mode,
+ const int ArgPos) {
+ const auto MatchedCall = Result.Nodes.getNodeAs<CallExpr>("func");
+ const auto FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
+ const auto ModeArg = MatchedCall->getArg(ArgPos);
+
+ // Check if the <Mode> may be in the mode string.
+ const auto ModeStr = dyn_cast<StringLiteral>(ModeArg->IgnoreParenCasts());
+ if (!ModeStr || (ModeStr->getString().find(Mode) != StringRef::npos))
+ return;
+
+ const std::string &ReplacementText = buildFixMsgForStringFlag(
+ ModeArg, *Result.SourceManager, Result.Context->getLangOpts(), Mode);
+
+ diag(ModeArg->getLocStart(), "use %0 mode '%1' to set O_CLOEXEC")
+ << FD << Mode
+ << FixItHint::CreateReplacement(ModeArg->getSourceRange(),
+ ReplacementText);
+}
+
+// Build the replace text. If it's string constant, add <Mode> directly in the
+// end of the string. Else, add <Mode>.
+StringRef CloexecCheck::buildFixMsgForStringFlag(const Expr *Arg,
+ const SourceManager &SM,
+ const LangOptions &LangOpts,
+ StringRef Mode) {
+ if (Arg->getLocStart().isMacroID())
+ return (Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SM,
+ LangOpts) +
+ " \"" + Twine(Mode) + "\"")
+ .str();
+
+ StringRef SR = cast<StringLiteral>(Arg->IgnoreParenCasts())->getString();
+ return ("\"" + SR + Twine(Mode) + "\"").str();
+}
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/android/CMakeLists.txt
===================================================================
--- clang-tidy/android/CMakeLists.txt
+++ clang-tidy/android/CMakeLists.txt
@@ -2,8 +2,10 @@
add_clang_library(clangTidyAndroidModule
AndroidTidyModule.cpp
+ CloexecCheck.cpp
CloexecCreatCheck.cpp
CloexecFopenCheck.cpp
+ CloexecMemfdCreateCheck.cpp
CloexecOpenCheck.cpp
CloexecSocketCheck.cpp
Index: clang-tidy/android/AndroidTidyModule.cpp
===================================================================
--- clang-tidy/android/AndroidTidyModule.cpp
+++ clang-tidy/android/AndroidTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "CloexecCreatCheck.h"
#include "CloexecFopenCheck.h"
+#include "CloexecMemfdCreateCheck.h"
#include "CloexecOpenCheck.h"
#include "CloexecSocketCheck.h"
@@ -27,6 +28,8 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<CloexecCreatCheck>("android-cloexec-creat");
CheckFactories.registerCheck<CloexecFopenCheck>("android-cloexec-fopen");
+ CheckFactories.registerCheck<CloexecMemfdCreateCheck>(
+ "android-cloexec-memfd-create");
CheckFactories.registerCheck<CloexecOpenCheck>("android-cloexec-open");
CheckFactories.registerCheck<CloexecSocketCheck>("android-cloexec-socket");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
