[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.
Dmitri Gribenko via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 31 09:52:01 PDT 2019
gribozavr added inline comments.
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31
+ "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC",
> the message doesn't seem to explain the reason, especially to the people who are not familiar with the `pipe` API, maybe `prefer pipe2() to pipe() to avoid file descriptor leakage` is clearer?
> Ah, it looks like you are following the existing `CloexecCreatCheck` check, no need to do it in this patch.
"prefer pipe2() with O_CLOEXEC to avoid leaking file descriptors to child processes"
No need to change in this patch if you're following an existing pattern, but please do update the message in all checks in a separate patch to explain things better.
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h:18
+/// pipe() is better to be replaced by pipe2().
"Suggests to replace calls to pipe() with calls to pipe2()."
Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:6
+Detects usage of ``pipe()``. The usage of ``pipe()`` is not recommended, it's better to use ``pipe2()``.
+Without this flag, an opened sensitive file descriptor would remain open across
WDYT about this rewrite?
This check detects usage of pipe(). Using pipe() is not recommended, pipe2() is the suggested replacement.
The check also adds the O_CLOEXEC flag that marks the file descriptor to be closed in child processes. Without this flag a sensitive file descriptor can be leaked to a child process, potentially into a lower-privileged SELinux domain.
Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:16
+ // becomes
Also use a separate code block for the replacement.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits