[PATCH 0/8] Android Binder IPC Fixes

[PATCH 0/8] Android Binder IPC Fixes

[Serban Constantinescu]

Hi all,

This set of patches will clean-up and fix some of the issues that arise
with the current binder interface when moving to a 64bit kernel. All these
changes will not affect the existing 32bit Android interface and are meant
to stand as the base for the 64bit binder compat layer.

This patch set has been successfully tested on 32bit platforms(ARMv7 VExpress)
and 64bit platforms(ARMv8 RTSM) running a 32bit Android userspace and an in
kernel binder compat layer.

Best Regards,
Serban Constantinescu

Serban Constantinescu (8):
  staging: android: binder: replace explicit size types
  staging: android: binder: replace IOCTL types with user-exportable
    types
  staging: android: binder: fix binder interface for 64bit compat layer
  staging: android: binder: fix printk() format specifier
  staging: android: binder: fix BINDER_SET_MAX_THREADS declaration
  staging: android: binder: fix BC_FREE_BUFFER ioctl declaration
  staging: android: binder: fix alignment issues
  staging: android: binder: replace types with portable ones

 drivers/staging/android/binder.c |  114 +++++++++++++++++++-------------------
 drivers/staging/android/binder.h |   54 +++++++++---------
 2 files changed, 84 insertions(+), 84 deletions(-)

-- 
1.7.9.5

[PATCH 1/8] staging: android: binder: replace explicit size types

[Serban Constantinescu]

Since the binder driver uses both uint32_t and unsigned int any further
kernel changes will be difficult to read. This patch fixes the inconsistent
types usage.

The patch does not change in any way the functionality of the binder driver.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.c |   74 +++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 24456a0..5794cf6 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -103,7 +103,7 @@ enum {
 	BINDER_DEBUG_PRIORITY_CAP           = 1U << 14,
 	BINDER_DEBUG_BUFFER_ALLOC_ASYNC     = 1U << 15,
 };
-static uint32_t binder_debug_mask = BINDER_DEBUG_USER_ERROR |
+static unsigned int binder_debug_mask = BINDER_DEBUG_USER_ERROR |
 	BINDER_DEBUG_FAILED_TRANSACTION | BINDER_DEBUG_DEAD_TRANSACTION;
 module_param_named(debug_mask, binder_debug_mask, uint, S_IWUSR | S_IRUGO);
 
@@ -255,7 +255,7 @@ struct binder_ref {
 	struct hlist_node node_entry;
 	struct binder_proc *proc;
 	struct binder_node *node;
-	uint32_t desc;
+	unsigned int desc;
 	int strong;
 	int weak;
 	struct binder_ref_death *death;
@@ -307,7 +307,7 @@ struct binder_proc {
 
 	struct page **pages;
 	size_t buffer_size;
-	uint32_t buffer_free;
+	unsigned int buffer_free;
 	struct list_head todo;
 	wait_queue_head_t wait;
 	struct binder_stats stats;
@@ -336,8 +336,8 @@ struct binder_thread {
 	int looper;
 	struct binder_transaction *transaction_stack;
 	struct list_head todo;
-	uint32_t return_error; /* Write failed, return error code in read buf */
-	uint32_t return_error2; /* Write failed, return error code in read */
+	unsigned int return_error; /* Write failed, return error code in read buf */
+	unsigned int return_error2; /* Write failed, return error code in read */
 		/* buffer. Used when sending a reply to a dead process that */
 		/* we are also waiting on */
 	wait_queue_head_t wait;
@@ -993,7 +993,7 @@ static int binder_dec_node(struct binder_node *node, int strong, int internal)
 
 
 static struct binder_ref *binder_get_ref(struct binder_proc *proc,
-					 uint32_t desc)
+					 unsigned int desc)
 {
 	struct rb_node *n = proc->refs_by_desc.rb_node;
 	struct binder_ref *ref;
@@ -1173,7 +1173,7 @@ static void binder_pop_transaction(struct binder_thread *target_thread,
 }
 
 static void binder_send_failed_reply(struct binder_transaction *t,
-				     uint32_t error_code)
+				     unsigned int error_code)
 {
 	struct binder_thread *target_thread;
 	BUG_ON(t->flags & TF_ONE_WAY);
@@ -1310,7 +1310,7 @@ static void binder_transaction(struct binder_proc *proc,
 	wait_queue_head_t *target_wait;
 	struct binder_transaction *in_reply_to = NULL;
 	struct binder_transaction_log_entry *e;
-	uint32_t return_error;
+	unsigned int return_error;
 
 	e = binder_transaction_log_add(&binder_transaction_log);
 	e->call_type = reply ? 2 : !!(tr->flags & TF_ONE_WAY);
@@ -1702,14 +1702,14 @@ 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)
 {
-	uint32_t cmd;
+	unsigned int cmd;
 	void __user *ptr = buffer + *consumed;
 	void __user *end = buffer + size;
 
 	while (ptr < end && thread->return_error == BR_OK) {
-		if (get_user(cmd, (uint32_t __user *)ptr))
+		if (get_user(cmd, (unsigned int __user *)ptr))
 			return -EFAULT;
-		ptr += sizeof(uint32_t);
+		ptr += sizeof(unsigned int);
 		trace_binder_command(cmd);
 		if (_IOC_NR(cmd) < ARRAY_SIZE(binder_stats.bc)) {
 			binder_stats.bc[_IOC_NR(cmd)]++;
@@ -1721,13 +1721,13 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread,
 		case BC_ACQUIRE:
 		case BC_RELEASE:
 		case BC_DECREFS: {
-			uint32_t target;
+			unsigned int target;
 			struct binder_ref *ref;
 			const char *debug_string;
 
-			if (get_user(target, (uint32_t __user *)ptr))
+			if (get_user(target, (unsigned int __user *)ptr))
 				return -EFAULT;
-			ptr += sizeof(uint32_t);
+			ptr += sizeof(unsigned int);
 			if (target == 0 && binder_context_mgr_node &&
 			    (cmd == BC_INCREFS || cmd == BC_ACQUIRE)) {
 				ref = binder_get_ref_for_node(proc,
@@ -1922,14 +1922,14 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread,
 
 		case BC_REQUEST_DEATH_NOTIFICATION:
 		case BC_CLEAR_DEATH_NOTIFICATION: {
-			uint32_t target;
+			unsigned int target;
 			void __user *cookie;
 			struct binder_ref *ref;
 			struct binder_ref_death *death;
 
-			if (get_user(target, (uint32_t __user *)ptr))
+			if (get_user(target, (unsigned int __user *)ptr))
 				return -EFAULT;
-			ptr += sizeof(uint32_t);
+			ptr += sizeof(unsigned int);
 			if (get_user(cookie, (void __user * __user *)ptr))
 				return -EFAULT;
 			ptr += sizeof(void *);
@@ -2055,7 +2055,7 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread,
 }
 
 void binder_stat_br(struct binder_proc *proc, struct binder_thread *thread,
-		    uint32_t cmd)
+		    unsigned int cmd)
 {
 	trace_binder_return(cmd);
 	if (_IOC_NR(cmd) < ARRAY_SIZE(binder_stats.br)) {
@@ -2090,9 +2090,9 @@ static int binder_thread_read(struct binder_proc *proc,
 	int wait_for_proc_work;
 
 	if (*consumed == 0) {
-		if (put_user(BR_NOOP, (uint32_t __user *)ptr))
+		if (put_user(BR_NOOP, (unsigned int __user *)ptr))
 			return -EFAULT;
-		ptr += sizeof(uint32_t);
+		ptr += sizeof(unsigned int);
 	}
 
 retry:
@@ -2101,17 +2101,17 @@ retry:
 
 	if (thread->return_error != BR_OK && ptr < end) {
 		if (thread->return_error2 != BR_OK) {
-			if (put_user(thread->return_error2, (uint32_t __user *)ptr))
+			if (put_user(thread->return_error2, (unsigned int __user *)ptr))
 				return -EFAULT;
-			ptr += sizeof(uint32_t);
+			ptr += sizeof(unsigned int);
 			binder_stat_br(proc, thread, thread->return_error2);
 			if (ptr == end)
 				goto done;
 			thread->return_error2 = BR_OK;
 		}
-		if (put_user(thread->return_error, (uint32_t __user *)ptr))
+		if (put_user(thread->return_error, (unsigned int __user *)ptr))
 			return -EFAULT;
-		ptr += sizeof(uint32_t);
+		ptr += sizeof(unsigned int);
 		binder_stat_br(proc, thread, thread->return_error);
 		thread->return_error = BR_OK;
 		goto done;
@@ -2159,7 +2159,7 @@ retry:
 		return ret;
 
 	while (1) {
-		uint32_t cmd;
+		unsigned int cmd;
 		struct binder_transaction_data tr;
 		struct binder_work *w;
 		struct binder_transaction *t = NULL;
@@ -2183,9 +2183,9 @@ retry:
 		} break;
 		case BINDER_WORK_TRANSACTION_COMPLETE: {
 			cmd = BR_TRANSACTION_COMPLETE;
-			if (put_user(cmd, (uint32_t __user *)ptr))
+			if (put_user(cmd, (unsigned int __user *)ptr))
 				return -EFAULT;
-			ptr += sizeof(uint32_t);
+			ptr += sizeof(unsigned int);
 
 			binder_stat_br(proc, thread, cmd);
 			binder_debug(BINDER_DEBUG_TRANSACTION_COMPLETE,
@@ -2198,7 +2198,7 @@ retry:
 		} break;
 		case BINDER_WORK_NODE: {
 			struct binder_node *node = container_of(w, struct binder_node, work);
-			uint32_t cmd = BR_NOOP;
+			unsigned int cmd = BR_NOOP;
 			const char *cmd_name;
 			int strong = node->internal_strong_refs || node->local_strong_refs;
 			int weak = !hlist_empty(&node->refs) || node->local_weak_refs || strong;
@@ -2224,9 +2224,9 @@ retry:
 				node->has_weak_ref = 0;
 			}
 			if (cmd != BR_NOOP) {
-				if (put_user(cmd, (uint32_t __user *)ptr))
+				if (put_user(cmd, (unsigned int __user *)ptr))
 					return -EFAULT;
-				ptr += sizeof(uint32_t);
+				ptr += sizeof(unsigned int);
 				if (put_user(node->ptr, (void * __user *)ptr))
 					return -EFAULT;
 				ptr += sizeof(void *);
@@ -2260,16 +2260,16 @@ retry:
 		case BINDER_WORK_DEAD_BINDER_AND_CLEAR:
 		case BINDER_WORK_CLEAR_DEATH_NOTIFICATION: {
 			struct binder_ref_death *death;
-			uint32_t cmd;
+			unsigned int cmd;
 
 			death = container_of(w, struct binder_ref_death, work);
 			if (w->type == BINDER_WORK_CLEAR_DEATH_NOTIFICATION)
 				cmd = BR_CLEAR_DEATH_NOTIFICATION_DONE;
 			else
 				cmd = BR_DEAD_BINDER;
-			if (put_user(cmd, (uint32_t __user *)ptr))
+			if (put_user(cmd, (unsigned int __user *)ptr))
 				return -EFAULT;
-			ptr += sizeof(uint32_t);
+			ptr += sizeof(unsigned int);
 			if (put_user(death->cookie, (void * __user *)ptr))
 				return -EFAULT;
 			ptr += sizeof(void *);
@@ -2334,9 +2334,9 @@ retry:
 					ALIGN(t->buffer->data_size,
 					    sizeof(void *));
 
-		if (put_user(cmd, (uint32_t __user *)ptr))
+		if (put_user(cmd, (unsigned int __user *)ptr))
 			return -EFAULT;
-		ptr += sizeof(uint32_t);
+		ptr += sizeof(unsigned int);
 		if (copy_to_user(ptr, &tr, sizeof(tr)))
 			return -EFAULT;
 		ptr += sizeof(tr);
@@ -2379,7 +2379,7 @@ done:
 		binder_debug(BINDER_DEBUG_THREADS,
 			     "%d:%d BR_SPAWN_LOOPER\n",
 			     proc->pid, thread->pid);
-		if (put_user(BR_SPAWN_LOOPER, (uint32_t __user *)buffer))
+		if (put_user(BR_SPAWN_LOOPER, (unsigned int __user *)buffer))
 			return -EFAULT;
 		binder_stat_br(proc, thread, BR_SPAWN_LOOPER);
 	}
@@ -2752,7 +2752,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
 
 #ifdef CONFIG_CPU_CACHE_VIPT
 	if (cache_is_vipt_aliasing()) {
-		while (CACHE_COLOUR((vma->vm_start ^ (uint32_t)proc->buffer))) {
+		while (CACHE_COLOUR((vma->vm_start ^ (unsigned int)proc->buffer))) {
 			pr_info("binder_mmap: %d %lx-%lx maps %p bad alignment\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer);
 			vma->vm_start += PAGE_SIZE;
 		}
-- 
1.7.9.5

[PATCH 2/8] staging: android: binder: replace IOCTL types with user-exportable types

[Serban Constantinescu]

This patch modifies the IOCTL macros to use user-exportable data types,
as they are the referred kernel types for the user/kernel interface.

The patch does not change in any way the functionality of the binder driver.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index f240464..dbe81ce 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/binder.h
@@ -85,11 +85,11 @@ struct binder_version {
 #define BINDER_CURRENT_PROTOCOL_VERSION 7
 
 #define BINDER_WRITE_READ		_IOWR('b', 1, struct binder_write_read)
-#define	BINDER_SET_IDLE_TIMEOUT		_IOW('b', 3, int64_t)
+#define	BINDER_SET_IDLE_TIMEOUT		_IOW('b', 3, __s64)
 #define	BINDER_SET_MAX_THREADS		_IOW('b', 5, size_t)
-#define	BINDER_SET_IDLE_PRIORITY	_IOW('b', 6, int)
-#define	BINDER_SET_CONTEXT_MGR		_IOW('b', 7, int)
-#define	BINDER_THREAD_EXIT		_IOW('b', 8, int)
+#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)
 #define BINDER_VERSION			_IOWR('b', 9, struct binder_version)
 
 /*
-- 
1.7.9.5

[PATCH 3/8] staging: android: binder: fix binder interface for 64bit compat layer

[Serban Constantinescu]

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 fix the function prototypes for binder_thread_read/write
and make use of kernel types as well as user-exportable ones.

The changes do not affect existing 32bit ABI.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.c |    6 +++---
 drivers/staging/android/binder.h |   16 ++++++++--------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 5794cf6..caa6dde 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -1700,7 +1700,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)
 {
 	unsigned int cmd;
 	void __user *ptr = buffer + *consumed;
@@ -2080,8 +2080,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;
diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index dbe81ce..8012921 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/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 */
+		__s32	    handle;		/* remote object */
 	};
 
 	/* extra data associated with local object */
@@ -67,18 +67,18 @@ 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;
 };
 
 /* 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. */
-- 
1.7.9.5

[PATCH 4/8] staging: android: binder: fix printk() format specifier

[Serban Constantinescu]

Fix format specifiers warnings introduced by the previous patch which
changes some structures in binder.h.

The patch does not change in any way the functionality of the binder driver.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index caa6dde..a2cdd9e 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -1271,7 +1271,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;
 			}
@@ -1283,13 +1283,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;
 		}
@@ -1547,7 +1547,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;
@@ -1590,13 +1590,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;
@@ -1604,7 +1604,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;
@@ -1618,13 +1618,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;
@@ -2578,7 +2578,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 %016lx, read %zd at %016lx\n",
 			     proc->pid, thread->pid, bwr.write_size,
 			     bwr.write_buffer, bwr.read_size, bwr.read_buffer);
 
@@ -2604,7 +2604,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))) {
-- 
1.7.9.5

[PATCH 5/8] staging: android: binder: fix BINDER_SET_MAX_THREADS declaration

[Serban Constantinescu]

This change will fix the BINDER_SET_MAX_THREADS ioctl to use __s32
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 as an int and
is 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.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index 8012921..5b9909f 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/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, __s32)
 #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)
-- 
1.7.9.5

[PATCH 6/8] staging: android: binder: fix BC_FREE_BUFFER ioctl declaration

[Serban Constantinescu]

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.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index 5b9909f..8789baa 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/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
 	 */
-- 
1.7.9.5

[PATCH 7/8] staging: android: binder: fix alignment issues

[Serban Constantinescu]

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.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index a2cdd9e..b5c2b59 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -658,8 +658,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
 		return NULL;
 	}
 
-	size = ALIGN(data_size, sizeof(void *)) +
-		ALIGN(offsets_size, sizeof(void *));
+	size = ALIGN(data_size, sizeof(u32)) +
+		ALIGN(offsets_size, sizeof(u32));
 
 	if (size < data_size || size < offsets_size) {
 		binder_user_error("%d: got transaction with invalid size %zd-%zd\n",
@@ -807,8 +807,8 @@ static void binder_free_buf(struct binder_proc *proc,
 
 	buffer_size = binder_buffer_size(proc, buffer);
 
-	size = ALIGN(buffer->data_size, sizeof(void *)) +
-		ALIGN(buffer->offsets_size, sizeof(void *));
+	size = ALIGN(buffer->data_size, sizeof(u32)) +
+		ALIGN(buffer->offsets_size, sizeof(u32));
 
 	binder_debug(BINDER_DEBUG_BUFFER_ALLOC,
 		     "%d: binder_free_buf %p size %zd buffer_size %zd\n",
@@ -1247,7 +1247,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;
@@ -1496,7 +1496,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;
-- 
1.7.9.5

[PATCH 8/8] staging: android: binder: replace types with portable ones

[Serban Constantinescu]

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.

Signed-off-by: Serban Constantinescu <serban.constantinescu@arm.com>
---
 drivers/staging/android/binder.h |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index 8789baa..f3ffacd 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/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, __s32),
+	BC_ACQUIRE = _IOW('c', 5, __s32),
+	BC_RELEASE = _IOW('c', 6, __s32),
+	BC_DECREFS = _IOW('c', 7, __s32),
 	/*
 	 * int:	descriptor
 	 */
-- 
1.7.9.5

Re: [PATCH 0/8] Android Binder IPC Fixes

[Greg KH]

On Thu, Apr 04, 2013 at 01:32:30PM +0100, Serban Constantinescu wrote:
> Hi all,
> 
> This set of patches will clean-up and fix some of the issues that arise
> with the current binder interface when moving to a 64bit kernel. All these
> changes will not affect the existing 32bit Android interface and are meant
> to stand as the base for the 64bit binder compat layer.
> 
> This patch set has been successfully tested on 32bit platforms(ARMv7 VExpress)
> and 64bit platforms(ARMv8 RTSM) running a 32bit Android userspace and an in
> kernel binder compat layer.

I need some acks from some of the Android developers before I can take
this.

thanks,

greg k-h

Re: [PATCH 1/8] staging: android: binder: replace explicit size types

[Arve Hjønnevåg]

On Thu, Apr 4, 2013 at 5:32 AM, Serban Constantinescu
<serban.constantinescu@arm.com> wrote:
>
> Since the binder driver uses both uint32_t and unsigned int any further
> kernel changes will be difficult to read. This patch fixes the inconsistent
> types usage.
>

Would it make more sense to only change the types that need to be
larger on a 64 bit system?

--
Arve Hjønnevåg

Re: [PATCH 0/8] Android Binder IPC Fixes

[Arve Hjønnevåg]

On Fri, Apr 5, 2013 at 3:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Apr 04, 2013 at 01:32:30PM +0100, Serban Constantinescu wrote:
>> Hi all,
>>
>> This set of patches will clean-up and fix some of the issues that arise
>> with the current binder interface when moving to a 64bit kernel. All these
>> changes will not affect the existing 32bit Android interface and are meant
>> to stand as the base for the 64bit binder compat layer.
>>
>> This patch set has been successfully tested on 32bit platforms(ARMv7 VExpress)
>> and 64bit platforms(ARMv8 RTSM) running a 32bit Android userspace and an in
>> kernel binder compat layer.
>
> I need some acks from some of the Android developers before I can take
> this.
>

I still think it is better to change user-space to use 64 bit pointer
types when running on a 64 bit kernel. These changes do not seem to
allow 64 bit user-space processes on a 64 bit kernel.

--
Arve Hjønnevåg

Re: [PATCH 1/8] staging: android: binder: replace explicit size types

[Greg KH]

On Fri, Apr 05, 2013 at 04:17:47PM -0700, Arve Hjønnevåg wrote:
> On Thu, Apr 4, 2013 at 5:32 AM, Serban Constantinescu
> <serban.constantinescu@arm.com> wrote:
> >
> > Since the binder driver uses both uint32_t and unsigned int any further
> > kernel changes will be difficult to read. This patch fixes the inconsistent
> > types usage.
> >
> 
> Would it make more sense to only change the types that need to be
> larger on a 64 bit system?

I agree.  You are also changing the type from explicit to "unexplicit",
the exact opposite from what I would be expecting to see here.  Why?

Re: [PATCH 4/8] staging: android: binder: fix printk() format specifier

[Greg KH]

On Thu, Apr 04, 2013 at 01:32:34PM +0100, Serban Constantinescu wrote:
> Fix format specifiers warnings introduced by the previous patch which
> changes some structures in binder.h.

Please merge this with the patch that caused the problems.

Re: [PATCH 1/8] staging: android: binder: replace explicit size types

[Serban Constantinescu]

On 06/04/13 07:00, Greg KH wrote:
> On Fri, Apr 05, 2013 at 04:17:47PM -0700, Arve Hjønnevåg wrote:
>> On Thu, Apr 4, 2013 at 5:32 AM, Serban Constantinescu
>> <serban.constantinescu@arm.com> wrote:
>>>
>>> Since the binder driver uses both uint32_t and unsigned int any further
>>> kernel changes will be difficult to read. This patch fixes the inconsistent
>>> types usage.
>>>
>>
>> Would it make more sense to only change the types that need to be
>> larger on a 64 bit system?
>
> I agree.  You are also changing the type from explicit to "unexplicit",
> the exact opposite from what I would be expecting to see here.  Why?

I have changed the types used so that they seem consistent throughout 
the driver(uint32_t is used in some of the internal structures and some 
of the function prototypes). Changing these types to kernel explicit 
types would have meant that we would have the same inconsistency for u32 
types.

We can live without this patch and I can take a look at modifying the 
function prototypes, internal structures and kernel/userspace 
interaction to use explicit kernel types it that is preferred.

Thanks for your feedback,
Serban

Re: [PATCH 0/8] Android Binder IPC Fixes

[Serban Constantinescu]

On 06/04/13 00:38, Arve Hjønnevåg wrote:
> On Fri, Apr 5, 2013 at 3:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Thu, Apr 04, 2013 at 01:32:30PM +0100, Serban Constantinescu wrote:
>>> Hi all,
>>>
>>> This set of patches will clean-up and fix some of the issues that arise
>>> with the current binder interface when moving to a 64bit kernel. All these
>>> changes will not affect the existing 32bit Android interface and are meant
>>> to stand as the base for the 64bit binder compat layer.
>>>
>>> This patch set has been successfully tested on 32bit platforms(ARMv7 VExpress)
>>> and 64bit platforms(ARMv8 RTSM) running a 32bit Android userspace and an in
>>> kernel binder compat layer.
>>
>> I need some acks from some of the Android developers before I can take
>> this.
>>
>
> I still think it is better to change user-space to use 64 bit pointer
> types when running on a 64 bit kernel. These changes do not seem to
> allow 64 bit user-space processes on a 64 bit kernel.

This patch set is independent of whether the binder compat layer lives 
in the kernel or in the userspace. However we need these changes so that 
the same kernel driver supports calls from 32bit userspace and calls 
from 64bit userspace(from the binder kernel driver perspective same as 
64bit kernel/32bit userspace with the compat layer expanding pointers 
and structures to 64bit in the userspace).

We have tested 64/64 functionality using a 64bit Linux filesystem and 
64bit binder unit tests. All tests passed for both kernel/filesystem 
configurations supported by this patch set - 32/32 and 64/64.

Thanks for your feedback,
Serban

Re: [PATCH 1/8] staging: android: binder: replace explicit size types

[Greg KH]

On Mon, Apr 08, 2013 at 01:25:14PM +0100, Serban Constantinescu wrote:
> On 06/04/13 07:00, Greg KH wrote:
> >On Fri, Apr 05, 2013 at 04:17:47PM -0700, Arve Hjønnevåg wrote:
> >>On Thu, Apr 4, 2013 at 5:32 AM, Serban Constantinescu
> >><serban.constantinescu@arm.com> wrote:
> >>>
> >>>Since the binder driver uses both uint32_t and unsigned int any further
> >>>kernel changes will be difficult to read. This patch fixes the inconsistent
> >>>types usage.
> >>>
> >>
> >>Would it make more sense to only change the types that need to be
> >>larger on a 64 bit system?
> >
> >I agree.  You are also changing the type from explicit to "unexplicit",
> >the exact opposite from what I would be expecting to see here.  Why?
> 
> I have changed the types used so that they seem consistent
> throughout the driver(uint32_t is used in some of the internal
> structures and some of the function prototypes). Changing these
> types to kernel explicit types would have meant that we would have
> the same inconsistency for u32 types.

What do you mean?  You changed uint32_t types to "int", which seems like
a step backwards from being explicit on how things should be.

If this is just a "cleanup" change, then great, but say it is such.
Your commit log message is odd in that you are saying it is making
things easier to read, yet I'm sure confused by it :)

greg k-h