Skip to content

Commit

Permalink
[clang-tidy] Add a close-on-exec check on accept4() in Android module.
Browse files Browse the repository at this point in the history
Summary:
accept4() is better to set SOCK_CLOEXEC flag to avoid file descriptor leakage.

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

llvm-svn: 311027
chih-hung committed Aug 16, 2017
1 parent 3d523a6 commit 5ac20c9
Showing 8 changed files with 169 additions and 0 deletions.
2 changes: 2 additions & 0 deletions clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "CloexecAccept4Check.h"
#include "CloexecAcceptCheck.h"
#include "CloexecCreatCheck.h"
#include "CloexecDupCheck.h"
@@ -30,6 +31,7 @@ namespace android {
class AndroidModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<CloexecAccept4Check>("android-cloexec-accept4");
CheckFactories.registerCheck<CloexecAcceptCheck>("android-cloexec-accept");
CheckFactories.registerCheck<CloexecCreatCheck>("android-cloexec-creat");
CheckFactories.registerCheck<CloexecDupCheck>("android-cloexec-dup");
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/android/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)

add_clang_library(clangTidyAndroidModule
AndroidTidyModule.cpp
CloexecAccept4Check.cpp
CloexecAcceptCheck.cpp
CloexecCheck.cpp
CloexecCreatCheck.cpp
40 changes: 40 additions & 0 deletions clang-tools-extra/clang-tidy/android/CloexecAccept4Check.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//===--- CloexecAccept4Check.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 "CloexecAccept4Check.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 {

void CloexecAccept4Check::registerMatchers(MatchFinder *Finder) {
auto SockAddrPointerType =
hasType(pointsTo(recordDecl(isStruct(), hasName("sockaddr"))));
auto SockLenPointerType = hasType(pointsTo(namedDecl(hasName("socklen_t"))));

registerMatchersImpl(Finder,
functionDecl(returns(isInteger()), hasName("accept4"),
hasParameter(0, hasType(isInteger())),
hasParameter(1, SockAddrPointerType),
hasParameter(2, SockLenPointerType),
hasParameter(3, hasType(isInteger()))));
}

void CloexecAccept4Check::check(const MatchFinder::MatchResult &Result) {
insertMacroFlag(Result, /*MarcoFlag=*/"SOCK_CLOEXEC", /*ArgPos=*/3);
}

} // namespace android
} // namespace tidy
} // namespace clang
35 changes: 35 additions & 0 deletions clang-tools-extra/clang-tidy/android/CloexecAccept4Check.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//===--- CloexecAccept4Check.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_ACCEPT4_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_ACCEPT4_H

#include "CloexecCheck.h"

namespace clang {
namespace tidy {
namespace android {

/// Finds code that uses accept4() without using the SOCK_CLOEXEC flag.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-accept4.html
class CloexecAccept4Check : public CloexecCheck {
public:
CloexecAccept4Check(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_ACCEPT4_H
6 changes: 6 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
@@ -75,6 +75,12 @@ Improvements to clang-tidy

Detects usage of ``accept()``.

- New `android-cloexec-accept4
<http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-accept4.html>`_ check

Checks if the required file flag ``SOCK_CLOEXEC`` is present in the argument of
``accept4()``.

- New `android-cloexec-dup
<http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-dup.html>`_ check

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
.. title:: clang-tidy - android-cloexec-accept4

android-cloexec-accept4
=======================

``accept4()`` 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++

accept4(sockfd, addr, addrlen, SOCK_NONBLOCK);

// becomes

accept4(sockfd, addr, addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ Clang-Tidy Checks

.. toctree::
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
android-cloexec-dup
android-cloexec-fopen
66 changes: 66 additions & 0 deletions clang-tools-extra/test/clang-tidy/android-cloexec-accept4.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// RUN: %check_clang_tidy %s android-cloexec-accept4 %t

typedef int socklen_t;
struct sockaddr {};

#define SOCK_NONBLOCK 1
#define __O_CLOEXEC 3
#define SOCK_CLOEXEC __O_CLOEXEC
#define TEMP_FAILURE_RETRY(exp) \
({ \
int _rc; \
do { \
_rc = (exp); \
} while (_rc == -1); \
})
#define NULL 0

extern "C" int accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags);

void a() {
accept4(0, NULL, NULL, SOCK_NONBLOCK);
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 'accept4' should use SOCK_CLOEXEC where possible [android-cloexec-accept4]
// CHECK-FIXES: accept4(0, NULL, NULL, SOCK_NONBLOCK | SOCK_CLOEXEC);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, SOCK_NONBLOCK));
// CHECK-MESSAGES: :[[@LINE-1]]:58: warning: 'accept4'
// CHECK-FIXES: TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, SOCK_NONBLOCK | SOCK_CLOEXEC));
}

void f() {
accept4(0, NULL, NULL, 3);
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'accept4'
// CHECK-FIXES: accept4(0, NULL, NULL, 3 | SOCK_CLOEXEC);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, 3));
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: 'accept4'
// CHECK-FIXES: TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, 3 | SOCK_CLOEXEC));

int flag = SOCK_NONBLOCK;
accept4(0, NULL, NULL, flag);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, flag));
}

namespace i {
int accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags);

void d() {
accept4(0, NULL, NULL, SOCK_NONBLOCK);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, SOCK_NONBLOCK));
}

} // namespace i

void e() {
accept4(0, NULL, NULL, SOCK_CLOEXEC);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, SOCK_CLOEXEC));
accept4(0, NULL, NULL, SOCK_NONBLOCK | SOCK_CLOEXEC);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, SOCK_NONBLOCK | SOCK_CLOEXEC));
}

class G {
public:
int accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags);
void d() {
accept4(0, NULL, NULL, SOCK_NONBLOCK);
TEMP_FAILURE_RETRY(accept4(0, NULL, NULL, SOCK_NONBLOCK));
}
};

0 comments on commit 5ac20c9

Please sign in to comment.