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

Commit dacdbd19 authored by Ytai Ben-Tsvi's avatar Ytai Ben-Tsvi
Browse files

MemoryHeapBase: Map as read-only when needed 

When creating a MemoryHeapBase around a file descriptor provided by a
different process, either via an fd or a device name, the existing
code would attempt to map it with PROT_WRITE, unconditionally, which
would result in a failure to map.

With this change, we omit PROT_WRITE from the mapping whenever the
READ_ONLY flag is set, but only when accessing via one of these ctors.
The ctor that allocates a new ashmem region continues to work as
before, with the caller process having write access, but any other
process not having it.

Test: atest -p frameworks/native/libs/binder
Change-Id: Iab3583d841c3dceed1a7cb61e922a85104b4b00b
parent 59842065

libs/binder/MemoryHeapBase.cpp

+9 −5
Original line number Original line Diff line number Diff line
@@ -49,7 +49,7 @@ MemoryHeapBase::MemoryHeapBase(size_t size, uint32_t flags, char const * name) 
    int fd = ashmem_create_region(name == nullptr ? "MemoryHeapBase" : name, size);

    int fd = ashmem_create_region(name == nullptr ? "MemoryHeapBase" : name, size);

    ALOGE_IF(fd<0, "error creating ashmem region: %s", strerror(errno));

    ALOGE_IF(fd<0, "error creating ashmem region: %s", strerror(errno));

    if (fd >= 0) {

    if (fd >= 0) {

        if (mapfd(fd, size) == NO_ERROR) {

        if (mapfd(fd, true, size) == NO_ERROR) {

            if (flags & READ_ONLY) {

            if (flags & READ_ONLY) {

                ashmem_set_prot_region(fd, PROT_READ);

                ashmem_set_prot_region(fd, PROT_READ);

            }

            }
@@ -70,7 +70,7 @@ MemoryHeapBase::MemoryHeapBase(const char* device, size_t size, uint32_t flags) 
    if (fd >= 0) {

    if (fd >= 0) {

        const size_t pagesize = getpagesize();

        const size_t pagesize = getpagesize();

        size = ((size + pagesize-1) & ~(pagesize-1));

        size = ((size + pagesize-1) & ~(pagesize-1));

        if (mapfd(fd, size) == NO_ERROR) {

        if (mapfd(fd, false, size) == NO_ERROR) {

            mDevice = device;

            mDevice = device;

        }

        }

    }

    }
@@ -82,7 +82,7 @@ MemoryHeapBase::MemoryHeapBase(int fd, size_t size, uint32_t flags, off_t offset 
{

{

    const size_t pagesize = getpagesize();

    const size_t pagesize = getpagesize();

    size = ((size + pagesize-1) & ~(pagesize-1));

    size = ((size + pagesize-1) & ~(pagesize-1));

    mapfd(fcntl(fd, F_DUPFD_CLOEXEC, 0), size, offset);

    mapfd(fcntl(fd, F_DUPFD_CLOEXEC, 0), false, size, offset);

}

}





status_t MemoryHeapBase::init(int fd, void *base, size_t size, int flags, const char* device)

status_t MemoryHeapBase::init(int fd, void *base, size_t size, int flags, const char* device)
@@ -98,7 +98,7 @@ status_t MemoryHeapBase::init(int fd, void *base, size_t size, int flags, const 
    return NO_ERROR;

    return NO_ERROR;

}

}





status_t MemoryHeapBase::mapfd(int fd, size_t size, off_t offset)

status_t MemoryHeapBase::mapfd(int fd, bool writeableByCaller, size_t size, off_t offset)

{

{

    if (size == 0) {

    if (size == 0) {

        // try to figure out the size automatically

        // try to figure out the size automatically
@@ -116,8 +116,12 @@ status_t MemoryHeapBase::mapfd(int fd, size_t size, off_t offset) 
    }

    }





    if ((mFlags & DONT_MAP_LOCALLY) == 0) {

    if ((mFlags & DONT_MAP_LOCALLY) == 0) {

        int prot = PROT_READ;

        if (writeableByCaller || (mFlags & READ_ONLY) == 0) {

            prot |= PROT_WRITE;

        }

        void* base = (uint8_t*)mmap(nullptr, size,

        void* base = (uint8_t*)mmap(nullptr, size,

                PROT_READ|PROT_WRITE, MAP_SHARED, fd, offset);

                prot, MAP_SHARED, fd, offset);

        if (base == MAP_FAILED) {

        if (base == MAP_FAILED) {

            ALOGE("mmap(fd=%d, size=%zu) failed (%s)",

            ALOGE("mmap(fd=%d, size=%zu) failed (%s)",

                    fd, size, strerror(errno));

                    fd, size, strerror(errno));

libs/binder/include/binder/MemoryHeapBase.h

+3 −1
Original line number Original line Diff line number Diff line
@@ -51,6 +51,8 @@ public: 




    /*

    /*

     * maps memory from ashmem, with the given name for debugging

     * maps memory from ashmem, with the given name for debugging

     * if the READ_ONLY flag is set, the memory will be writeable by the calling process,

     * but not by others. this is NOT the case with the other ctors.

     */

     */

    explicit MemoryHeapBase(size_t size, uint32_t flags = 0, char const* name = nullptr);

    explicit MemoryHeapBase(size_t size, uint32_t flags = 0, char const* name = nullptr);
@@ -78,7 +80,7 @@ protected: 
            int flags = 0, const char* device = nullptr);

            int flags = 0, const char* device = nullptr);





private:

private:

    status_t mapfd(int fd, size_t size, off_t offset = 0);

    status_t mapfd(int fd, bool writeableByCaller, size_t size, off_t offset = 0);





    int         mFD;

    int         mFD;

    size_t      mSize;

    size_t      mSize;