[PATCH v1 4/9] staging: android: binder: Add align_helper() macro
This patch adds align_helper() macro that will be used for enforcing
the desired alignment on 64bit systems where the alignment will differ
depending on the userspace used (32bit /64bit).
This patch is a temporary patch that will be extended with 32bit compat
handling.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c |9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 16109cd..6719a53 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -140,6 +140,8 @@ module_param_call(stop_on_user_error,
binder_set_stop_on_user_error,
binder_stop_on_user_error = 2; \
} while (0)
+#define align_helper(ptr) ALIGN(ptr, sizeof(void *))
+
enum binder_stat_types {
BINDER_STAT_PROC,
BINDER_STAT_THREAD,
@@ -1239,7 +1241,7 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
if (buffer->target_node)
binder_dec_node(buffer->target_node, 1, 0);
- offp = (size_t *)(buffer->data + ALIGN(buffer->data_size, sizeof(void
*)));
+ offp = (size_t *)(buffer->data + align_helper(buffer->data_size));
if (failed_at)
off_end = failed_at;
else
@@ -1472,7 +1474,7 @@ static void binder_transaction(struct binder_proc *proc,
if (target_node)
binder_inc_node(target_node, 1, 0, NULL);
- offp = (size_t *)(t->buffer->data + ALIGN(tr->data_size, sizeof(void
*)));
+ offp = (size_t *)(t->buffer->data + align_helper(tr->data_size));
if (copy_from_user(t->buffer->data, tr->data.ptr.buffer,
tr->data_size)) {
binder_user_error("%d:%d got transaction with invalid data
ptr\n",
@@ -2378,8 +2380,7 @@ retry:
tr.data.ptr.buffer = (void *)t->buffer->data +
proc->user_buffer_offset;
tr.data.ptr.offsets = tr.data.ptr.buffer +
- ALIGN(t->buffer->data_size,
- sizeof(void *));
+ align_helper(t->buffer->data_size);
if (binder_copy_to_user(cmd, &tr, &ptr, sizeof(struct
binder_transaction_data)))
return -EFAULT;
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 6/9] staging: android: binder: Add size_helper() macro
This patch adds size_helper() macro that will be used for indexing into
different buffers on 64bit systems where the size of particular
structures will differ depending on the userspace used (32bit /64bit).
This patch is a temporary patch that will be extended with 32bit compat
handling.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 95c2581..6d22717 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -142,6 +142,7 @@ module_param_call(stop_on_user_error,
binder_set_stop_on_user_error,
#define align_helper(ptr) ALIGN(ptr, sizeof(void *))
#define deref_helper(ptr) (*(typeof(size_t *))ptr)
+#define size_helper(x) sizeof(x)
enum binder_stat_types {
BINDER_STAT_PROC,
@@ -1247,10 +1248,10 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
off_end = failed_at;
else
off_end = (void *)offp + buffer->offsets_size;
- for (; offp < off_end; offp += sizeof(size_t)) {
+ for (; offp < off_end; offp += size_helper(size_t)) {
struct flat_binder_object *fp;
- if (deref_helper(offp) > buffer->data_size - sizeof(*fp) ||
- buffer->data_size < sizeof(*fp) ||
+ if (deref_helper(offp) > buffer->data_size - size_helper(*fp) ||
+ buffer->data_size < size_helper(*fp) ||
!IS_ALIGNED(deref_helper(offp), sizeof(u32))) {
pr_err("transaction release %d bad offset %zd, size
%zd\n",
debug_id, deref_helper(offp), buffer->data_size);
@@ -1489,17 +1490,17 @@ static void binder_transaction(struct binder_proc *proc,
return_error = BR_FAILED_REPLY;
goto err_copy_data_failed;
}
- if (!IS_ALIGNED(tr->offsets_size, sizeof(size_t))) {
+ if (!IS_ALIGNED(tr->offsets_size, size_helper(size_t))) {
binder_user_error("%d:%d got transaction with invalid offsets
size, %zd\n",
proc->pid, thread->pid, tr->offsets_size);
return_error = BR_FAILED_REPLY;
goto err_bad_offset;
}
off_end = (void *)offp + tr->offsets_size;
- for (; offp < off_end; offp += sizeof(size_t)) {
+ for (; offp < off_end; offp += size_helper(size_t)) {
struct flat_binder_object *fp;
- if (deref_helper(offp) > t->buffer->data_size - sizeof(*fp) ||
- t->buffer->data_size < sizeof(*fp) ||
+ if (deref_helper(offp) > t->buffer->data_size -
size_helper(*fp) ||
+ t->buffer->data_size < size_helper(*fp) ||
!IS_ALIGNED(deref_helper(offp), sizeof(u32))) {
binder_user_error("%d:%d got transaction with invalid
offset, %zd\n",
proc->pid, thread->pid,
deref_helper(offp));
@@ -2229,7 +2230,7 @@ retry:
break;
}
- if (end - ptr < sizeof(tr) + 4)
+ if (end - ptr < size_helper(tr) + 4)
break;
switch (w->type) {
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 2/9] staging: android: binder: Add binder_copy_to_user()
This patch adds binder_copy_to_user() to be used for copying binder
commands to user address space. This way we can abstract away the
copy_to_user() calls and add separate handling for the compat layer.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 39 --
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 233889c..6fbb340 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -2117,6 +2117,18 @@ static int binder_has_thread_work(struct binder_thread
*thread)
(thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
}
+static int binder_copy_to_user(uint32_t cmd, void *parcel,
+ void __user **ptr, size_t size)
+{
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, parcel, size))
+ return -EFAULT;
+ *ptr += size;
+ return 0;
+}
+
static int binder_thread_read(struct binder_proc *proc,
struct binder_thread *thread,
void __user *buffer, size_t size,
@@ -2263,15 +2275,12 @@ retry:
node->has_weak_ref = 0;
}
if (cmd != BR_NOOP) {
- if (put_user(cmd, (uint32_t __user *)ptr))
- return -EFAULT;
- ptr += sizeof(uint32_t);
- if (put_user(node->ptr, (void * __user *)ptr))
- return -EFAULT;
- ptr += sizeof(void *);
- if (put_user(node->cookie, (void * __user
*)ptr))
+ struct binder_ptr_cookie tmp;
+
+ tmp.ptr = node->ptr;
+ tmp.cookie = node->cookie;
+ if (binder_copy_to_user(cmd, &tmp, &ptr,
sizeof(struct binder_ptr_cookie)))
return -EFAULT;
- ptr += sizeof(void *);
binder_stat_br(proc, thread, cmd);
binder_debug(BINDER_DEBUG_USER_REFS,
@@ -2306,12 +2315,10 @@ retry:
cmd = BR_CLEAR_DEATH_NOTIFICATION_DONE;
else
cmd = BR_DEAD_BINDER;
- if (put_user(cmd, (uint32_t __user *)ptr))
- return -EFAULT;
- ptr += sizeof(uint32_t);
- if (put_user(death->cookie, (void * __user *)ptr))
+
+ if (binder_copy_to_user(cmd, &death->cookie, &ptr,
sizeof(void *)))
return -EFAULT;
- ptr += sizeof(void *);
+
binder_stat_br(proc, thread, cmd);
binder_debug(BINDER_DEBUG_DEATH_NOTIFICATION,
"%d:%d %s %p\n",
@@ -2373,12 +2380,8 @@ retry:
ALIGN(t->buffer->data_size,
sizeof(void *));
- if (put_user(cmd, (uint32_t __user *)ptr))
- return -EFAULT;
- ptr += sizeof(uint32_t);
- if (copy_to_user(ptr, &tr, sizeof(tr)))
+ if (binder_copy_to_user(cmd, &tr, &ptr, sizeof(struct
binder_transaction_data)))
return -EFAULT;
- ptr += sizeof(tr);
trace_binder_transaction_received(t);
binder_stat_br(proc, thread, cmd);
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 5/9] staging: android: binder: Add deref_helper() macro
This patch adds a deref_helper() macro that will be used to dereference
the binder offsets on 64bit systems where the offset is either a 32bit
or a 64bit value, depending on the userpace used (32bit /64bit)
This patch is a temporary patch that will be extended with 32bit compat
handling.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 6719a53..95c2581 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -141,6 +141,7 @@ module_param_call(stop_on_user_error,
binder_set_stop_on_user_error,
} while (0)
#define align_helper(ptr) ALIGN(ptr, sizeof(void *))
+#define deref_helper(ptr) (*(typeof(size_t *))ptr)
enum binder_stat_types {
BINDER_STAT_PROC,
@@ -1230,7 +1231,7 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
struct binder_buffer *buffer,
size_t *failed_at)
{
- size_t *offp, *off_end;
+ void *offp, *off_end;
int debug_id = buffer->debug_id;
binder_debug(BINDER_DEBUG_TRANSACTION,
@@ -1246,16 +1247,16 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
off_end = failed_at;
else
off_end = (void *)offp + buffer->offsets_size;
- for (; offp < off_end; offp++) {
+ for (; offp < off_end; offp += sizeof(size_t)) {
struct flat_binder_object *fp;
- if (*offp > buffer->data_size - sizeof(*fp) ||
+ if (deref_helper(offp) > buffer->data_size - sizeof(*fp) ||
buffer->data_size < sizeof(*fp) ||
- !IS_ALIGNED(*offp, sizeof(u32))) {
+ !IS_ALIGNED(deref_helper(offp), sizeof(u32))) {
pr_err("transaction release %d bad offset %zd, size
%zd\n",
-debug_id, *offp, buffer->data_size);
+debug_id, deref_helper(offp), buffer->data_size);
continue;
}
- fp = (struct flat_binder_object *)(buffer->data + *offp);
+ fp = (struct flat_binder_object *)(buffer->data +
deref_helper(offp));
switch (fp->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER: {
@@ -1305,7 +1306,7 @@ static void binder_transaction(struct binder_proc *proc,
{
struct binder_transaction *t;
struct binder_work *tcomplete;
- size_t *offp, *off_end;
+ void *offp, *off_end;
struct binder_proc *target_proc;
struct binder_thread *target_thread = NULL;
struct binder_node *target_node = NULL;
@@ -1495,17 +1496,17 @@ static void binder_transaction(struct binder_proc *proc,
goto err_bad_offset;
}
off_end = (void *)offp + tr->offsets_size;
- for (; offp < off_end; offp++) {
+ for (; offp < off_end; offp += sizeof(size_t)) {
struct flat_binder_object *fp;
- if (*offp > t->buffer->data_size - sizeof(*fp) ||
+ if (deref_helper(offp) > t->buffer->data_size - sizeof(*fp) ||
t->buffer->data_size < sizeof(*fp) ||
- !IS_ALIGNED(*offp, sizeof(u32))) {
+ !IS_ALIGNED(deref_helper(offp), sizeof(u32))) {
binder_user_error("%d:%d got transaction with invalid
offset, %zd\n",
- proc->pid, thread->pid, *offp);
+ proc->pid, thread->pid,
deref_helper(offp));
return_error = BR_FAILED_REPLY;
goto err_bad_offset;
}
- fp = (struct flat_binder_object *)(t->buffer->data + *offp);
+ fp = (struct flat_binder_object *)(t->buffer->data +
deref_helper(offp));
switch (fp->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER: {
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 0/9] Android: Add Support for Binder Compat
Hi all, The patches attached add support for 32bit userspace running on 64bit kernels. This is the last series of patches needed for basic 32bit Android bring-up on 64bit kernels. The series is split into refactoring the binder driver and the addition of the compat layer. Please note that the helper macros are extended with compat handling by the last patch of the series. The patches have been successfully tested on 32 and 64bit platforms. Thanks for your help, Best regards, Serban Constantinescu Serban Constantinescu (9): staging: android: binder: Move some of the logic into subfunction staging: android: binder: Add binder_copy_to_user() staging: android: binder: Add cmd == CMD_NAME handling staging: android: binder: Add align_helper() macro staging: android: binder: Add deref_helper() macro staging: android: binder: Add size_helper() macro staging: android: binder: Add copy_flat_binder_object() staging: android: binder: Add binder compat handling to binder.h staging: android: binder: Add binder compat layer drivers/staging/android/binder.c | 846 -- drivers/staging/android/binder.h | 109 + 2 files changed, 732 insertions(+), 223 deletions(-) -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 9/9] staging: android: binder: Add binder compat layer
This patch adds support for 32bit userspace running on 64bit kernels.
All the changes done in this patch have been tested on 32bit and 64bit
systems.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 355 +-
1 file changed, 353 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 855d348..941973a 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "binder.h"
#include "binder_trace.h"
@@ -140,6 +141,77 @@ module_param_call(stop_on_user_error,
binder_set_stop_on_user_error,
binder_stop_on_user_error = 2; \
} while (0)
+#ifdef CONFIG_COMPAT
+static inline void writeback_flat_binder_object(struct flat_binder_object *fp,
+void __user *ptr)
+{
+ struct compat_flat_binder_object *tmp_fp;
+
+ tmp_fp = (struct compat_flat_binder_object *) ptr;
+ tmp_fp->type = fp->type;
+ tmp_fp->flags = fp->flags;
+ tmp_fp->binder = ptr_to_compat(fp->binder);
+ tmp_fp->cookie = ptr_to_compat(fp->cookie);
+}
+
+static inline struct flat_binder_object *copy_flat_binder_object(void __user
*ptr)
+{
+ struct flat_binder_object *fp;
+ struct compat_flat_binder_object *tmp_fp;
+
+ if (!is_compat_task())
+ return (struct flat_binder_object *) ptr;
+
+ fp = kzalloc(sizeof(*fp), GFP_KERNEL);
+ if (fp == NULL)
+ return NULL;
+
+ tmp_fp = (struct compat_flat_binder_object *) ptr;
+ /* copy compat struct */
+ fp->type = tmp_fp->type;
+ fp->flags = tmp_fp->flags;
+ fp->binder = compat_ptr(tmp_fp->binder);
+ fp->cookie = compat_ptr(tmp_fp->cookie);
+
+ return fp;
+}
+
+static inline uint32_t compat_change_size(uint32_t cmd, size_t size)
+{
+ uint32_t compat_cmd;
+
+ compat_cmd = cmd & ~IOCSIZE_MASK;
+ compat_cmd = compat_cmd | ((size << _IOC_SIZESHIFT) & IOCSIZE_MASK);
+ return compat_cmd;
+}
+
+#define align_helper(x) ( \
+ is_compat_task() ? \
+ ALIGN(x, sizeof(compat_uptr_t)) : \
+ ALIGN(x, sizeof(void *))\
+ )
+
+#define deref_helper(x) ( \
+ is_compat_task() ? \
+ (size_t) *(compat_size_t *)x : \
+ *(size_t *)x\
+ )
+
+#define size_helper(x) ({ \
+ size_t __size; \
+ if (!is_compat_task()) \
+ __size = sizeof(x); \
+ else if (sizeof(x) == sizeof(struct flat_binder_object))\
+ __size = sizeof(struct compat_flat_binder_object); \
+ else if (sizeof(x) == sizeof(struct binder_transaction_data)) \
+ __size = sizeof(struct compat_binder_transaction_data); \
+ else if (sizeof(x) == sizeof(size_t)) \
+ __size = sizeof(compat_size_t); \
+ else\
+BUG(); \
+ __size; \
+ })
+#else
#define align_helper(ptr) ALIGN(ptr, sizeof(void *))
#define deref_helper(ptr) (*(typeof(size_t *))ptr)
#define size_helper(x) sizeof(x)
@@ -148,6 +220,7 @@ static inline struct flat_binder_object
*copy_flat_binder_object(void __user *pt
{
return (struct flat_binder_object *)ptr;
}
+#endif
enum binder_stat_types {
BINDER_STAT_PROC,
@@ -1254,7 +1327,7 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
else
off_end = (void *)offp + buffer->offsets_size;
for (; offp < off_end; offp += size_helper(size_t)) {
- struct flat_binder_object *fp;
+ struct flat_binder_object *fp = NULL;
if (deref_helper(offp) > buffer->data_size - size_helper(*fp) ||
buffer->data_size < size_helper(*fp) ||
!IS_ALIGNED(deref_helper(offp), sizeof(u32))) {
@@ -1303,6 +1376,8 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
[PATCH v1 3/9] staging: android: binder: Add cmd == CMD_NAME handling
This patch modifies the functions that need to be passed the explicit
command to use a boolean flag. This way we can reuse the code for 64bit
compat commands.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 6fbb340..16109cd 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -1701,7 +1701,7 @@ err_no_context_mgr_node:
}
static void bc_increfs_done(struct binder_proc *proc,
- struct binder_thread *thread, uint32_t cmd,
+ struct binder_thread *thread, bool acquire,
void __user *node_ptr, void __user *cookie)
{
struct binder_node *node;
@@ -1710,22 +1710,22 @@ static void bc_increfs_done(struct binder_proc *proc,
if (node == NULL) {
binder_user_error("%d:%d %s u%p no match\n",
proc->pid, thread->pid,
- cmd == BC_INCREFS_DONE ?
- "BC_INCREFS_DONE" :
- "BC_ACQUIRE_DONE",
+ acquire ?
+ "BC_ACQUIRE_DONE" :
+ "BC_INCREFS_DONE",
node_ptr);
return;
}
if (cookie != node->cookie) {
binder_user_error("%d:%d %s u%p node %d cookie mismatch %p !=
%p\n",
proc->pid, thread->pid,
- cmd == BC_INCREFS_DONE ?
- "BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
+ acquire ?
+ "BC_ACQUIRE_DONE" : "BC_INCREFS_DONE",
node_ptr, node->debug_id,
cookie, node->cookie);
return;
}
- if (cmd == BC_ACQUIRE_DONE) {
+ if (acquire) {
if (node->pending_strong_ref == 0) {
binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no
pending acquire request\n",
proc->pid, thread->pid,
@@ -1742,13 +1742,13 @@ static void bc_increfs_done(struct binder_proc *proc,
}
node->pending_weak_ref = 0;
}
- binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
+ binder_dec_node(node, acquire, 0);
binder_debug(BINDER_DEBUG_USER_REFS,
"%d:%d %s node %d ls %d lw %d\n",
proc->pid, thread->pid,
-cmd == BC_INCREFS_DONE ?
-"BC_INCREFS_DONE" :
-"BC_ACQUIRE_DONE",
+acquire ?
+"BC_ACQUIRE_DONE" :
+"BC_INCREFS_DONE",
node->debug_id, node->local_strong_refs,
node->local_weak_refs);
return;
@@ -1793,7 +1793,7 @@ static void bc_free_buffer(struct binder_proc *proc,
}
static void bc_clear_death_notif(struct binder_proc *proc,
- struct binder_thread *thread, uint32_t cmd,
+ struct binder_thread *thread, bool request,
uint32_t target, void __user *cookie)
{
struct binder_ref *ref;
@@ -1803,7 +1803,7 @@ static void bc_clear_death_notif(struct binder_proc *proc,
if (ref == NULL) {
binder_user_error("%d:%d %s invalid ref %d\n",
proc->pid, thread->pid,
- cmd == BC_REQUEST_DEATH_NOTIFICATION ?
+ request ?
"BC_REQUEST_DEATH_NOTIFICATION" :
"BC_CLEAR_DEATH_NOTIFICATION",
target);
@@ -1813,13 +1813,13 @@ static void bc_clear_death_notif(struct binder_proc
*proc,
binder_debug(BINDER_DEBUG_DEATH_NOTIFICATION,
"%d:%d %s %p ref %d desc %d s %d w %d for node %d\n",
proc->pid, thread->pid,
-cmd == BC_REQUEST_DEATH_NOTIFICATION ?
+request ?
"BC_REQUEST_DEATH_NOTIFICATION" :
"BC_CLEAR_DEATH_NOTIFICATION",
cookie, ref->debug_id, ref->desc,
ref->strong, ref->weak, ref->node->debug_id);
- if (cmd == BC_REQUEST_DEATH_NOTIFICATION) {
+ if (request) {
if (ref->death) {
binder_user_error("%d:%d BC_REQUEST_DEATH_NOTIFICATION
death notification already set\n",
proc->pid, thread->pid);
@@ -1993,7 +1993,7 @@ static int binder_thread_write(struct
[PATCH v1 7/9] staging: android: binder: Add copy_flat_binder_object()
This patch adds copy_flat_binder_object macro() that will help
dereference struct flat_binder_object on 64bit systems where the
structure differs between 32bit and 64bit userspace.
This patch is a temporary patch that will be extended with 32bit compat
handling.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c |9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 6d22717..855d348 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -144,6 +144,11 @@ module_param_call(stop_on_user_error,
binder_set_stop_on_user_error,
#define deref_helper(ptr) (*(typeof(size_t *))ptr)
#define size_helper(x) sizeof(x)
+static inline struct flat_binder_object *copy_flat_binder_object(void __user
*ptr)
+{
+ return (struct flat_binder_object *)ptr;
+}
+
enum binder_stat_types {
BINDER_STAT_PROC,
BINDER_STAT_THREAD,
@@ -1257,7 +1262,7 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
debug_id, deref_helper(offp), buffer->data_size);
continue;
}
- fp = (struct flat_binder_object *)(buffer->data +
deref_helper(offp));
+ fp = copy_flat_binder_object(buffer->data + deref_helper(offp));
switch (fp->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER: {
@@ -1507,7 +1512,7 @@ static void binder_transaction(struct binder_proc *proc,
return_error = BR_FAILED_REPLY;
goto err_bad_offset;
}
- fp = (struct flat_binder_object *)(t->buffer->data +
deref_helper(offp));
+ fp = copy_flat_binder_object(t->buffer->data +
deref_helper(offp));
switch (fp->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER: {
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 8/9] staging: android: binder: Add binder compat handling to binder.h
This patch adds all the needed compat structures to binder.h. All the
structures defined in this patch mirror the structure and size of 32bit
ones.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.h | 109 ++
1 file changed, 109 insertions(+)
diff --git a/drivers/staging/android/binder.h b/drivers/staging/android/binder.h
index cbe3451..6c9849d 100644
--- a/drivers/staging/android/binder.h
+++ b/drivers/staging/android/binder.h
@@ -22,6 +22,10 @@
#include
+#ifdef CONFIG_COMPAT
+#include
+#endif
+
#define B_PACK_CHARS(c1, c2, c3, c4) \
c1)<<24)) | (((c2)<<16)) | (((c3)<<8)) | (c4))
#define B_TYPE_LARGE 0x85
@@ -326,5 +330,110 @@ enum binder_driver_command_protocol {
*/
};
+/* Support for 32bit userspace on a 64bit system */
+#ifdef CONFIG_COMPAT
+struct compat_flat_binder_object {
+ /* 8 bytes for large_flat_header. */
+ __u32 type;
+ __u32 flags;
+
+ /* 8 bytes of data. */
+ union {
+ compat_uptr_t binder; /* local object */
+ __u32 handle; /* remote object */
+ };
+
+ /* extra data associated with local object */
+ compat_uptr_t cookie;
+};
+
+struct compat_binder_write_read {
+ compat_size_t write_size; /* bytes to write */
+ compat_size_t write_consumed; /* bytes consumed by driver */
+ compat_ulong_t write_buffer;
+ compat_size_t read_size; /* bytes to read */
+ compat_size_t read_consumed; /* bytes consumed by driver */
+ compat_ulong_t read_buffer;
+};
+
+#define COMPAT_BINDER_WRITE_READ _IOWR('b', 1, struct
compat_binder_write_read)
+
+struct compat_binder_transaction_data {
+ /* The first two are only used for bcTRANSACTION and brTRANSACTION,
+* identifying the target and contents of the transaction.
+*/
+ union {
+ __u32 handle; /* target descriptor of command
transaction */
+ compat_uptr_t ptr;/* target descriptor of return
transaction */
+ } target;
+ compat_uptr_t cookie; /* target object cookie */
+ __u32 code; /* transaction command */
+
+ /* General information about the transaction. */
+ __u32 flags;
+ pid_t sender_pid;
+ uid_t sender_euid;
+ compat_size_t data_size; /* number of bytes of data */
+ compat_size_t offsets_size; /* number of bytes of offsets */
+
+ /* If this transaction is inline, the data immediately
+* follows here; otherwise, it ends with a pointer to
+* the data buffer.
+*/
+ union {
+ struct {
+ /* transaction data */
+ compat_uptr_t buffer;
+ /* offsets from buffer to flat_binder_object structs */
+ compat_uptr_t offsets;
+ } ptr;
+ __u8buf[8];
+ } data;
+};
+
+struct compat_binder_ptr_cookie {
+ compat_uptr_t ptr;
+ compat_uptr_t cookie;
+};
+
+/* legacy - not used anymore */
+struct compat_binder_pri_ptr_cookie {
+ __s32 priority;
+ compat_uptr_t ptr;
+ compat_uptr_t cookie;
+};
+
+enum compat_binder_driver_return_protocol {
+ COMPAT_BR_TRANSACTION = _IOR('r', 2, struct
compat_binder_transaction_data),
+ COMPAT_BR_REPLY = _IOR('r', 3, struct compat_binder_transaction_data),
+
+ COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie),
+ COMPAT_BR_ACQUIRE = _IOR('r', 8, struct compat_binder_ptr_cookie),
+ COMPAT_BR_RELEASE = _IOR('r', 9, struct compat_binder_ptr_cookie),
+ COMPAT_BR_DECREFS = _IOR('r', 10, struct compat_binder_ptr_cookie),
+
+ /* legacy - not used anymore */
+ COMPAT_BR_ATTEMPT_ACQUIRE = _IOR('r', 11, struct
compat_binder_pri_ptr_cookie),
+
+ COMPAT_BR_DEAD_BINDER = _IOR('r', 15, compat_uptr_t),
+ COMPAT_BR_CLEAR_DEATH_NOTIFICATION_DONE = _IOR('r', 16, compat_uptr_t),
+};
+
+enum compat_binder_driver_command_protocol {
+ COMPAT_BC_TRANSACTION = _IOW('c', 0, struct
compat_binder_transaction_data),
+ COMPAT_BC_REPLY = _IOW('c', 1, struct compat_binder_transaction_data),
+
+ COMPAT_BC_FREE_BUFFER = _IOW('c', 3, compat_uptr_t),
+
+ COMPAT_BC_INCREFS_DONE = _IOW('c', 8, struct compat_binder_ptr_cookie),
+ COMPAT_BC_ACQUIRE_DONE = _IOW('c', 9, struct compat_binder_ptr_cookie),
+
+ COMPAT_BC_REQUEST_DEATH_NOTIFICATION = _IOW('c', 14, struct
compat_binder_ptr_cookie),
+ COMPAT_BC_CLEAR_DEATH_NOTIFICATION = _IOW('c', 15, struct
compat_binder_ptr_cookie),
+
+ COMPAT_BC_DEAD_BINDER_DONE = _IOW('c', 1
[PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
This patch moves some of the logic for binder_thread_write() into
subfunctions. This way we can share more code with the binder compat
layer.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 403 +-
1 file changed, 220 insertions(+), 183 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index eaec1da..233889c 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -1700,6 +1700,217 @@ err_no_context_mgr_node:
thread->return_error = return_error;
}
+static void bc_increfs_done(struct binder_proc *proc,
+ struct binder_thread *thread, uint32_t cmd,
+ void __user *node_ptr, void __user *cookie)
+{
+ struct binder_node *node;
+
+ node = binder_get_node(proc, node_ptr);
+ if (node == NULL) {
+ binder_user_error("%d:%d %s u%p no match\n",
+ proc->pid, thread->pid,
+ cmd == BC_INCREFS_DONE ?
+ "BC_INCREFS_DONE" :
+ "BC_ACQUIRE_DONE",
+ node_ptr);
+ return;
+ }
+ if (cookie != node->cookie) {
+ binder_user_error("%d:%d %s u%p node %d cookie mismatch %p !=
%p\n",
+ proc->pid, thread->pid,
+ cmd == BC_INCREFS_DONE ?
+ "BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
+ node_ptr, node->debug_id,
+ cookie, node->cookie);
+ return;
+ }
+ if (cmd == BC_ACQUIRE_DONE) {
+ if (node->pending_strong_ref == 0) {
+ binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no
pending acquire request\n",
+ proc->pid, thread->pid,
+ node->debug_id);
+ return;
+ }
+ node->pending_strong_ref = 0;
+ } else {
+ if (node->pending_weak_ref == 0) {
+ binder_user_error("%d:%d BC_INCREFS_DONE node %d has no
pending increfs request\n",
+ proc->pid, thread->pid,
+ node->debug_id);
+ return;
+ }
+ node->pending_weak_ref = 0;
+ }
+ binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
+ binder_debug(BINDER_DEBUG_USER_REFS,
+"%d:%d %s node %d ls %d lw %d\n",
+proc->pid, thread->pid,
+cmd == BC_INCREFS_DONE ?
+"BC_INCREFS_DONE" :
+"BC_ACQUIRE_DONE",
+node->debug_id, node->local_strong_refs,
+node->local_weak_refs);
+ return;
+}
+
+static void bc_free_buffer(struct binder_proc *proc,
+ struct binder_thread *thread, void __user *data_ptr)
+{
+ struct binder_buffer *buffer;
+
+ buffer = binder_buffer_lookup(proc, data_ptr);
+ if (buffer == NULL) {
+ binder_user_error("%d:%d BC_FREE_BUFFER u%p no match\n",
+ proc->pid, thread->pid, data_ptr);
+ return;
+ }
+ if (!buffer->allow_user_free) {
+ binder_user_error("%d:%d BC_FREE_BUFFER u%p matched unreturned
buffer\n",
+ proc->pid, thread->pid, data_ptr);
+ return;
+ }
+ binder_debug(BINDER_DEBUG_FREE_BUFFER,
+"%d:%d BC_FREE_BUFFER u%p found buffer %d for %s
transaction\n",
+proc->pid, thread->pid, data_ptr, buffer->debug_id,
+buffer->transaction ? "active" : "finished");
+
+ if (buffer->transaction) {
+ buffer->transaction->buffer = NULL;
+ buffer->transaction = NULL;
+ }
+ if (buffer->async_transaction && buffer->target_node) {
+ BUG_ON(!buffer->target_node->has_async_transaction);
+ if (list_empty(&buffer->target_node->async_todo))
+ buffer->target_node->has_async_transaction = 0;
+ else
+ list_move_tail(buffer->target_node->async_todo.next,
&thread->todo);
+ }
+ trace_binder_transaction_buffer_release(buffer);
+ binder_transaction_buffer_release(proc, buffer, NULL);
+ binder_free_buf(proc, buffer);
+ return;
+}
+
+static void bc_clear_death_notif(struct binder_proc *proc,
+ struct binder_thread *thread, uint32_t cmd,
+ uint32_
Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer
Hi all,
Thanks for your feedback! Sadly enough, being in a different time-zone,
is not useful.
Sorry for the confusion related to why is this patch needed or not. I
should highlight a bit more what is the patch enabling and what would be
the different alternatives, at least from my perspective.
*64bit kernel/ 32bit userspace*
This patch series adds support for 32bit userspace running on 64bit
kernels. Thus by applying this patch to your kernel you will be able to
use any existing 32bit Android userspace on your 64bit platform, running
a 64bit kernel. That is pure 32bit userspace with no 64bit support!
This means *no modifications to the 32bit userspace*. Therefore any
applications or userspace side drivers, that uses the binder interface
at a native level will not have to be modified. These kind of
applications are not "good citizens" - the native binder API is not
exposed in the Android NDK. However I do not know how many applications
do this and if breaking the compatibility is a concernt for 32bit
userspace running on 64bit kernels.
Below you will find more information on one of the alternative
solutions, the userspace compat.
*32bit kernel/ 32bit userspace*
*64bit kernel/ 64bit userspace*
My last series of binder patches, accepted into 3.12 revision of the
Linux kernel, audits the binder interface for 64bit. A kernel with these
changes applied can be used with a pure 64bit userspace (this does not
include support for 32bit applications coexisting with 64bit ones). The
userspace side needs trivial changes to libbinder.so and servicemanager,
that I will upstream ASAP to AOSP, and that work for 32/32 systems and
64/64 systems without modifying the existing 32bit interface ABI (most
of the changes are related to uint32_t != void *).
Author: Serban Constantinescu
Date: Thu Jul 4 10:54:48 2013 +0100
staging: android: binder: fix binder interface for 64bit compat layer
*64bit kernel/ 64bit coexisting with 32bit userspace
Please note that *none of the above solutions fix this yet*. To
understand why this is a special case please take a look at
how the binder driver works, seen from a high level perspective:
ServiceManager
App1 <-> App2
Thus, for two apps to exchange information between each other they will
have to communicate with the userspace governor, ServiceManager. All the
interaction between App1, App2 and ServiceManager is done through a
combination of libbinder.so (Userspace HAL) and the binder driver. Note
that there can only been one ServiceManager process, that is set during
the userspace boot process.
Now lets consider the case that Arve described earlier 32bit
Applications coexisting with 64bit ones. In this case all the commands
and interface used will have to be the same, the ones used by the 64bit
ServiceManager. For this the kernel entry point for 32bit compat will
have to be changed to:
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -3893,9 +3893,13 @@ static const struct file_operations binder_fops = {
.owner = THIS_MODULE,
.poll = binder_poll,
.unlocked_ioctl = binder_ioctl,
-#ifdef CONFIG_COMPAT
+#if defined(CONFIG_COMPAT) && !defined(COEXIST_32BIT_64BIT)
.compat_ioctl = compat_binder_ioctl,
#endif
+
+#if defined(COEXIST_32BIT_64BIT)
+ .compat_ioctl = binder_ioctl,
+#endif
.mmap = binder_mmap,
.open = binder_open,
.flush = binder_flush,
thus from the perspective of a kernel that works with a 64bit userspace
where 64bit applications coexist with 32bit applications the handling
will be the same for both 32bit and 64bit processes. This will also
involve modifying libbinder.so to use 64bit structures like:
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
+#if defined(COEXIST_32BIT_64BIT) && !defined(__LP64__)
+/*
+ * For running 32-bit processes on a 64-bit system, make the interaction with
the
+ * binder driver the same from this, when built for 32-bit, as for a 64-bit
process.
+ */
+struct coexist_binder_write_read {
+uint64_twrite_size; /* __LP64__ size_t: bytes to write */
+uint64_twrite_consumed; /* __LP64__ size_t: bytes consumed by driver */
+uint64_twrite_buffer; /* __LP64__ unsigned long */
+uint64_tread_size; /* __LP64__ size_t: bytes to read */
+uint64_tread_consumed; /* __LP64__ size_t: bytes consumed by driver */
+uint64_tread_buffer;/* __LP64__ unsigned long */
+};
"Good citizen" Android applications will go through these changes (since
they should only use the binder Java API), and so shouldn't encounter
any problem. Any 32bit applications that use the binder interface at a
native level will not work with this model (they should not do so!)ยท
This is exactly what the kernel compat "guarantees" *only* for 64/32bit
systems.
The
Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On 05/12/13 08:00, Dan Carpenter wrote:
On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
+static void bc_dead_binder_done(struct binder_proc *proc,
+ struct binder_thread *thread, void __user *cookie)
+{
+ struct binder_work *w;
+ struct binder_ref_death *death = NULL;
+
+ list_for_each_entry(w, &proc->delivered_death, entry) {
+ struct binder_ref_death *tmp_death = container_of(w, struct
binder_ref_death, work);
Put a blank line after the variable declaration block. Also break it up
into two lines instead of having the lines be a million characters long.
I will do that, I have followed the same conventions used by the code
allready there.
list_for_each_entry(w, &proc->delivered_death, entry) {
struct binder_ref_death *tmp_death;
tmp_death = container_of(w, struct binder_ref_death, work);
+ if (tmp_death->cookie == cookie) {
+ death = tmp_death;
+ return;
Should be a break here. This function wasn't tested.
There should be a break there! I have tested the driver with 32bit
Android tests, none of which seem to hit this.
Thanks for your review,
Serban C
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On 05/12/13 08:18, Dan Carpenter wrote:
On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
+static void bc_increfs_done(struct binder_proc *proc,
+ struct binder_thread *thread, uint32_t cmd,
+ void __user *node_ptr, void __user *cookie)
+{
+ struct binder_node *node;
+
+ node = binder_get_node(proc, node_ptr);
+ if (node == NULL) {
+ binder_user_error("%d:%d %s u%p no match\n",
+ proc->pid, thread->pid,
+ cmd == BC_INCREFS_DONE ?
+ "BC_INCREFS_DONE" :
+ "BC_ACQUIRE_DONE",
+ node_ptr);
+ return;
+ }
+ if (cookie != node->cookie) {
+ binder_user_error("%d:%d %s u%p node %d cookie mismatch %p !=
%p\n",
+ proc->pid, thread->pid,
+ cmd == BC_INCREFS_DONE ?
+ "BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
+ node_ptr, node->debug_id,
+ cookie, node->cookie);
+ return;
+ }
+ if (cmd == BC_ACQUIRE_DONE) {
+ if (node->pending_strong_ref == 0) {
+ binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no
pending acquire request\n",
+ proc->pid, thread->pid,
+ node->debug_id);
+ return;
+ }
+ node->pending_strong_ref = 0;
+ } else {
+ if (node->pending_weak_ref == 0) {
+ binder_user_error("%d:%d BC_INCREFS_DONE node %d has no
pending increfs request\n",
+ proc->pid, thread->pid,
+ node->debug_id);
+ return;
+ }
+ node->pending_weak_ref = 0;
+ }
+ binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
+ binder_debug(BINDER_DEBUG_USER_REFS,
+"%d:%d %s node %d ls %d lw %d\n",
+proc->pid, thread->pid,
+cmd == BC_INCREFS_DONE ?
+"BC_INCREFS_DONE" :
+"BC_ACQUIRE_DONE",
+node->debug_id, node->local_strong_refs,
+node->local_weak_refs);
+ return;
+}
This function is 52 lines long but at least 32 of those lines are
debug code.
Just extra of lines of code means you have increased the number of bugs
150%. But what I know is that from a static analysis perspective, debug
code has more defects per line then regular code it's worse than that.
Plus all the extra noise obscures the actual logic and makes the real
code annoying to look at so it's worse still.
If you want to move this stuff out of staging, then remove all the extra
crap so it doesn't look like barf.
This patch moves code around so that some of it can be shared with the
compat layer. I agree that due to the abundance of debug code it is, at
times, hard to have a clear idea of what is actually happening and adds
extra obscurities to the logic.
Thanks,
Serban C
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 2/9] staging: android: binder: Add binder_copy_to_user()
On 04/12/13 23:17, Greg KH wrote:
On Wed, Dec 04, 2013 at 06:09:34PM +, Serban Constantinescu wrote:
This patch adds binder_copy_to_user() to be used for copying binder
commands to user address space. This way we can abstract away the
copy_to_user() calls and add separate handling for the compat layer.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 39 --
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 233889c..6fbb340 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -2117,6 +2117,18 @@ static int binder_has_thread_work(struct binder_thread
*thread)
(thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
}
+static int binder_copy_to_user(uint32_t cmd, void *parcel,
+ void __user **ptr, size_t size)
+{
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, parcel, size))
+ return -EFAULT;
+ *ptr += size;
+ return 0;
+}
I know what you are trying to do here, but ick, why not just use the
structure involved in the copying out here? Or just copy the thing out
in one "chunk", not two different calls, which should make this go
faster, right?
Ick... agree. I do this split here for the compat handling added in the
next patches, where the cmd will have to be converted to a 32bit compat
cmd and the size, parcel ,passed here will change to a 32bit compat
size, parcel.
This patch makes more sense when looking at the following snippet:
**
+static int binder_copy_to_user(uint32_t cmd, void *parcel,
+ void __user **ptr, size_t size)
+{
+ if (!is_compat_task()) {
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, parcel, size))
+ return -EFAULT;
+ *ptr += size;
+ return 0;
+ }
+ switch (cmd) {
+ case BR_INCREFS:
+ case BR_ACQUIRE:
+ case BR_RELEASE:
+ case BR_DECREFS: {
+ struct binder_ptr_cookie *fp;
+ struct compat_binder_ptr_cookie tmp;
+
+ cmd = compat_change_size(cmd, sizeof(tmp));
Passing the cmd, and the structure to be copied to binder_copy_to_user()
allows me to add this extra handling here. Where, first, cmd is changed
to a compat cmd, and then copied to userspace - i.e:
I change
cmd = BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie)
to
cmd = COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie)
where
struct binder_ptr_cookie {
void* ptr;
void* cookie;
};
and
struct compat_binder_ptr_cookie {
compat_uptr_t ptr;
compat_uptr_t cookie;
};
Thus BR_INCREFS will be different between 32bit userspace and 64bit kernel.
+ BUG_ON((cmd != COMPAT_BR_INCREFS) &&
+ (cmd != COMPAT_BR_ACQUIRE) &&
+ (cmd != COMPAT_BR_RELEASE) &&
+ (cmd != COMPAT_BR_DECREFS));
+
+ fp = (struct binder_ptr_cookie *) parcel;
+ tmp.ptr = ptr_to_compat(fp->ptr);
+ tmp.cookie = ptr_to_compat(fp->cookie);
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, &tmp, sizeof(tmp)))
+ return -EFAULT;
+ *ptr += sizeof(tmp);
Also, since the size of the parcel will differ when copied to a compat
task (32bit userspace) I increment the buffer ptr here, rather then
doing this in binder_thread_read().
This way we can safely move to the next cmd, by incrementing the buffer
ptr accordingly whether 32 or 64bit structures are needed.
**
+
static int binder_thread_read(struct binder_proc *proc,
struct binder_thread *thread,
void __user *buffer, size_t size,
@@ -2263,15 +2275,12 @@ retry:
node->has_weak_ref = 0;
}
if (cmd != BR_NOOP) {
- if (put_user(cmd, (uint32_t __user *)ptr))
- return -EFAULT;
- ptr += sizeof(uint32_t);
- if (put_user(node->ptr, (void * __user *)ptr))
- return -EFAULT;
- ptr += sizeof(void *);
- if (put_user(node->cookie, (void * __user
*)ptr))
+ str
Re: [PATCH v1 3/9] staging: android: binder: Add cmd == CMD_NAME handling
On 05/12/13 08:40, Dan Carpenter wrote:
On Wed, Dec 04, 2013 at 06:09:35PM +, Serban Constantinescu wrote:
This patch modifies the functions that need to be passed the explicit
command to use a boolean flag. This way we can reuse the code for 64bit
compat commands.
I don't understand this at all. cmd seems like it should be 32 bits
on both arches.
Command is 32bit for both arches but does not have the same value. Here
is what happens, this patch make more sense when looking at the compat
layer.
The binder commands differ between 32bit userspace and 64bit kernels. E.g:
BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie)
COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie)
struct compat_binder_ptr_cookie {
compat_uptr_t ptr;
compat_uptr_t cookie;
};
struct binder_ptr_cookie {
void *ptr;
void *cookie;
};
(the IOR() macro encodes the size of the transaction - struct
binder_ptr_cookie).
This enables me to use the same handler for:
* native 64bit kernel/ 64bit userspace
case BC_INCREFS_DONE:
case BC_ACQUIRE_DONE: {
void __user *node_ptr;
void *cookie;
if (get_user(node_ptr, (void * __user *)ptr))
return -EFAULT;
ptr += sizeof(void *);
if (get_user(cookie, (void * __user *)ptr))
return -EFAULT;
ptr += sizeof(void *);
bc_increfs_done(proc, thread, cmd == BC_ACQUIRE_DONE,
node_ptr, cookie);
break;
}
* compat 64bit kernel/ 32bit userspace.
+ case COMPAT_BC_INCREFS_DONE:
+ case COMPAT_BC_ACQUIRE_DONE: {
+ compat_uptr_t node_ptr;
+ compat_uptr_t cookie;
+
+ if (get_user(node_ptr, (compat_uptr_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(compat_uptr_t);
+ if (get_user(cookie, (compat_uptr_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(compat_uptr_t);
+ bc_increfs_done(proc, thread, cmd == COMPAT_BC_ACQUIRE_DONE,
+ compat_ptr(node_ptr), compat_ptr(cookie));
+ break;
where the prototype for bc_increfs_done() has changed to:
static void bc_increfs_done(struct binder_proc *proc,
-struct binder_thread *thread, uint32_t cmd,
+struct binder_thread *thread, bool acquire,
Obsucre on its own, I should add more details in the commit message.
Thanks,
Serban C
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
