maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clang-tidy] Rename android-file-open-flag and fix a bug 

Summary:
1. Rename android-file-open-flag to android-cloexec-open.
2. Handle a case when the function is passed as an argument of a function-like macro.

Reviewers: chh

Reviewed By: chh

Subscribers: srhines, mgorny, JDevlieghere, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D34633

llvm-svn: 306728
This commit is contained in:
Yan Wang 2017-06-29 19:13:29 +00:00
parent 99886f09a1
commit 600a6133ad
9 changed files with 208 additions and 136 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
View File

@ -12,7 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "CloexecCreatCheck.h"
#include "CloexecFopenCheck.h"
#include "FileOpenFlagCheck.h"
#include "CloexecOpenCheck.h"
using namespace clang::ast_matchers;
@ -24,9 +24,9 @@ namespace android {
class AndroidModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<FileOpenFlagCheck>("android-file-open-flag");
CheckFactories.registerCheck<CloexecCreatCheck>("android-cloexec-creat");
CheckFactories.registerCheck<CloexecFopenCheck>("android-cloexec-fopen");
CheckFactories.registerCheck<CloexecOpenCheck>("android-cloexec-open");
}
};

2
clang-tools-extra/clang-tidy/android/CMakeLists.txt
View File

@ -4,7 +4,7 @@ add_clang_library(clangTidyAndroidModule
AndroidTidyModule.cpp
CloexecCreatCheck.cpp
CloexecFopenCheck.cpp
FileOpenFlagCheck.cpp
CloexecOpenCheck.cpp
LINK_LIBS
clangAST

24
clang-tools-extra/clang-tidy/android/FileOpenFlagCheck.cpp → clang-tools-extra/clang-tidy/android/CloexecOpenCheck.cpp
View File

@ -1,4 +1,4 @@
//===--- FileOpenFlagCheck.cpp - clang-tidy--------------------------------===//
//===--- CloexecOpenCheck.cpp - clang-tidy---------------------------------===//
//
// The LLVM Compiler Infrastructure
//
@ -7,7 +7,7 @@
//
//===----------------------------------------------------------------------===//
#include "FileOpenFlagCheck.h"
#include "CloexecOpenCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
@ -21,11 +21,12 @@ namespace android {
namespace {
static constexpr const char *O_CLOEXEC = "O_CLOEXEC";
bool HasCloseOnExecFlag(const Expr *Flags, const SourceManager &SM,
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()))
if (!SM.isMacroBodyExpansion(Flags->getLocStart()) &&
!SM.isMacroArgExpansion(Flags->getLocStart()))
return false;
// Get the Marco name.
@ -37,16 +38,16 @@ bool HasCloseOnExecFlag(const Expr *Flags, const SourceManager &SM,
// 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,
return hasCloseOnExecFlag(BO->getLHS()->IgnoreParenCasts(), SM,
LangOpts) ||
HasCloseOnExecFlag(BO->getRHS()->IgnoreParenCasts(), SM, LangOpts);
hasCloseOnExecFlag(BO->getRHS()->IgnoreParenCasts(), SM, LangOpts);
// Otherwise, assume it has the flag.
return true;
}
} // namespace
void FileOpenFlagCheck::registerMatchers(MatchFinder *Finder) {
void CloexecOpenCheck::registerMatchers(MatchFinder *Finder) {
auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
Finder->addMatcher(
@ -68,7 +69,7 @@ void FileOpenFlagCheck::registerMatchers(MatchFinder *Finder) {
this);
}
void FileOpenFlagCheck::check(const MatchFinder::MatchResult &Result) {
void CloexecOpenCheck::check(const MatchFinder::MatchResult &Result) {
const Expr *FlagArg = nullptr;
if (const auto *OpenFnCall = Result.Nodes.getNodeAs<CallExpr>("openFn"))
FlagArg = OpenFnCall->getArg(1);
@ -81,12 +82,13 @@ void FileOpenFlagCheck::check(const MatchFinder::MatchResult &Result) {
// Check the required flag.
SourceManager &SM = *Result.SourceManager;
if (HasCloseOnExecFlag(FlagArg->IgnoreParenCasts(), SM,
if (hasCloseOnExecFlag(FlagArg->IgnoreParenCasts(), SM,
Result.Context->getLangOpts()))
return;
SourceLocation EndLoc = Lexer::getLocForEndOfToken(
FlagArg->getLocEnd(), 0, SM, Result.Context->getLangOpts());
SourceLocation EndLoc =
Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
Result.Context->getLangOpts());
diag(EndLoc, "%0 should use %1 where possible")
<< FD << O_CLOEXEC

12
clang-tools-extra/clang-tidy/android/FileOpenFlagCheck.h → clang-tools-extra/clang-tidy/android/CloexecOpenCheck.h
View File

@ -1,4 +1,4 @@
//===--- FileOpenFlagCheck.h - clang-tidy----------------------------------===//
//===--- CloexecOpenCheck.h - clang-tidy-----------------------------------===//
//
// The LLVM Compiler Infrastructure
//
@ -7,8 +7,8 @@
//
//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_FILE_OPEN_FLAG_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_FILE_OPEN_FLAG_H
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H
#include "../ClangTidy.h"
@ -25,9 +25,9 @@ namespace android {
///
/// Only the symbolic 'O_CLOEXEC' macro definition is checked, not the concrete
/// value.
class FileOpenFlagCheck : public ClangTidyCheck {
class CloexecOpenCheck : public ClangTidyCheck {
public:
FileOpenFlagCheck(StringRef Name, ClangTidyContext *Context)
CloexecOpenCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@ -37,4 +37,4 @@ public:
} // namespace tidy
} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_FILE_OPEN_FLAG_H
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H

4
clang-tools-extra/docs/ReleaseNotes.rst
View File

@ -62,8 +62,8 @@ Improvements to clang-tidy
Detect usage of ``creat()``.
- New `android-file-open-flag
<http://clang.llvm.org/extra/clang-tidy/checks/android-file-open-flag.html>`_ check
- New `android-cloexec-open
<http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-open.html>`_ check
Checks if the required file flag ``O_CLOEXEC`` exists in ``open()``,
``open64()`` and ``openat()``.

6
clang-tools-extra/docs/clang-tidy/checks/android-file-open-flag.rst → clang-tools-extra/docs/clang-tidy/checks/android-cloexec-open.rst
View File

@ -1,7 +1,7 @@
.. title:: clang-tidy - android-file-open-flag
.. title:: clang-tidy - android-cloexec-open
android-file-open-flag
======================
android-cloexec-open
====================
A common source of security bugs is code that opens a file without using the
``O_CLOEXEC`` flag. Without that flag, an opened sensitive file would remain

2
clang-tools-extra/docs/clang-tidy/checks/list.rst
View File

@ -6,7 +6,7 @@ Clang-Tidy Checks
.. toctree::
android-cloexec-creat
android-cloexec-fopen
android-file-open-flag
android-cloexec-open
boost-use-to-string
cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
cert-dcl21-cpp

180
clang-tools-extra/test/clang-tidy/android-cloexec-open.cpp Normal file
View File

@ -0,0 +1,180 @@
// RUN: %check_clang_tidy %s android-cloexec-open %t
#define O_RDWR 1
#define O_EXCL 2
#define __O_CLOEXEC 3
#define O_CLOEXEC __O_CLOEXEC
#define TEMP_FAILURE_RETRY(exp) \
({ \
int _rc; \
do { \
_rc = (exp); \
} while (_rc == -1); \
})
extern "C" int open(const char *fn, int flags, ...);
extern "C" int open64(const char *fn, int flags, ...);
extern "C" int openat(int dirfd, const char *pathname, int flags, ...);
void a() {
open("filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'open' should use O_CLOEXEC where possible [android-cloexec-open]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
TEMP_FAILURE_RETRY(open("filename", O_RDWR));
// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'open' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_CLOEXEC
open("filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: 'open' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
TEMP_FAILURE_RETRY(open("filename", O_RDWR | O_EXCL));
// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'open' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void b() {
open64("filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'open64' should use O_CLOEXEC where possible [android-cloexec-open]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
// CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'open64' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_CLOEXEC
open64("filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'open64' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
TEMP_FAILURE_RETRY(open64("filename", O_RDWR | O_EXCL));
// CHECK-MESSAGES: :[[@LINE-1]]:56: warning: 'open64' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void c() {
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'openat' should use O_CLOEXEC where possible [android-cloexec-open]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
// CHECK-MESSAGES: :[[@LINE-1]]:50: warning: 'openat' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_CLOEXEC
openat(0, "filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'openat' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR | O_EXCL));
// CHECK-MESSAGES: :[[@LINE-1]]:59: warning: 'openat' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void f() {
open("filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'open' should use O_CLOEXEC where possible [android-cloexec-open]
// CHECK-FIXES: 3 | O_CLOEXEC
TEMP_FAILURE_RETRY(open("filename", 3));
// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'open' should use O_CLOEXEC where
// CHECK-FIXES: 3 | O_CLOEXEC
open64("filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'open64' should use O_CLOEXEC where possible [android-cloexec-open]
// CHECK-FIXES: 3 | O_CLOEXEC
TEMP_FAILURE_RETRY(open64("filename", 3));
// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: 'open64' should use O_CLOEXEC where
// CHECK-FIXES: 3 | O_CLOEXEC
openat(0, "filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'openat' should use O_CLOEXEC where possible [android-cloexec-open]
// CHECK-FIXES: 3 | O_CLOEXEC
TEMP_FAILURE_RETRY(openat(0, "filename", 3));
// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'openat' should use O_CLOEXEC where
// CHECK-FIXES: 3 | O_CLOEXEC
int flag = 3;
open("filename", flag);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open("filename", flag));
// CHECK-MESSAGES-NOT: warning:
open64("filename", flag);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open64("filename", flag));
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", flag);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(openat(0, "filename", flag));
// CHECK-MESSAGES-NOT: warning:
}
namespace i {
int open(const char *pathname, int flags, ...);
int open64(const char *pathname, int flags, ...);
int openat(int dirfd, const char *pathname, int flags, ...);
void d() {
open("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open("filename", O_RDWR));
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
// CHECK-MESSAGES-NOT: warning:
}
} // namespace i
void e() {
open("filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open("filename", O_CLOEXEC));
// CHECK-MESSAGES-NOT: warning:
open("filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open("filename", O_RDWR | O_CLOEXEC));
// CHECK-MESSAGES-NOT: warning:
open("filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open("filename", O_RDWR | O_CLOEXEC | O_EXCL));
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open64("filename", O_CLOEXEC));
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open64("filename", O_RDWR | O_CLOEXEC));
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open64("filename", O_RDWR | O_CLOEXEC | O_EXCL));
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(openat(0, "filename", O_CLOEXEC));
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR | O_CLOEXEC));
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR | O_CLOEXEC | O_EXCL));
// CHECK-MESSAGES-NOT: warning:
}
class G {
public:
int open(const char *pathname, int flags, ...);
int open64(const char *pathname, int flags, ...);
int openat(int dirfd, const char *pathname, int flags, ...);
void h() {
open("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open("filename", O_RDWR));
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
// CHECK-MESSAGES-NOT: warning:
}
};

110
clang-tools-extra/test/clang-tidy/android-file-open-flag.cpp
View File

@ -1,110 +0,0 @@
// RUN: %check_clang_tidy %s android-file-open-flag %t
#define O_RDWR 1
#define O_EXCL 2
#define __O_CLOEXEC 3
#define O_CLOEXEC __O_CLOEXEC
extern "C" int open(const char *fn, int flags, ...);
extern "C" int open64(const char *fn, int flags, ...);
extern "C" int openat(int dirfd, const char *pathname, int flags, ...);
void a() {
open("filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
open("filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: 'open' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void b() {
open64("filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
open64("filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'open64' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void c() {
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
openat(0, "filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'openat' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void f() {
open("filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: 3 | O_CLOEXEC
open64("filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: 3 | O_CLOEXEC
openat(0, "filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: 3 | O_CLOEXEC
int flag = 3;
open("filename", flag);
// CHECK-MESSAGES-NOT: warning:
open64("filename", flag);
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", flag);
// CHECK-MESSAGES-NOT: warning:
}
namespace i {
int open(const char *pathname, int flags, ...);
int open64(const char *pathname, int flags, ...);
int openat(int dirfd, const char *pathname, int flags, ...);
void d() {
open("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
}
} // namespace i
void e() {
open("filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
open("filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
open("filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
}
class G {
public:
int open(const char *pathname, int flags, ...);
int open64(const char *pathname, int flags, ...);
int openat(int dirfd, const char *pathname, int flags, ...);
void h() {
open("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
}
};