libbinder: replace dup() with fcntl(F_DUPFD_CLOEXEC)
Replace calls to dup() with fcntl(F_DUPFD_CLOEXEC). The only difference
between the two is that O_CLOEXEC is set on the newly duped file
descriptor. This helps address file descriptor leaks crossing an exec()
boundary in multi-threaded processes, and potentially fixes the following
non-reproducible SELinux denials which may be occurring because of FD
leakage from netd to clatd/dnsmasq.
avc: denied { use } for comm="clatd" path="socket:[860297]" dev="sockfs"
ino=860297 scontext=u:r:clatd:s0 tcontext=u:r:untrusted_app:s0:c512,c768
tclass=fd permissive=0
avc: denied { read write } for comm="clatd" path="socket:[1414454]"
dev="sockfs" ino=1414454 scontext=u:r:clatd:s0
tcontext=u:r:system_server:s0 tclass=tcp_socket permissive=0
avc: denied { use } for comm="clatd" path="socket:[681600]" dev="sockfs"
ino=681600 scontext=u:r:clatd:s0 tcontext=u:r:priv_app:s0:c512,c768
tclass=fd permissive=0
Test: Device boots and no obvious problems
Change-Id: I9dcd9911a093f329c6f12e39d2c49ef3df651ae5
diff --git a/libs/binder/IActivityManager.cpp b/libs/binder/IActivityManager.cpp
index 898ee52..87b295a 100644
--- a/libs/binder/IActivityManager.cpp
+++ b/libs/binder/IActivityManager.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#include <unistd.h>
+#include <fcntl.h>
+
#include <binder/IActivityManager.h>
#include <binder/Parcel.h>
@@ -42,7 +45,7 @@
// Success is indicated here by a nonzero int followed by the fd;
// failure by a zero int with no data following.
if (reply.readInt32() != 0) {
- fd = dup(reply.readParcelFileDescriptor());
+ fd = fcntl(reply.readParcelFileDescriptor(), F_DUPFD_CLOEXEC, 0);
}
} else {
// An exception was thrown back; fall through to return failure
diff --git a/libs/binder/IMemory.cpp b/libs/binder/IMemory.cpp
index 790fa8c..4a39741 100644
--- a/libs/binder/IMemory.cpp
+++ b/libs/binder/IMemory.cpp
@@ -288,7 +288,7 @@
mBase = heap->mBase;
mSize = heap->mSize;
mOffset = heap->mOffset;
- int fd = dup(heap->mHeapId.load(memory_order_relaxed));
+ int fd = fcntl(heap->mHeapId.load(memory_order_relaxed), F_DUPFD_CLOEXEC, 0);
ALOGE_IF(fd==-1, "cannot dup fd=%d",
heap->mHeapId.load(memory_order_relaxed));
mHeapId.store(fd, memory_order_release);
@@ -323,7 +323,7 @@
Mutex::Autolock _l(mLock);
if (mHeapId.load(memory_order_relaxed) == -1) {
- int fd = dup( parcel_fd );
+ int fd = fcntl(parcel_fd, F_DUPFD_CLOEXEC, 0);
ALOGE_IF(fd==-1, "cannot dup fd=%d, size=%zd, err=%d (%s)",
parcel_fd, size, err, strerror(errno));
diff --git a/libs/binder/IShellCallback.cpp b/libs/binder/IShellCallback.cpp
index 8b97301..c793df3 100644
--- a/libs/binder/IShellCallback.cpp
+++ b/libs/binder/IShellCallback.cpp
@@ -16,6 +16,9 @@
#define LOG_TAG "ShellCallback"
+#include <unistd.h>
+#include <fcntl.h>
+
#include <binder/IShellCallback.h>
#include <utils/Log.h>
@@ -44,7 +47,7 @@
remote()->transact(OP_OPEN_OUTPUT_FILE, data, &reply, 0);
reply.readExceptionCode();
int fd = reply.readParcelFileDescriptor();
- return fd >= 0 ? dup(fd) : fd;
+ return fd >= 0 ? fcntl(fd, F_DUPFD_CLOEXEC, 0) : fd;
}
};
diff --git a/libs/binder/MemoryHeapBase.cpp b/libs/binder/MemoryHeapBase.cpp
index 43a01e4..b4b1062 100644
--- a/libs/binder/MemoryHeapBase.cpp
+++ b/libs/binder/MemoryHeapBase.cpp
@@ -83,7 +83,7 @@
{
const size_t pagesize = getpagesize();
size = ((size + pagesize-1) & ~(pagesize-1));
- mapfd(dup(fd), size, offset);
+ mapfd(fcntl(fd, F_DUPFD_CLOEXEC, 0), size, offset);
}
status_t MemoryHeapBase::init(int fd, void *base, int size, int flags, const char* device)
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 15c441e..a6ccb53 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -539,7 +539,7 @@
// If this is a file descriptor, we need to dup it so the
// new Parcel now owns its own fd, and can declare that we
// officially know we have fds.
- flat->handle = dup(flat->handle);
+ flat->handle = fcntl(flat->handle, F_DUPFD_CLOEXEC, 0);
flat->cookie = 1;
mHasFds = mFdsKnown = true;
if (!mAllowFds) {
@@ -1142,7 +1142,7 @@
status_t Parcel::writeDupFileDescriptor(int fd)
{
- int dupFd = dup(fd);
+ int dupFd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
if (dupFd < 0) {
return -errno;
}
@@ -1972,7 +1972,7 @@
}
for (int i=0 ; err==NO_ERROR && i<numFds ; i++) {
- h->data[i] = dup(readFileDescriptor());
+ h->data[i] = fcntl(readFileDescriptor(), F_DUPFD_CLOEXEC, 0);
if (h->data[i] < 0) {
for (int j = 0; j < i; j++) {
close(h->data[j]);
@@ -2020,7 +2020,7 @@
return BAD_TYPE;
}
- val->reset(dup(got));
+ val->reset(fcntl(got, F_DUPFD_CLOEXEC, 0));
if (val->get() < 0) {
return BAD_VALUE;
@@ -2095,9 +2095,9 @@
status_t err = NO_ERROR;
for (size_t i=0 ; i<fd_count && err==NO_ERROR ; i++) {
int fd = this->readFileDescriptor();
- if (fd < 0 || ((fds[i] = dup(fd)) < 0)) {
+ if (fd < 0 || ((fds[i] = fcntl(fd, F_DUPFD_CLOEXEC, 0)) < 0)) {
err = BAD_VALUE;
- ALOGE("dup() failed in Parcel::read, i is %zu, fds[i] is %d, fd_count is %zu, error: %s",
+ ALOGE("fcntl(F_DUPFD_CLOEXEC) failed in Parcel::read, i is %zu, fds[i] is %d, fd_count is %zu, error: %s",
i, fds[i], fd_count, strerror(fd < 0 ? -fd : errno));
// Close all the file descriptors that were dup-ed.
for (size_t j=0; j<i ;j++) {