diff options
author | 2016-12-17 19:47:27 -0800 | |
---|---|---|
committer | 2016-12-17 19:55:46 -0800 | |
commit | ec9ec7d55c63f791ab3ed9221e68d6215f7b928a (patch) | |
tree | fca8c716c710c682fda737df3982a1cda995a525 | |
parent | c4c286f30a60ef9ebfc959ea4869d87ceeb831dc (diff) |
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
-rw-r--r-- | libs/binder/IActivityManager.cpp | 5 | ||||
-rw-r--r-- | libs/binder/IMemory.cpp | 4 | ||||
-rw-r--r-- | libs/binder/IShellCallback.cpp | 5 | ||||
-rw-r--r-- | libs/binder/MemoryHeapBase.cpp | 2 | ||||
-rw-r--r-- | libs/binder/Parcel.cpp | 12 |
5 files changed, 17 insertions, 11 deletions
diff --git a/libs/binder/IActivityManager.cpp b/libs/binder/IActivityManager.cpp index 898ee52533..87b295a415 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 @@ public: // 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 790fa8c931..4a39741569 100644 --- a/libs/binder/IMemory.cpp +++ b/libs/binder/IMemory.cpp @@ -288,7 +288,7 @@ void BpMemoryHeap::assertMapped() const 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 @@ void BpMemoryHeap::assertReallyMapped() const 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 8b973019ee..c793df3266 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 @@ public: 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 43a01e472c..b4b1062b66 100644 --- a/libs/binder/MemoryHeapBase.cpp +++ b/libs/binder/MemoryHeapBase.cpp @@ -83,7 +83,7 @@ MemoryHeapBase::MemoryHeapBase(int fd, size_t size, uint32_t flags, uint32_t off { 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 15c441ef0c..a6ccb53852 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -539,7 +539,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) // 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::writeFileDescriptor(int fd, bool takeOwnership) 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 @@ native_handle* Parcel::readNativeHandle() const } 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 @@ status_t Parcel::readUniqueFileDescriptor(base::unique_fd* val) const 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 Parcel::read(FlattenableHelperInterface& val) const 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++) { |