gralloc1: Import ion handle into registering process
Importing the ion handle when retain() is called validates the
incoming ion fd and holds the client process accountable for
releasing the handle once it's done.
CRs-Fixed: 2020175
Change-Id: I3a169983b6d6b201d044e1c777a631aa16f9cb9a
diff --git a/libgralloc1/gr_allocator.cpp b/libgralloc1/gr_allocator.cpp
index aba6dc5..c044827 100644
--- a/libgralloc1/gr_allocator.cpp
+++ b/libgralloc1/gr_allocator.cpp
@@ -129,6 +129,13 @@
return -EINVAL;
}
+int Allocator::ImportBuffer(int fd) {
+ if (ion_allocator_) {
+ return ion_allocator_->ImportBuffer(fd);
+ }
+ return -EINVAL;
+}
+
int Allocator::FreeBuffer(void *base, unsigned int size, unsigned int offset, int fd,
int handle) {
if (ion_allocator_) {
@@ -138,9 +145,9 @@
return -EINVAL;
}
-int Allocator::CleanBuffer(void *base, unsigned int size, unsigned int offset, int fd, int op) {
+int Allocator::CleanBuffer(void *base, unsigned int size, unsigned int offset, int handle, int op) {
if (ion_allocator_) {
- return ion_allocator_->CleanBuffer(base, size, offset, fd, op);
+ return ion_allocator_->CleanBuffer(base, size, offset, handle, op);
}
return -EINVAL;
diff --git a/libgralloc1/gr_allocator.h b/libgralloc1/gr_allocator.h
index e73bd8c..df1a30c 100644
--- a/libgralloc1/gr_allocator.h
+++ b/libgralloc1/gr_allocator.h
@@ -51,8 +51,9 @@
~Allocator();
bool Init();
int MapBuffer(void **base, unsigned int size, unsigned int offset, int fd);
+ int ImportBuffer(int fd);
int FreeBuffer(void *base, unsigned int size, unsigned int offset, int fd, int handle);
- int CleanBuffer(void *base, unsigned int size, unsigned int offset, int fd, int op);
+ int CleanBuffer(void *base, unsigned int size, unsigned int offset, int handle, int op);
int AllocateMem(AllocData *data, gralloc1_producer_usage_t prod_usage,
gralloc1_consumer_usage_t cons_usage);
// @return : index of the descriptor with maximum buffer size req
diff --git a/libgralloc1/gr_buf_mgr.cpp b/libgralloc1/gr_buf_mgr.cpp
index e2cfdd4..04afd35 100644
--- a/libgralloc1/gr_buf_mgr.cpp
+++ b/libgralloc1/gr_buf_mgr.cpp
@@ -155,6 +155,7 @@
void BufferManager::CreateSharedHandle(buffer_handle_t inbuffer, const BufferDescriptor &descriptor,
buffer_handle_t *outbuffer) {
+ // TODO(user): This path is not verified
private_handle_t const *input = reinterpret_cast<private_handle_t const *>(inbuffer);
// Get Buffer attributes or dimension
@@ -182,8 +183,7 @@
descriptor.GetConsumerUsage());
out_hnd->id = ++next_id_;
// TODO(user): Base address of shared handle and ion handles
- auto buffer = std::make_shared<Buffer>(out_hnd);
- handles_map_.emplace(std::make_pair(out_hnd, buffer));
+ RegisterHandle(out_hnd, -1, -1);
*outbuffer = out_hnd;
}
@@ -205,6 +205,42 @@
return GRALLOC1_ERROR_NONE;
}
+void BufferManager::RegisterHandle(const private_handle_t *hnd,
+ int ion_handle,
+ int ion_handle_meta) {
+ auto buffer = std::make_shared<Buffer>(hnd, ion_handle, ion_handle_meta);
+ handles_map_.emplace(std::make_pair(hnd, buffer));
+}
+
+gralloc1_error_t BufferManager::ImportHandle(private_handle_t* hnd) {
+ int ion_handle = allocator_->ImportBuffer(hnd->fd);
+ if (ion_handle < 0) {
+ ALOGE("Failed to import ion buffer: hnd: %p, fd:%d, id:%" PRIu64, hnd, hnd->fd, hnd->id);
+ return GRALLOC1_ERROR_BAD_HANDLE;
+ }
+ int ion_handle_meta = allocator_->ImportBuffer(hnd->fd_metadata);
+ if (ion_handle_meta < 0) {
+ ALOGE("Failed to import ion metadata buffer: hnd: %p, fd:%d, id:%" PRIu64, hnd,
+ hnd->fd, hnd->id);
+ return GRALLOC1_ERROR_BAD_HANDLE;
+ }
+ // Set base pointers to NULL since the data here was received over binder
+ hnd->base = 0;
+ hnd->base_metadata = 0;
+ RegisterHandle(hnd, ion_handle, ion_handle_meta);
+ return GRALLOC1_ERROR_NONE;
+}
+
+std::shared_ptr<BufferManager::Buffer>
+BufferManager::GetBufferFromHandle(const private_handle_t *hnd) {
+ auto it = handles_map_.find(hnd);
+ if (it != handles_map_.end()) {
+ return it->second;
+ } else {
+ return nullptr;
+ }
+}
+
gralloc1_error_t BufferManager::MapBuffer(private_handle_t const *handle) {
private_handle_t *hnd = const_cast<private_handle_t *>(handle);
@@ -215,7 +251,7 @@
return GRALLOC1_ERROR_BAD_HANDLE;
}
- unsigned int size = ALIGN((unsigned int)sizeof(MetaData_t), PAGE_SIZE);
+ unsigned int size = ALIGN((unsigned int)sizeof(MetaData_t), getpagesize());
if (allocator_->MapBuffer(reinterpret_cast<void **>(&hnd->base_metadata), size,
hnd->offset_metadata, hnd->fd_metadata) != 0) {
return GRALLOC1_ERROR_BAD_HANDLE;
@@ -227,39 +263,32 @@
gralloc1_error_t BufferManager::RetainBuffer(private_handle_t const *hnd) {
std::lock_guard<std::mutex> lock(locker_);
ALOGD_IF(DEBUG, "Retain buffer handle:%p id: %" PRIu64, hnd, hnd->id);
-
- // find if this handle is already in map
- auto it = handles_map_.find(hnd);
- if (it != handles_map_.end()) {
- // It's already in map, Just increment refcnt
- // No need to mmap the memory.
- auto buf = it->second;
- buf->ref_count++;
+ gralloc1_error_t err = GRALLOC1_ERROR_NONE;
+ auto buf = GetBufferFromHandle(hnd);
+ if (buf != nullptr) {
+ buf->IncRef();
} else {
- // not present in the map. mmap and then add entry to map
- if (MapBuffer(hnd) == GRALLOC1_ERROR_NONE) {
- auto buffer = std::make_shared<Buffer>(hnd);
- handles_map_.emplace(std::make_pair(hnd, buffer));
+ private_handle_t *handle = const_cast<private_handle_t *>(hnd);
+ err = ImportHandle(handle);
+ if (err == GRALLOC1_ERROR_NONE) {
+ // TODO(user): Do not map here, map should be in lock()
+ err = MapBuffer(hnd);
}
}
-
- return GRALLOC1_ERROR_NONE;
+ return err;
}
gralloc1_error_t BufferManager::ReleaseBuffer(private_handle_t const *hnd) {
std::lock_guard<std::mutex> lock(locker_);
ALOGD_IF(DEBUG, "Release buffer handle:%p id: %" PRIu64, hnd, hnd->id);
- // find if this handle is already in map
- auto it = handles_map_.find(hnd);
- if (it == handles_map_.end()) {
- // Corrupt handle or map.
- ALOGE("Could not find handle");
+ auto buf = GetBufferFromHandle(hnd);
+ if (buf == nullptr) {
+ ALOGE("Could not find handle: %p id: %" PRIu64, hnd, hnd->id);
return GRALLOC1_ERROR_BAD_HANDLE;
} else {
- auto buf = it->second;
- buf->ref_count--;
- if (buf->ref_count == 0) {
- handles_map_.erase(it);
+ if (buf->DecRef()) {
+ handles_map_.erase(hnd);
+ // Unmap, close ion handle and close fd
FreeBuffer(buf);
}
}
@@ -269,6 +298,7 @@
gralloc1_error_t BufferManager::LockBuffer(const private_handle_t *hnd,
gralloc1_producer_usage_t prod_usage,
gralloc1_consumer_usage_t cons_usage) {
+ std::lock_guard<std::mutex> lock(locker_);
gralloc1_error_t err = GRALLOC1_ERROR_NONE;
// If buffer is not meant for CPU return err
@@ -278,18 +308,23 @@
if (hnd->base == 0) {
// we need to map for real
- locker_.lock();
err = MapBuffer(hnd);
- locker_.unlock();
+ }
+
+ auto buf = GetBufferFromHandle(hnd);
+ if (buf == nullptr) {
+ return GRALLOC1_ERROR_BAD_HANDLE;
}
// Invalidate if CPU reads in software and there are non-CPU
// writers. No need to do this for the metadata buffer as it is
// only read/written in software.
+
+ // todo use handle here
if (!err && (hnd->flags & private_handle_t::PRIV_FLAGS_USES_ION) &&
(hnd->flags & private_handle_t::PRIV_FLAGS_CACHED)) {
if (allocator_->CleanBuffer(reinterpret_cast<void *>(hnd->base), hnd->size, hnd->offset,
- hnd->fd, CACHE_INVALIDATE)) {
+ buf->ion_handle_main, CACHE_INVALIDATE)) {
return GRALLOC1_ERROR_BAD_HANDLE;
}
}
@@ -304,20 +339,23 @@
}
gralloc1_error_t BufferManager::UnlockBuffer(const private_handle_t *handle) {
+ std::lock_guard<std::mutex> lock(locker_);
gralloc1_error_t status = GRALLOC1_ERROR_NONE;
- locker_.lock();
private_handle_t *hnd = const_cast<private_handle_t *>(handle);
+ auto buf = GetBufferFromHandle(hnd);
+ if (buf == nullptr) {
+ return GRALLOC1_ERROR_BAD_HANDLE;
+ }
if (hnd->flags & private_handle_t::PRIV_FLAGS_NEEDS_FLUSH) {
if (allocator_->CleanBuffer(reinterpret_cast<void *>(hnd->base), hnd->size, hnd->offset,
- hnd->fd, CACHE_CLEAN) != 0) {
+ buf->ion_handle_main, CACHE_CLEAN) != 0) {
status = GRALLOC1_ERROR_BAD_HANDLE;
}
hnd->flags &= ~private_handle_t::PRIV_FLAGS_NEEDS_FLUSH;
}
- locker_.unlock();
return status;
}
@@ -459,8 +497,7 @@
ColorSpace_t colorSpace = ITU_R_601;
setMetaData(hnd, UPDATE_COLOR_SPACE, reinterpret_cast<void *>(&colorSpace));
*handle = hnd;
- auto buffer = std::make_shared<Buffer>(hnd, data.ion_handle, e_data.ion_handle);
- handles_map_.emplace(std::make_pair(hnd, buffer));
+ RegisterHandle(hnd, data.ion_handle, e_data.ion_handle);
ALOGD_IF(DEBUG, "Allocated buffer handle: %p id: %" PRIu64, hnd, hnd->id);
if (DEBUG) {
private_handle_t::Dump(hnd);
diff --git a/libgralloc1/gr_buf_mgr.h b/libgralloc1/gr_buf_mgr.h
index 95b64ef..0677d96 100644
--- a/libgralloc1/gr_buf_mgr.h
+++ b/libgralloc1/gr_buf_mgr.h
@@ -86,6 +86,12 @@
void CreateSharedHandle(buffer_handle_t inbuffer, const BufferDescriptor &descriptor,
buffer_handle_t *out_buffer);
+ // Imports the ion fds into the current process. Returns an error for invalid handles
+ gralloc1_error_t ImportHandle(private_handle_t* hnd);
+
+ // Creates a Buffer from the valid private handle and adds it to the map
+ void RegisterHandle(const private_handle_t *hnd, int ion_handle, int ion_handle_meta);
+
// Wrapper structure over private handle
// Values associated with the private handle
// that do not need to go over IPC can be placed here
@@ -106,9 +112,15 @@
ion_handle_main(ih_main),
ion_handle_meta(ih_meta) {
}
+ void IncRef() { ++ref_count; }
+ bool DecRef() { return --ref_count == 0; }
};
+
gralloc1_error_t FreeBuffer(std::shared_ptr<Buffer> buf);
+ // Get the wrapper Buffer object from the handle, returns nullptr if handle is not found
+ std::shared_ptr<Buffer> GetBufferFromHandle(const private_handle_t *hnd);
+
bool map_fb_mem_ = false;
bool ubwc_for_fb_ = false;
Allocator *allocator_ = NULL;
diff --git a/libgralloc1/gr_ion_alloc.cpp b/libgralloc1/gr_ion_alloc.cpp
index b76fa9f..25792e5 100644
--- a/libgralloc1/gr_ion_alloc.cpp
+++ b/libgralloc1/gr_ion_alloc.cpp
@@ -123,11 +123,13 @@
if (base) {
err = UnmapBuffer(base, size, offset);
}
- struct ion_handle_data handle_data;
- handle_data.handle = ion_handle;
- ioctl(ion_dev_fd_, INT(ION_IOC_FREE), &handle_data);
- close(fd);
+ if (ion_handle > 0) {
+ struct ion_handle_data handle_data;
+ handle_data.handle = ion_handle;
+ ioctl(ion_dev_fd_, INT(ION_IOC_FREE), &handle_data);
+ }
+ close(fd);
return err;
}
@@ -150,6 +152,18 @@
return err;
}
+int IonAlloc::ImportBuffer(int fd) {
+ struct ion_fd_data fd_data;
+ int err = 0;
+ fd_data.fd = fd;
+ if (ioctl(ion_dev_fd_, INT(ION_IOC_IMPORT), &fd_data)) {
+ err = -errno;
+ ALOGE("%s: ION_IOC_IMPORT failed with error - %s", __FUNCTION__, strerror(errno));
+ return err;
+ }
+ return fd_data.handle;
+}
+
int IonAlloc::UnmapBuffer(void *base, unsigned int size, unsigned int /*offset*/) {
ATRACE_CALL();
ALOGD_IF(DEBUG, "ion: Unmapping buffer base:%p size:%u", base, size);
@@ -163,23 +177,13 @@
return err;
}
-int IonAlloc::CleanBuffer(void *base, unsigned int size, unsigned int offset, int fd, int op) {
+int IonAlloc::CleanBuffer(void *base, unsigned int size, unsigned int offset, int handle, int op) {
ATRACE_CALL();
ATRACE_INT("operation id", op);
struct ion_flush_data flush_data;
- struct ion_fd_data fd_data;
- struct ion_handle_data handle_data;
int err = 0;
- fd_data.fd = fd;
- if (ioctl(ion_dev_fd_, INT(ION_IOC_IMPORT), &fd_data)) {
- err = -errno;
- ALOGE("%s: ION_IOC_IMPORT failed with error - %s", __FUNCTION__, strerror(errno));
- return err;
- }
-
- handle_data.handle = fd_data.handle;
- flush_data.handle = fd_data.handle;
+ flush_data.handle = handle;
flush_data.vaddr = base;
// offset and length are unsigned int
flush_data.offset = offset;
@@ -202,12 +206,9 @@
if (ioctl(ion_dev_fd_, INT(ION_IOC_CUSTOM), &d)) {
err = -errno;
ALOGE("%s: ION_IOC_CLEAN_INV_CACHES failed with error - %s", __FUNCTION__, strerror(errno));
- ioctl(ion_dev_fd_, INT(ION_IOC_FREE), &handle_data);
return err;
}
- ioctl(ion_dev_fd_, INT(ION_IOC_FREE), &handle_data);
-
return 0;
}
diff --git a/libgralloc1/gr_ion_alloc.h b/libgralloc1/gr_ion_alloc.h
index 68f453c..b25f509 100644
--- a/libgralloc1/gr_ion_alloc.h
+++ b/libgralloc1/gr_ion_alloc.h
@@ -66,8 +66,9 @@
int AllocBuffer(AllocData *data);
int FreeBuffer(void *base, unsigned int size, unsigned int offset, int fd, int ion_handle);
int MapBuffer(void **base, unsigned int size, unsigned int offset, int fd);
+ int ImportBuffer(int fd);
int UnmapBuffer(void *base, unsigned int size, unsigned int offset);
- int CleanBuffer(void *base, unsigned int size, unsigned int offset, int fd, int op);
+ int CleanBuffer(void *base, unsigned int size, unsigned int offset, int handle, int op);
private:
const char *kIonDevice = "/dev/ion";