From 7b397ae09a9d11abdbb6c3d2f1ce52cd58db3aca Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 4 Jul 2013 10:54:43 +0100 Subject: [PATCH 1/7] staging: android: binder: modify struct binder_write_read to use size_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change mirrors the userspace operation where struct binder_write_read members that specify the buffer size and consumed size are size_t elements. The patch also fixes the binder_thread_write() and binder_thread_read() functions prototypes to conform with the definition of binder_write_read. The changes do not affect existing 32bit ABI. Change-Id: I987246d507b9c5e4627c62a1da971d11869ac5a0 Signed-off-by: Serban Constantinescu Acked-by: Arve Hjønnevåg Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/binder.c | 10 +++++----- drivers/staging/android/uapi/binder.h | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c index f8d5b03f5444..59e714cd8e1b 100644 --- a/drivers/staging/android/binder.c +++ b/drivers/staging/android/binder.c @@ -1719,7 +1719,7 @@ err_no_context_mgr_node: } int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread, - void __user *buffer, int size, signed long *consumed) + void __user *buffer, size_t size, size_t *consumed) { uint32_t cmd; void __user *ptr = buffer + *consumed; @@ -2099,8 +2099,8 @@ static int binder_has_thread_work(struct binder_thread *thread) static int binder_thread_read(struct binder_proc *proc, struct binder_thread *thread, - void __user *buffer, int size, - signed long *consumed, int non_block) + void __user *buffer, size_t size, + size_t *consumed, int non_block) { void __user *ptr = buffer + *consumed; void __user *end = buffer + size; @@ -2597,7 +2597,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) goto err; } binder_debug(BINDER_DEBUG_READ_WRITE, - "%d:%d write %ld at %08lx, read %ld at %08lx\n", + "%d:%d write %zd at %08lx, read %zd at %08lx\n", proc->pid, thread->pid, bwr.write_size, bwr.write_buffer, bwr.read_size, bwr.read_buffer); @@ -2623,7 +2623,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) } } binder_debug(BINDER_DEBUG_READ_WRITE, - "%d:%d wrote %ld of %ld, read return %ld of %ld\n", + "%d:%d wrote %zd of %zd, read return %zd of %zd\n", proc->pid, thread->pid, bwr.write_consumed, bwr.write_size, bwr.read_consumed, bwr.read_size); if (copy_to_user(ubuf, &bwr, sizeof(bwr))) { diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h index b6cb483592ca..9e19f64e25aa 100644 --- a/drivers/staging/android/uapi/binder.h +++ b/drivers/staging/android/uapi/binder.h @@ -67,11 +67,11 @@ struct flat_binder_object { */ struct binder_write_read { - signed long write_size; /* bytes to write */ - signed long write_consumed; /* bytes consumed by driver */ + size_t write_size; /* bytes to write */ + size_t write_consumed; /* bytes consumed by driver */ unsigned long write_buffer; - signed long read_size; /* bytes to read */ - signed long read_consumed; /* bytes consumed by driver */ + size_t read_size; /* bytes to read */ + size_t read_consumed; /* bytes consumed by driver */ unsigned long read_buffer; }; From b91bf7c0044e13ff0e28bf96c2a8265d3438ba11 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 4 Jul 2013 10:54:44 +0100 Subject: [PATCH 2/7] staging: android: binder: fix BINDER_SET_MAX_THREADS declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change will fix the BINDER_SET_MAX_THREADS ioctl to use __u32 instead of size_t for setting the max threads. Thus using the same handler for 32 and 64bit kernels. This value is stored internally in struct binder_proc and set to 15 on open_binder() in the libbinder API(thus no need for a 64bit size_t on 64bit platforms). The change does not affect existing 32bit ABI. Change-Id: Ibdfe10a70d475a91c247dc36e9cfd74a259d50e4 Signed-off-by: Serban Constantinescu Acked-by: Arve Hjønnevåg Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/uapi/binder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h index 9e19f64e25aa..db9abe37ac94 100644 --- a/drivers/staging/android/uapi/binder.h +++ b/drivers/staging/android/uapi/binder.h @@ -86,7 +86,7 @@ struct binder_version { #define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read) #define BINDER_SET_IDLE_TIMEOUT _IOW('b', 3, __s64) -#define BINDER_SET_MAX_THREADS _IOW('b', 5, size_t) +#define BINDER_SET_MAX_THREADS _IOW('b', 5, __u32) #define BINDER_SET_IDLE_PRIORITY _IOW('b', 6, __s32) #define BINDER_SET_CONTEXT_MGR _IOW('b', 7, __s32) #define BINDER_THREAD_EXIT _IOW('b', 8, __s32) From 6fcd5e8cd07c1abc3ed37bd4cc5d87451b3ae191 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 4 Jul 2013 10:54:45 +0100 Subject: [PATCH 3/7] staging: android: binder: fix BC_FREE_BUFFER ioctl declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BinderDriverCommands mirror the ioctl usage. Thus the size of the structure passed through the interface should be used to generate the ioctl No. The change reflects the type being passed from the user space-a pointer to a binder_buffer. This change should not affect the existing 32bit user space since BC_FREE_BUFFER is computed as: #define _IOW(type,nr,size) \ ((type) << _IOC_TYPESHIFT) | \ ((nr) << _IOC_NRSHIFT) | \ ((size) << _IOC_SIZESHIFT)) and for a 32bit compiler BC_FREE_BUFFER will have the same computed value. This change will also ease our work in differentiating BC_FREE_BUFFER from COMPAT_BC_FREE_BUFFER. The change does not affect existing 32bit ABI. Change-Id: I2e0ae87bc4e913225a8eb2912913f7e3617cb575 Signed-off-by: Serban Constantinescu Acked-by: Arve Hjønnevåg Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/uapi/binder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h index db9abe37ac94..3de85afb5b69 100644 --- a/drivers/staging/android/uapi/binder.h +++ b/drivers/staging/android/uapi/binder.h @@ -265,7 +265,7 @@ enum binder_driver_command_protocol { * Else you have acquired a primary reference on the object. */ - BC_FREE_BUFFER = _IOW('c', 3, int), + BC_FREE_BUFFER = _IOW('c', 3, void *), /* * void *: ptr to transaction data received on a read */ From 6780a236302cc482c3261d03aec4f54437533500 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 4 Jul 2013 10:54:46 +0100 Subject: [PATCH 4/7] staging: android: binder: fix alignment issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Android userspace aligns the data written to the binder buffers to 4bytes. Thus for 32bit platforms or 64bit platforms running an 32bit Android userspace we can have a buffer looking like this: platform buffer(binder_cmd pointer) size 32/32 32b 32b 8B 64/32 32b 64b 12B 64/64 32b 64b 12B Thus the kernel needs to check that the buffer size is aligned to 4bytes not to (void *) that will be 8bytes on 64bit machines. The change does not affect existing 32bit ABI. Change-Id: I7535f07301519623ea6334f525d312d687407ed4 Signed-off-by: Serban Constantinescu Acked-by: Arve Hjønnevåg Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/binder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c index 59e714cd8e1b..6e8b0e742405 100644 --- a/drivers/staging/android/binder.c +++ b/drivers/staging/android/binder.c @@ -1249,7 +1249,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, struct flat_binder_object *fp; if (*offp > buffer->data_size - sizeof(*fp) || buffer->data_size < sizeof(*fp) || - !IS_ALIGNED(*offp, sizeof(void *))) { + !IS_ALIGNED(*offp, sizeof(u32))) { pr_err("transaction release %d bad offset %zd, size %zd\n", debug_id, *offp, buffer->data_size); continue; @@ -1502,7 +1502,7 @@ static void binder_transaction(struct binder_proc *proc, struct flat_binder_object *fp; if (*offp > t->buffer->data_size - sizeof(*fp) || t->buffer->data_size < sizeof(*fp) || - !IS_ALIGNED(*offp, sizeof(void *))) { + !IS_ALIGNED(*offp, sizeof(u32))) { binder_user_error("%d:%d got transaction with invalid offset, %zd\n", proc->pid, thread->pid, *offp); return_error = BR_FAILED_REPLY; From 7e3039eda270f9820a3840f74057dd856bdeff1b Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 4 Jul 2013 10:54:47 +0100 Subject: [PATCH 5/7] staging: android: binder: replace types with portable ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since this driver is meant to be used on different types of processors and a portable driver should specify the size a variable expects to be this patch changes the types used throughout the binder interface. We use "userspace" types since this header will be exported and used by the Android filesystem. The patch does not change in any way the functionality of the binder driver. Change-Id: Ib26daab8bc44b92d4a09badc8ecb64d37ee8773b Signed-off-by: Serban Constantinescu Acked-by: Arve Hjønnevåg Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/uapi/binder.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h index 3de85afb5b69..e1f547bb485f 100644 --- a/drivers/staging/android/uapi/binder.h +++ b/drivers/staging/android/uapi/binder.h @@ -123,10 +123,10 @@ struct binder_transaction_data { void *ptr; /* target descriptor of return transaction */ } target; void *cookie; /* target object cookie */ - unsigned int code; /* transaction command */ + __u32 code; /* transaction command */ /* General information about the transaction. */ - unsigned int flags; + __u32 flags; pid_t sender_pid; uid_t sender_euid; size_t data_size; /* number of bytes of data */ @@ -143,7 +143,7 @@ struct binder_transaction_data { /* offsets from buffer to flat_binder_object structs */ const void __user *offsets; } ptr; - uint8_t buf[8]; + __u8 buf[8]; } data; }; @@ -153,18 +153,18 @@ struct binder_ptr_cookie { }; struct binder_pri_desc { - int priority; - int desc; + __s32 priority; + __s32 desc; }; struct binder_pri_ptr_cookie { - int priority; + __s32 priority; void *ptr; void *cookie; }; enum binder_driver_return_protocol { - BR_ERROR = _IOR('r', 0, int), + BR_ERROR = _IOR('r', 0, __s32), /* * int: error code */ @@ -178,7 +178,7 @@ enum binder_driver_return_protocol { * binder_transaction_data: the received command. */ - BR_ACQUIRE_RESULT = _IOR('r', 4, int), + BR_ACQUIRE_RESULT = _IOR('r', 4, __s32), /* * not currently supported * int: 0 if the last bcATTEMPT_ACQUIRE was not successful. @@ -258,7 +258,7 @@ enum binder_driver_command_protocol { * binder_transaction_data: the sent command. */ - BC_ACQUIRE_RESULT = _IOW('c', 2, int), + BC_ACQUIRE_RESULT = _IOW('c', 2, __s32), /* * not currently supported * int: 0 if the last BR_ATTEMPT_ACQUIRE was not successful. @@ -270,10 +270,10 @@ enum binder_driver_command_protocol { * void *: ptr to transaction data received on a read */ - BC_INCREFS = _IOW('c', 4, int), - BC_ACQUIRE = _IOW('c', 5, int), - BC_RELEASE = _IOW('c', 6, int), - BC_DECREFS = _IOW('c', 7, int), + BC_INCREFS = _IOW('c', 4, __u32), + BC_ACQUIRE = _IOW('c', 5, __u32), + BC_RELEASE = _IOW('c', 6, __u32), + BC_DECREFS = _IOW('c', 7, __u32), /* * int: descriptor */ From c44aa763f58444937c8320171e08ceffde811ba7 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 4 Jul 2013 10:54:48 +0100 Subject: [PATCH 6/7] staging: android: binder: fix binder interface for 64bit compat layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changes in this patch will fix the binder interface for use on 64bit machines and stand as the base of the 64bit compat support. The changes apply to the structures that are passed between the kernel and userspace. Most of the changes applied mirror the change to struct binder_version where there is no need for a 64bit wide protocol_version(on 64bit machines). The change inlines with the existing 32bit userspace(the structure has the same size) and simplifies the compat layer such that the same handler can service the BINDER_VERSION ioctl. Other changes make use of kernel types as well as user-exportable ones and fix format specifier issues. The changes do not affect existing 32bit ABI. Change-Id: Icccc8d47c302930cc61cddc5749b4cc74dc84117 Signed-off-by: Serban Constantinescu Acked-by: Arve Hjønnevåg Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/binder.c | 20 ++++++++++---------- drivers/staging/android/uapi/binder.h | 12 ++++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c index 6e8b0e742405..b9a7d9347784 100644 --- a/drivers/staging/android/binder.c +++ b/drivers/staging/android/binder.c @@ -1273,7 +1273,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, case BINDER_TYPE_WEAK_HANDLE: { struct binder_ref *ref = binder_get_ref(proc, fp->handle); if (ref == NULL) { - pr_err("transaction release %d bad handle %ld\n", + pr_err("transaction release %d bad handle %d\n", debug_id, fp->handle); break; } @@ -1285,13 +1285,13 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, case BINDER_TYPE_FD: binder_debug(BINDER_DEBUG_TRANSACTION, - " fd %ld\n", fp->handle); + " fd %d\n", fp->handle); if (failed_at) task_close_fd(proc, fp->handle); break; default: - pr_err("transaction release %d bad object type %lx\n", + pr_err("transaction release %d bad object type %x\n", debug_id, fp->type); break; } @@ -1557,7 +1557,7 @@ static void binder_transaction(struct binder_proc *proc, case BINDER_TYPE_WEAK_HANDLE: { struct binder_ref *ref = binder_get_ref(proc, fp->handle); if (ref == NULL) { - binder_user_error("%d:%d got transaction with invalid handle, %ld\n", + binder_user_error("%d:%d got transaction with invalid handle, %d\n", proc->pid, thread->pid, fp->handle); return_error = BR_FAILED_REPLY; @@ -1604,13 +1604,13 @@ static void binder_transaction(struct binder_proc *proc, if (reply) { if (!(in_reply_to->flags & TF_ACCEPT_FDS)) { - binder_user_error("%d:%d got reply with fd, %ld, but target does not allow fds\n", + binder_user_error("%d:%d got reply with fd, %d, but target does not allow fds\n", proc->pid, thread->pid, fp->handle); return_error = BR_FAILED_REPLY; goto err_fd_not_allowed; } } else if (!target_node->accept_fds) { - binder_user_error("%d:%d got transaction with fd, %ld, but target does not allow fds\n", + binder_user_error("%d:%d got transaction with fd, %d, but target does not allow fds\n", proc->pid, thread->pid, fp->handle); return_error = BR_FAILED_REPLY; goto err_fd_not_allowed; @@ -1618,7 +1618,7 @@ static void binder_transaction(struct binder_proc *proc, file = fget(fp->handle); if (file == NULL) { - binder_user_error("%d:%d got transaction with invalid fd, %ld\n", + binder_user_error("%d:%d got transaction with invalid fd, %d\n", proc->pid, thread->pid, fp->handle); return_error = BR_FAILED_REPLY; goto err_fget_failed; @@ -1637,13 +1637,13 @@ static void binder_transaction(struct binder_proc *proc, task_fd_install(target_proc, target_fd, file); trace_binder_transaction_fd(t, fp->handle, target_fd); binder_debug(BINDER_DEBUG_TRANSACTION, - " fd %ld -> %d\n", fp->handle, target_fd); + " fd %d -> %d\n", fp->handle, target_fd); /* TODO: fput? */ fp->handle = target_fd; } break; default: - binder_user_error("%d:%d got transaction with invalid object type, %lx\n", + binder_user_error("%d:%d got transaction with invalid object type, %x\n", proc->pid, thread->pid, fp->type); return_error = BR_FAILED_REPLY; goto err_bad_object_type; @@ -2597,7 +2597,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) goto err; } binder_debug(BINDER_DEBUG_READ_WRITE, - "%d:%d write %zd at %08lx, read %zd at %08lx\n", + "%d:%d write %zd at %016lx, read %zd at %016lx\n", proc->pid, thread->pid, bwr.write_size, bwr.write_buffer, bwr.read_size, bwr.read_buffer); diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h index e1f547bb485f..e76cfa876daa 100644 --- a/drivers/staging/android/uapi/binder.h +++ b/drivers/staging/android/uapi/binder.h @@ -48,13 +48,13 @@ enum { */ struct flat_binder_object { /* 8 bytes for large_flat_header. */ - unsigned long type; - unsigned long flags; + __u32 type; + __u32 flags; /* 8 bytes of data. */ union { void __user *binder; /* local object */ - signed long handle; /* remote object */ + __u32 handle; /* remote object */ }; /* extra data associated with local object */ @@ -78,7 +78,7 @@ struct binder_write_read { /* Use with BINDER_VERSION, driver fills in fields. */ struct binder_version { /* driver protocol version -- increment with incompatible change */ - signed long protocol_version; + __s32 protocol_version; }; /* This is the current protocol version. */ @@ -119,7 +119,7 @@ struct binder_transaction_data { * identifying the target and contents of the transaction. */ union { - size_t handle; /* target descriptor of command transaction */ + __u32 handle; /* target descriptor of command transaction */ void *ptr; /* target descriptor of return transaction */ } target; void *cookie; /* target object cookie */ @@ -154,7 +154,7 @@ struct binder_ptr_cookie { struct binder_pri_desc { __s32 priority; - __s32 desc; + __u32 desc; }; struct binder_pri_ptr_cookie { From e6395fe2411c2dda02c6052c7bbca32b56f10026 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Wed, 15 Jan 2014 11:28:36 +0000 Subject: [PATCH 7/7] staging: android: binder: fix ABI for 64bit Android This patch fixes the ABI for 64bit Android userspace. BC_REQUEST_DEATH_NOTIFICATION and BC_CLEAR_DEATH_NOTIFICATION claim to be using struct binder_ptr_cookie, but they are using a 32bit handle and a pointer. On 32bit systems the payload size is the same as the size of struct binder_ptr_cookie, however for 64bit systems this will differ. This patch adds struct binder_handle_cookie that fixes this issue for 64bit Android. Since there are no 64bit users of this interface that we know of this change should not affect any existing systems. Change-Id: I8909cbc50aad48ccf371270bad6f69ff242a8c22 Signed-off-by: Serban Constantinescu --- drivers/staging/android/uapi/binder.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h index e76cfa876daa..008722a249bc 100644 --- a/drivers/staging/android/uapi/binder.h +++ b/drivers/staging/android/uapi/binder.h @@ -152,6 +152,11 @@ struct binder_ptr_cookie { void *cookie; }; +struct binder_handle_cookie { + __u32 handle; + void *cookie; +} __attribute__((packed)); + struct binder_pri_desc { __s32 priority; __u32 desc; @@ -308,15 +313,15 @@ enum binder_driver_command_protocol { * of looping threads it has available. */ - BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct binder_ptr_cookie), + BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct binder_handle_cookie), /* - * void *: ptr to binder + * int: handle * void *: cookie */ - BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15, struct binder_ptr_cookie), + BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15, struct binder_handle_cookie), /* - * void *: ptr to binder + * int: handle * void *: cookie */