Fix binder memory handling for 64 bit devices

Change MemoryHeap offset to use off_t.
Always transmit Memory related size and offset as 64 bits.

Test: CTS, native binder tests, sanity
Bug: 117556990
Change-Id: Icaabf7442f561a53941f9ebebe4029ddc533b0c2

4 files changed, 51 insertions, 36 deletions

diff --git a/libs/binder/IMemory.cpp b/libs/binder/IMemory.cpp
index 507ce53b66..0b89879ec1 100644
--- a/libs/binder/IMemory.cpp
+++ b/libs/binder/IMemory.cpp
@@ -86,7 +86,7 @@ public:
     virtual void* getBase() const;
     virtual size_t getSize() const;
     virtual uint32_t getFlags() const;
-    virtual uint32_t getOffset() const;
+    off_t getOffset() const override;
 
 private:
     friend class IMemory;
@@ -113,7 +113,7 @@ private:
     mutable void*       mBase;
     mutable size_t      mSize;
     mutable uint32_t    mFlags;
-    mutable uint32_t    mOffset;
+    mutable off_t       mOffset;
     mutable bool        mRealHeap;
     mutable Mutex       mLock;
 };
@@ -187,13 +187,16 @@ sp<IMemoryHeap> BpMemory::getMemory(ssize_t* offset, size_t* size) const
         data.writeInterfaceToken(IMemory::getInterfaceDescriptor());
         if (remote()->transact(GET_MEMORY, data, &reply) == NO_ERROR) {
             sp<IBinder> heap = reply.readStrongBinder();
-            ssize_t o = reply.readInt32();
-            size_t s = reply.readInt32();
             if (heap != nullptr) {
                 mHeap = interface_cast<IMemoryHeap>(heap);
                 if (mHeap != nullptr) {
+                    const int64_t offset64 = reply.readInt64();
+                    const uint64_t size64 = reply.readUint64();
+                    const ssize_t o = (ssize_t)offset64;
+                    const size_t s = (size_t)size64;
                     size_t heapSize = mHeap->getSize();
-                    if (s <= heapSize
+                    if (s == size64 && o == offset64 // ILP32 bounds check
+                            && s <= heapSize
                             && o >= 0
                             && (static_cast<size_t>(o) <= heapSize - s)) {
                         mOffset = o;
@@ -233,8 +236,8 @@ status_t BnMemory::onTransact(
             ssize_t offset;
             size_t size;
             reply->writeStrongBinder( IInterface::asBinder(getMemory(&offset, &size)) );
-            reply->writeInt32(offset);
-            reply->writeInt32(size);
+            reply->writeInt64(offset);
+            reply->writeUint64(size);
             return NO_ERROR;
         } break;
         default:
@@ -313,18 +316,23 @@ void BpMemoryHeap::assertReallyMapped() const
         data.writeInterfaceToken(IMemoryHeap::getInterfaceDescriptor());
         status_t err = remote()->transact(HEAP_ID, data, &reply);
         int parcel_fd = reply.readFileDescriptor();
-        ssize_t size = reply.readInt32();
-        uint32_t flags = reply.readInt32();
-        uint32_t offset = reply.readInt32();
-
-        ALOGE_IF(err, "binder=%p transaction failed fd=%d, size=%zd, err=%d (%s)",
-                IInterface::asBinder(this).get(),
-                parcel_fd, size, err, strerror(-err));
+        const uint64_t size64 = reply.readUint64();
+        const int64_t offset64 = reply.readInt64();
+        const uint32_t flags = reply.readUint32();
+        const size_t size = (size_t)size64;
+        const off_t offset = (off_t)offset64;
+        if (err != NO_ERROR || // failed transaction
+                size != size64 || offset != offset64) { // ILP32 size check
+            ALOGE("binder=%p transaction failed fd=%d, size=%zu, err=%d (%s)",
+                    IInterface::asBinder(this).get(),
+                    parcel_fd, size, err, strerror(-err));
+            return;
+        }
 
         Mutex::Autolock _l(mLock);
         if (mHeapId.load(memory_order_relaxed) == -1) {
             int fd = fcntl(parcel_fd, F_DUPFD_CLOEXEC, 0);
-            ALOGE_IF(fd==-1, "cannot dup fd=%d, size=%zd, err=%d (%s)",
+            ALOGE_IF(fd == -1, "cannot dup fd=%d, size=%zu, err=%d (%s)",
                     parcel_fd, size, err, strerror(errno));
 
             int access = PROT_READ;
@@ -334,7 +342,7 @@ void BpMemoryHeap::assertReallyMapped() const
             mRealHeap = true;
             mBase = mmap(nullptr, size, access, MAP_SHARED, fd, offset);
             if (mBase == MAP_FAILED) {
-                ALOGE("cannot map BpMemoryHeap (binder=%p), size=%zd, fd=%d (%s)",
+                ALOGE("cannot map BpMemoryHeap (binder=%p), size=%zu, fd=%d (%s)",
                         IInterface::asBinder(this).get(), size, fd, strerror(errno));
                 close(fd);
             } else {
@@ -368,7 +376,7 @@ uint32_t BpMemoryHeap::getFlags() const {
     return mFlags;
 }
 
-uint32_t BpMemoryHeap::getOffset() const {
+off_t BpMemoryHeap::getOffset() const {
     assertMapped();
     return mOffset;
 }
@@ -390,9 +398,9 @@ status_t BnMemoryHeap::onTransact(
        case HEAP_ID: {
             CHECK_INTERFACE(IMemoryHeap, data, reply);
             reply->writeFileDescriptor(getHeapID());
-            reply->writeInt32(getSize());
-            reply->writeInt32(getFlags());
-            reply->writeInt32(getOffset());
+            reply->writeUint64(getSize());
+            reply->writeInt64(getOffset());
+            reply->writeUint32(getFlags());
             return NO_ERROR;
         } break;
         default:
diff --git a/libs/binder/MemoryHeapBase.cpp b/libs/binder/MemoryHeapBase.cpp
index 9850ad9624..4c300b47c6 100644
--- a/libs/binder/MemoryHeapBase.cpp
+++ b/libs/binder/MemoryHeapBase.cpp
@@ -76,7 +76,7 @@ MemoryHeapBase::MemoryHeapBase(const char* device, size_t size, uint32_t flags)
     }
 }
 
-MemoryHeapBase::MemoryHeapBase(int fd, size_t size, uint32_t flags, uint32_t offset)
+MemoryHeapBase::MemoryHeapBase(int fd, size_t size, uint32_t flags, off_t offset)
     : mFD(-1), mSize(0), mBase(MAP_FAILED), mFlags(flags),
       mDevice(nullptr), mNeedUnmap(false), mOffset(0)
 {
@@ -85,7 +85,7 @@ MemoryHeapBase::MemoryHeapBase(int fd, size_t size, uint32_t flags, uint32_t off
     mapfd(fcntl(fd, F_DUPFD_CLOEXEC, 0), size, offset);
 }
 
-status_t MemoryHeapBase::init(int fd, void *base, int size, int flags, const char* device)
+status_t MemoryHeapBase::init(int fd, void *base, size_t size, int flags, const char* device)
 {
     if (mFD != -1) {
         return INVALID_OPERATION;
@@ -98,13 +98,20 @@ status_t MemoryHeapBase::init(int fd, void *base, int size, int flags, const cha
     return NO_ERROR;
 }
 
-status_t MemoryHeapBase::mapfd(int fd, size_t size, uint32_t offset)
+status_t MemoryHeapBase::mapfd(int fd, size_t size, off_t offset)
 {
     if (size == 0) {
         // try to figure out the size automatically
         struct stat sb;
-        if (fstat(fd, &sb) == 0)
-            size = sb.st_size;
+        if (fstat(fd, &sb) == 0) {
+            size = (size_t)sb.st_size;
+            // sb.st_size is off_t which on ILP32 may be 64 bits while size_t is 32 bits.
+            if ((off_t)size != sb.st_size) {
+                ALOGE("%s: size of file %lld cannot fit in memory",
+                        __func__, (long long)sb.st_size);
+                return INVALID_OPERATION;
+            }
+        }
         // if it didn't work, let mmap() fail.
     }
 
@@ -112,12 +119,12 @@ status_t MemoryHeapBase::mapfd(int fd, size_t size, uint32_t offset)
         void* base = (uint8_t*)mmap(nullptr, size,
                 PROT_READ|PROT_WRITE, MAP_SHARED, fd, offset);
         if (base == MAP_FAILED) {
-            ALOGE("mmap(fd=%d, size=%u) failed (%s)",
-                    fd, uint32_t(size), strerror(errno));
+            ALOGE("mmap(fd=%d, size=%zu) failed (%s)",
+                    fd, size, strerror(errno));
             close(fd);
             return -errno;
         }
-        //ALOGD("mmap(fd=%d, base=%p, size=%lu)", fd, base, size);
+        //ALOGD("mmap(fd=%d, base=%p, size=%zu)", fd, base, size);
         mBase = base;
         mNeedUnmap = true;
     } else  {
@@ -140,7 +147,7 @@ void MemoryHeapBase::dispose()
     int fd = android_atomic_or(-1, &mFD);
     if (fd >= 0) {
         if (mNeedUnmap) {
-            //ALOGD("munmap(fd=%d, base=%p, size=%lu)", fd, mBase, mSize);
+            //ALOGD("munmap(fd=%d, base=%p, size=%zu)", fd, mBase, mSize);
             munmap(mBase, mSize);
         }
         mBase = nullptr;
@@ -169,7 +176,7 @@ const char* MemoryHeapBase::getDevice() const {
     return mDevice;
 }
 
-uint32_t MemoryHeapBase::getOffset() const {
+off_t MemoryHeapBase::getOffset() const {
     return mOffset;
 }
 
diff --git a/libs/binder/include/binder/IMemory.h b/libs/binder/include/binder/IMemory.h
index 3099bf5fb8..11f7efa97d 100644
--- a/libs/binder/include/binder/IMemory.h
+++ b/libs/binder/include/binder/IMemory.h
@@ -43,7 +43,7 @@ public:
     virtual void*       getBase() const = 0;
     virtual size_t      getSize() const = 0;
     virtual uint32_t    getFlags() const = 0;
-    virtual uint32_t    getOffset() const = 0;
+    virtual off_t       getOffset() const = 0;
 
     // these are there just for backward source compatibility
     int32_t heapID() const { return getHeapID(); }
diff --git a/libs/binder/include/binder/MemoryHeapBase.h b/libs/binder/include/binder/MemoryHeapBase.h
index 5e0a382a7e..2f5039dcc9 100644
--- a/libs/binder/include/binder/MemoryHeapBase.h
+++ b/libs/binder/include/binder/MemoryHeapBase.h
@@ -42,7 +42,7 @@ public:
      * maps the memory referenced by fd. but DOESN'T take ownership
      * of the filedescriptor (it makes a copy with dup()
      */
-    MemoryHeapBase(int fd, size_t size, uint32_t flags = 0, uint32_t offset = 0);
+    MemoryHeapBase(int fd, size_t size, uint32_t flags = 0, off_t offset = 0);
 
     /*
      * maps memory from the given device
@@ -64,7 +64,7 @@ public:
 
     virtual size_t      getSize() const;
     virtual uint32_t    getFlags() const;
-    virtual uint32_t    getOffset() const;
+            off_t       getOffset() const override;
 
     const char*         getDevice() const;
 
@@ -82,11 +82,11 @@ public:
 protected:
             MemoryHeapBase();
     // init() takes ownership of fd
-    status_t init(int fd, void *base, int size,
+    status_t init(int fd, void *base, size_t size,
             int flags = 0, const char* device = nullptr);
 
 private:
-    status_t mapfd(int fd, size_t size, uint32_t offset = 0);
+    status_t mapfd(int fd, size_t size, off_t offset = 0);
 
     int         mFD;
     size_t      mSize;
@@ -94,7 +94,7 @@ private:
     uint32_t    mFlags;
     const char* mDevice;
     bool        mNeedUnmap;
-    uint32_t    mOffset;
+    off_t       mOffset;
 };
 
 // ---------------------------------------------------------------------------