Author: yawanng Date: Wed Jul 12 10:43:36 2017 New Revision: 307818 URL: http://llvm.org/viewvc/llvm-project?rev=307818&view=rev Log: [clang-tidy] Add a new Android check "android-cloexec-socket"
Summary: socket() is better to include SOCK_CLOEXEC in its type argument to avoid the file descriptor leakage. Reviewers: chh, Eugene.Zelenko, alexfh, hokein, aaron.ballman Reviewed By: chh, alexfh Subscribers: srhines, mgorny, JDevlieghere, xazax.hun, cfe-commits Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D34913 Added: clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/android-cloexec-socket.rst clang-tools-extra/trunk/test/clang-tidy/android-cloexec-socket.cpp Modified: clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp clang-tools-extra/trunk/clang-tidy/android/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h clang-tools-extra/trunk/docs/ReleaseNotes.rst clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Modified: clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp Wed Jul 12 10:43:36 2017 @@ -13,6 +13,7 @@ #include "CloexecCreatCheck.h" #include "CloexecFopenCheck.h" #include "CloexecOpenCheck.h" +#include "CloexecSocketCheck.h" using namespace clang::ast_matchers; @@ -27,6 +28,7 @@ public: CheckFactories.registerCheck<CloexecCreatCheck>("android-cloexec-creat"); CheckFactories.registerCheck<CloexecFopenCheck>("android-cloexec-fopen"); CheckFactories.registerCheck<CloexecOpenCheck>("android-cloexec-open"); + CheckFactories.registerCheck<CloexecSocketCheck>("android-cloexec-socket"); } }; Modified: clang-tools-extra/trunk/clang-tidy/android/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CMakeLists.txt?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/android/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/android/CMakeLists.txt Wed Jul 12 10:43:36 2017 @@ -5,6 +5,7 @@ add_clang_library(clangTidyAndroidModule CloexecCreatCheck.cpp CloexecFopenCheck.cpp CloexecOpenCheck.cpp + CloexecSocketCheck.cpp LINK_LIBS clangAST Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp Wed Jul 12 10:43:36 2017 @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "CloexecOpenCheck.h" +#include "../utils/ASTUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" @@ -18,35 +19,8 @@ namespace clang { namespace tidy { namespace android { -namespace { static constexpr const char *O_CLOEXEC = "O_CLOEXEC"; -bool hasCloseOnExecFlag(const Expr *Flags, const SourceManager &SM, - const LangOptions &LangOpts) { - // If the Flag is an integer constant, check it. - if (isa<IntegerLiteral>(Flags)) { - if (!SM.isMacroBodyExpansion(Flags->getLocStart()) && - !SM.isMacroArgExpansion(Flags->getLocStart())) - return false; - - // Get the Marco name. - auto MacroName = Lexer::getSourceText( - CharSourceRange::getTokenRange(Flags->getSourceRange()), SM, LangOpts); - - return MacroName == O_CLOEXEC; - } - // If it's a binary OR operation. - if (const auto *BO = dyn_cast<BinaryOperator>(Flags)) - if (BO->getOpcode() == clang::BinaryOperatorKind::BO_Or) - return hasCloseOnExecFlag(BO->getLHS()->IgnoreParenCasts(), SM, - LangOpts) || - hasCloseOnExecFlag(BO->getRHS()->IgnoreParenCasts(), SM, LangOpts); - - // Otherwise, assume it has the flag. - return true; -} -} // namespace - void CloexecOpenCheck::registerMatchers(MatchFinder *Finder) { auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter()))); @@ -82,8 +56,8 @@ void CloexecOpenCheck::check(const Match // Check the required flag. SourceManager &SM = *Result.SourceManager; - if (hasCloseOnExecFlag(FlagArg->IgnoreParenCasts(), SM, - Result.Context->getLangOpts())) + if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM, + Result.Context->getLangOpts(), O_CLOEXEC)) return; SourceLocation EndLoc = Added: clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp?rev=307818&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp (added) +++ clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp Wed Jul 12 10:43:36 2017 @@ -0,0 +1,57 @@ +//===--- CloexecSocketCheck.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 "CloexecSocketCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace android { + +static constexpr const char *SOCK_CLOEXEC = "SOCK_CLOEXEC"; + +void CloexecSocketCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(callee(functionDecl(isExternC(), returns(isInteger()), + hasName("socket"), + hasParameter(0, hasType(isInteger())), + hasParameter(1, hasType(isInteger())), + hasParameter(2, hasType(isInteger()))) + .bind("funcDecl"))) + .bind("socketFn"), + this); +} + +void CloexecSocketCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("socketFn"); + const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl"); + const Expr *FlagArg = MatchedCall->getArg(1); + SourceManager &SM = *Result.SourceManager; + + if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM, + Result.Context->getLangOpts(), SOCK_CLOEXEC)) + return; + + SourceLocation EndLoc = + Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM, + Result.Context->getLangOpts()); + + diag(EndLoc, "%0 should use %1 where possible") + << FD << SOCK_CLOEXEC + << FixItHint::CreateInsertion(EndLoc, + (Twine(" | ") + SOCK_CLOEXEC).str()); +} + +} // namespace android +} // namespace tidy +} // namespace clang Added: clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h?rev=307818&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h (added) +++ clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h Wed Jul 12 10:43:36 2017 @@ -0,0 +1,35 @@ +//===--- CloexecSocketCheck.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_SOCKET_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_SOCKET_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace android { + +/// Finds code that uses socket() without using the SOCK_CLOEXEC flag. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-socket.html +class CloexecSocketCheck : public ClangTidyCheck { +public: + CloexecSocketCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(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_SOCKET_H Modified: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp Wed Jul 12 10:43:36 2017 @@ -11,6 +11,7 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" namespace clang { namespace tidy { @@ -39,6 +40,33 @@ bool IsBinaryOrTernary(const Expr *E) { return false; } +bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM, + const LangOptions &LangOpts, + StringRef FlagName) { + // If the Flag is an integer constant, check it. + if (isa<IntegerLiteral>(Flags)) { + if (!SM.isMacroBodyExpansion(Flags->getLocStart()) && + !SM.isMacroArgExpansion(Flags->getLocStart())) + return false; + + // Get the marco name. + auto MacroName = Lexer::getSourceText( + CharSourceRange::getTokenRange(Flags->getSourceRange()), SM, LangOpts); + + return MacroName == FlagName; + } + // If it's a binary OR operation. + if (const auto *BO = dyn_cast<BinaryOperator>(Flags)) + if (BO->getOpcode() == clang::BinaryOperatorKind::BO_Or) + return exprHasBitFlagWithSpelling(BO->getLHS()->IgnoreParenCasts(), SM, + LangOpts, FlagName) || + exprHasBitFlagWithSpelling(BO->getRHS()->IgnoreParenCasts(), SM, + LangOpts, FlagName); + + // Otherwise, assume it has the flag. + return true; +} + } // namespace utils } // namespace tidy } // namespace clang Modified: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h (original) +++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h Wed Jul 12 10:43:36 2017 @@ -20,6 +20,13 @@ const FunctionDecl *getSurroundingFuncti const Stmt &Statement); // Determine whether Expr is a Binary or Ternary expression. bool IsBinaryOrTernary(const Expr *E); + +/// Checks whether a macro flag is present in the given argument. Only considers +/// cases of single match or match in a binary OR expression. For example, +/// <needed-flag> or <flag> | <needed-flag> | ... +bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM, + const LangOptions &LangOpts, + StringRef FlagName); } // namespace utils } // namespace tidy } // namespace clang Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original) +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Jul 12 10:43:36 2017 @@ -73,6 +73,12 @@ Improvements to clang-tidy Checks if the required mode ``e`` exists in the mode argument of ``fopen()``. +- New `android-cloexec-socket + <http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-socket.html>`_ check + + Checks if the required file flag ``SOCK_CLOEXEC`` is present in the argument of + ``socket()``. + - New `cert-dcl21-cpp <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html>`_ check Added: clang-tools-extra/trunk/docs/clang-tidy/checks/android-cloexec-socket.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/android-cloexec-socket.rst?rev=307818&view=auto ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/android-cloexec-socket.rst (added) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/android-cloexec-socket.rst Wed Jul 12 10:43:36 2017 @@ -0,0 +1,18 @@ +.. title:: clang-tidy - android-cloexec-socket + +android-cloexec-socket +====================== + +``socket()`` should include ``SOCK_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++ + + socket(domain, type, SOCK_STREAM); + + // becomes + + socket(domain, type, SOCK_STREAM | SOCK_CLOEXEC); Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=307818&r1=307817&r2=307818&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Jul 12 10:43:36 2017 @@ -7,6 +7,7 @@ Clang-Tidy Checks android-cloexec-creat android-cloexec-fopen android-cloexec-open + android-cloexec-socket boost-use-to-string cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c> cert-dcl21-cpp Added: clang-tools-extra/trunk/test/clang-tidy/android-cloexec-socket.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/android-cloexec-socket.cpp?rev=307818&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/android-cloexec-socket.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/android-cloexec-socket.cpp Wed Jul 12 10:43:36 2017 @@ -0,0 +1,75 @@ +// RUN: %check_clang_tidy %s android-cloexec-socket %t + +#define SOCK_STREAM 1 +#define SOCK_DGRAM 2 +#define __O_CLOEXEC 3 +#define SOCK_CLOEXEC __O_CLOEXEC +#define TEMP_FAILURE_RETRY(exp) \ + ({ \ + int _rc; \ + do { \ + _rc = (exp); \ + } while (_rc == -1); \ + }) + +extern "C" int socket(int domain, int type, int protocol); + +void a() { + socket(0, SOCK_STREAM, 0); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket] + // CHECK-FIXES: socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0) + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0)); + // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: 'socket' + // CHECK-FIXES: TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0)) + socket(0, SOCK_STREAM | SOCK_DGRAM, 0); + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'socket' + // CHECK-FIXES: socket(0, SOCK_STREAM | SOCK_DGRAM | SOCK_CLOEXEC, 0) + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM, 0)); + // CHECK-MESSAGES: :[[@LINE-1]]:56: warning: 'socket' + // CHECK-FIXES: TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM | SOCK_CLOEXEC, 0)) +} + +void f() { + socket(0, 3, 0); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'socket' + // CHECK-FIXES: socket(0, 3 | SOCK_CLOEXEC, 0) + TEMP_FAILURE_RETRY(socket(0, 3, 0)); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'socket' + // CHECK-FIXES: TEMP_FAILURE_RETRY(socket(0, 3 | SOCK_CLOEXEC, 0)) + + int flag = 3; + socket(0, flag, 0); + TEMP_FAILURE_RETRY(socket(0, flag, 0)); +} + +namespace i { +int socket(int domain, int type, int protocol); + +void d() { + socket(0, SOCK_STREAM, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0)); + socket(0, SOCK_STREAM | SOCK_DGRAM, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM, 0)); +} + +} // namespace i + +void e() { + socket(0, SOCK_CLOEXEC, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_CLOEXEC, 0)); + socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0)); + socket(0, SOCK_STREAM | SOCK_CLOEXEC | SOCK_DGRAM, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_CLOEXEC | SOCK_DGRAM, 0)); +} + +class G { +public: + int socket(int domain, int type, int protocol); + void d() { + socket(0, SOCK_STREAM, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0)); + socket(0, SOCK_STREAM | SOCK_DGRAM, 0); + TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM, 0)); + } +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
