Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec9ec7d5 authored by Nick Kralevich's avatar Nick Kralevich
Browse files

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
parent c4c286f3

libs/binder/IActivityManager.cpp

+4 −1
Original line number Diff line number Diff line
@@ -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

libs/binder/IMemory.cpp

+2 −2
Original line number Diff line number Diff line
@@ -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));

libs/binder/IShellCallback.cpp

+4 −1
Original line number Diff line number Diff line
@@ -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;



    }

};

libs/binder/MemoryHeapBase.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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)

libs/binder/Parcel.cpp

+6 −6
Original line number Diff line number Diff line
@@ -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++) {