From bcf38880c65297da58194eb0c0ce8d6e2bab7d94 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Fri, 10 Jan 2014 13:56:27 +0000 Subject: [PATCH 01/11] ServiceManager: Make use of kernel exported structures This patch switches ServiceManager to use the structures exported in the kernel headers rather then redefining its own. struct binder_txn is replaced with struct binder_transaction_data and struct binder_object with struct flat_binder_object, both defined in the binder driver header . Change-Id: I3b3e97918173ea35a289e184774ae06193192da3 Signed-off-by: Serban Constantinescu --- cmds/servicemanager/binder.c | 86 +++++++++++++-------------- cmds/servicemanager/binder.h | 26 +------- cmds/servicemanager/service_manager.c | 4 +- 3 files changed, 46 insertions(+), 70 deletions(-) diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c index 1985756a6..43cb7d35c 100644 --- a/cmds/servicemanager/binder.c +++ b/cmds/servicemanager/binder.c @@ -17,7 +17,7 @@ #define LOG_TAG "Binder" #include -void bio_init_from_txn(struct binder_io *io, struct binder_txn *txn); +void bio_init_from_txn(struct binder_io *io, struct binder_transaction_data *txn); #if TRACE void hexdump(void *_data, unsigned len) @@ -38,21 +38,21 @@ void hexdump(void *_data, unsigned len) fprintf(stderr,"\n"); } -void binder_dump_txn(struct binder_txn *txn) +void binder_dump_txn(struct binder_transaction_data *txn) { - struct binder_object *obj; - unsigned *offs = txn->offs; - unsigned count = txn->offs_size / 4; + struct flat_binder_object *obj; + unsigned *offs = txn->data.ptr.offsets; + unsigned count = txn->offsets_size / 4; fprintf(stderr," target %p cookie %p code %08x flags %08x\n", - txn->target, txn->cookie, txn->code, txn->flags); - fprintf(stderr," pid %8d uid %8d data %8d offs %8d\n", - txn->sender_pid, txn->sender_euid, txn->data_size, txn->offs_size); - hexdump(txn->data, txn->data_size); + txn->target.ptr, txn->cookie, txn->code, txn->flags); + fprintf(stderr," pid %8d uid %8d data %zu offs %zu\n", + txn->sender_pid, txn->sender_euid, txn->data_size, txn->offsets_size); + hexdump(txn->data.ptr.buffer, txn->data_size); while (count--) { - obj = (void*) (((char*) txn->data) + *offs++); + obj = (struct flat_binder_object *) (((char*) txn->data.ptr.buffer) + *offs++); fprintf(stderr," - type %08x flags %08x ptr %p cookie %p\n", - obj->type, obj->flags, obj->pointer, obj->cookie); + obj->type, obj->flags, obj->binder, obj->cookie); } } @@ -166,27 +166,27 @@ void binder_send_reply(struct binder_state *bs, uint32_t cmd_free; void *buffer; uint32_t cmd_reply; - struct binder_txn txn; + struct binder_transaction_data txn; } __attribute__((packed)) data; data.cmd_free = BC_FREE_BUFFER; data.buffer = buffer_to_free; data.cmd_reply = BC_REPLY; - data.txn.target = 0; + data.txn.target.ptr = 0; data.txn.cookie = 0; data.txn.code = 0; if (status) { data.txn.flags = TF_STATUS_CODE; data.txn.data_size = sizeof(int); - data.txn.offs_size = 0; - data.txn.data = &status; - data.txn.offs = 0; + data.txn.offsets_size = 0; + data.txn.data.ptr.buffer = &status; + data.txn.data.ptr.offsets = 0; } else { data.txn.flags = 0; data.txn.data_size = reply->data - reply->data0; - data.txn.offs_size = ((char*) reply->offs) - ((char*) reply->offs0); - data.txn.data = reply->data0; - data.txn.offs = reply->offs0; + data.txn.offsets_size = ((char*) reply->offs) - ((char*) reply->offs0); + data.txn.data.ptr.buffer = reply->data0; + data.txn.data.ptr.offsets = reply->offs0; } binder_write(bs, &data, sizeof(data)); } @@ -217,8 +217,8 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, ptr += 2; break; case BR_TRANSACTION: { - struct binder_txn *txn = (void *) ptr; - if ((end - ptr) * sizeof(uint32_t) < sizeof(struct binder_txn)) { + struct binder_transaction_data *txn = (struct binder_transaction_data *) ptr; + if ((end - ptr) * sizeof(uint32_t) < sizeof(*txn)) { ALOGE("parse: txn too small!\n"); return -1; } @@ -232,14 +232,14 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, bio_init(&reply, rdata, sizeof(rdata), 4); bio_init_from_txn(&msg, txn); res = func(bs, txn, &msg, &reply); - binder_send_reply(bs, &reply, txn->data, res); + binder_send_reply(bs, &reply, txn->data.ptr.buffer, res); } ptr += sizeof(*txn) / sizeof(uint32_t); break; } case BR_REPLY: { - struct binder_txn *txn = (void*) ptr; - if ((end - ptr) * sizeof(uint32_t) < sizeof(struct binder_txn)) { + struct binder_transaction_data *txn = (struct binder_transaction_data *) ptr; + if ((end - ptr) * sizeof(uint32_t) < sizeof(*txn)) { ALOGE("parse: reply too small!\n"); return -1; } @@ -308,7 +308,7 @@ int binder_call(struct binder_state *bs, struct binder_write_read bwr; struct { uint32_t cmd; - struct binder_txn txn; + struct binder_transaction_data txn; } writebuf; unsigned readbuf[32]; @@ -318,13 +318,13 @@ int binder_call(struct binder_state *bs, } writebuf.cmd = BC_TRANSACTION; - writebuf.txn.target = target; + writebuf.txn.target.handle = target; writebuf.txn.code = code; writebuf.txn.flags = 0; writebuf.txn.data_size = msg->data - msg->data0; - writebuf.txn.offs_size = ((char*) msg->offs) - ((char*) msg->offs0); - writebuf.txn.data = msg->data0; - writebuf.txn.offs = msg->offs0; + writebuf.txn.offsets_size = ((char*) msg->offs) - ((char*) msg->offs0); + writebuf.txn.data.ptr.buffer = msg->data0; + writebuf.txn.data.ptr.offsets = msg->offs0; bwr.write_size = sizeof(writebuf); bwr.write_consumed = 0; @@ -391,12 +391,12 @@ void binder_loop(struct binder_state *bs, binder_handler func) } } -void bio_init_from_txn(struct binder_io *bio, struct binder_txn *txn) +void bio_init_from_txn(struct binder_io *bio, struct binder_transaction_data *txn) { - bio->data = bio->data0 = txn->data; - bio->offs = bio->offs0 = txn->offs; + bio->data = bio->data0 = txn->data.ptr.buffer; + bio->offs = bio->offs0 = txn->data.ptr.offsets; bio->data_avail = txn->data_size; - bio->offs_avail = txn->offs_size / 4; + bio->offs_avail = txn->offsets_size / 4; bio->flags = BIO_F_SHARED; } @@ -446,9 +446,9 @@ void binder_done(struct binder_state *bs, } } -static struct binder_object *bio_alloc_obj(struct binder_io *bio) +static struct flat_binder_object *bio_alloc_obj(struct binder_io *bio) { - struct binder_object *obj; + struct flat_binder_object *obj; obj = bio_alloc(bio, sizeof(*obj)); @@ -471,7 +471,7 @@ void bio_put_uint32(struct binder_io *bio, uint32_t n) void bio_put_obj(struct binder_io *bio, void *ptr) { - struct binder_object *obj; + struct flat_binder_object *obj; obj = bio_alloc_obj(bio); if (!obj) @@ -479,13 +479,13 @@ void bio_put_obj(struct binder_io *bio, void *ptr) obj->flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; obj->type = BINDER_TYPE_BINDER; - obj->pointer = ptr; + obj->binder = ptr; obj->cookie = 0; } void bio_put_ref(struct binder_io *bio, void *ptr) { - struct binder_object *obj; + struct flat_binder_object *obj; if (ptr) obj = bio_alloc_obj(bio); @@ -497,7 +497,7 @@ void bio_put_ref(struct binder_io *bio, void *ptr) obj->flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; obj->type = BINDER_TYPE_HANDLE; - obj->pointer = ptr; + obj->handle = ptr; obj->cookie = 0; } @@ -585,7 +585,7 @@ uint16_t *bio_get_string16(struct binder_io *bio, unsigned *sz) return bio_get(bio, (len + 1) * sizeof(uint16_t)); } -static struct binder_object *_bio_get_obj(struct binder_io *bio) +static struct flat_binder_object *_bio_get_obj(struct binder_io *bio) { unsigned n; unsigned off = bio->data - bio->data0; @@ -593,7 +593,7 @@ static struct binder_object *_bio_get_obj(struct binder_io *bio) /* TODO: be smarter about this? */ for (n = 0; n < bio->offs_avail; n++) { if (bio->offs[n] == off) - return bio_get(bio, sizeof(struct binder_object)); + return bio_get(bio, sizeof(struct flat_binder_object)); } bio->data_avail = 0; @@ -603,14 +603,14 @@ static struct binder_object *_bio_get_obj(struct binder_io *bio) void *bio_get_ref(struct binder_io *bio) { - struct binder_object *obj; + struct flat_binder_object *obj; obj = _bio_get_obj(bio); if (!obj) return 0; if (obj->type == BINDER_TYPE_HANDLE) - return obj->pointer; + return obj->handle; return 0; } diff --git a/cmds/servicemanager/binder.h b/cmds/servicemanager/binder.h index d8c51ef6b..07d74a0b2 100644 --- a/cmds/servicemanager/binder.h +++ b/cmds/servicemanager/binder.h @@ -9,30 +9,6 @@ struct binder_state; -struct binder_object -{ - uint32_t type; - uint32_t flags; - void *pointer; - void *cookie; -}; - -struct binder_txn -{ - void *target; - void *cookie; - uint32_t code; - uint32_t flags; - - uint32_t sender_pid; - uint32_t sender_euid; - - uint32_t data_size; - uint32_t offs_size; - void *data; - void *offs; -}; - struct binder_io { char *data; /* pointer to read/write from */ @@ -64,7 +40,7 @@ enum { }; typedef int (*binder_handler)(struct binder_state *bs, - struct binder_txn *txn, + struct binder_transaction_data *txn, struct binder_io *msg, struct binder_io *reply); diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index 3eaf1ebb1..3625e29f1 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -201,7 +201,7 @@ int do_add_service(struct binder_state *bs, } int svcmgr_handler(struct binder_state *bs, - struct binder_txn *txn, + struct binder_transaction_data *txn, struct binder_io *msg, struct binder_io *reply) { @@ -215,7 +215,7 @@ int svcmgr_handler(struct binder_state *bs, // ALOGI("target=%p code=%d pid=%d uid=%d\n", // txn->target, txn->code, txn->sender_pid, txn->sender_euid); - if (txn->target != svcmgr_handle) + if (txn->target.handle != svcmgr_handle) return -1; // Equivalent to Parcel::enforceInterface(), reading the RPC From dc832dc5513f0767c153f90a57356c3466f45dd4 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 30 Jan 2014 14:58:32 +0000 Subject: [PATCH 02/11] ServiceManager: Fix Android.mk This patch fixes Android.mk and enables building bctest as an optional module without any extra hacks. Change-Id: Icaf8bf9452776db2ea4a2ba75f3abf05b4e2cdab Signed-off-by: Serban Constantinescu --- cmds/servicemanager/Android.mk | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cmds/servicemanager/Android.mk b/cmds/servicemanager/Android.mk index 8840867a9..138be5232 100644 --- a/cmds/servicemanager/Android.mk +++ b/cmds/servicemanager/Android.mk @@ -1,12 +1,19 @@ LOCAL_PATH:= $(call my-dir) -#include $(CLEAR_VARS) -#LOCAL_SRC_FILES := bctest.c binder.c -#LOCAL_MODULE := bctest -#include $(BUILD_EXECUTABLE) +svc_c_flags = \ + -Wall -Wextra \ + +include $(CLEAR_VARS) +LOCAL_SHARED_LIBRARIES := liblog +LOCAL_SRC_FILES := bctest.c binder.c +LOCAL_CFLAGS += $(svc_c_flags) +LOCAL_MODULE := bctest +LOCAL_MODULE_TAGS := optional +include $(BUILD_EXECUTABLE) include $(CLEAR_VARS) LOCAL_SHARED_LIBRARIES := liblog LOCAL_SRC_FILES := service_manager.c binder.c +LOCAL_CFLAGS += $(svc_c_flags) LOCAL_MODULE := servicemanager include $(BUILD_EXECUTABLE) From a44542ca74b7da5b44ba30c205c3244805bb0600 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 30 Jan 2014 15:16:45 +0000 Subject: [PATCH 03/11] ServiceManager: Add extra error handling This patch extends the error handling. It also adds a check for a matching binder version - kernel/userspace. Change-Id: I43a262934b38c5711536aaa42754fed1ef04b39e Signed-off-by: Serban Constantinescu --- cmds/servicemanager/bctest.c | 4 ++++ cmds/servicemanager/binder.c | 9 +++++++-- cmds/servicemanager/service_manager.c | 4 ++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cmds/servicemanager/bctest.c b/cmds/servicemanager/bctest.c index ff5acedfa..ee49679fd 100644 --- a/cmds/servicemanager/bctest.c +++ b/cmds/servicemanager/bctest.c @@ -62,6 +62,10 @@ int main(int argc, char **argv) void *svcmgr = BINDER_SERVICE_MANAGER; bs = binder_open(128*1024); + if (!bs) { + fprintf(stderr, "failed to open binder driver\n"); + return -1; + } argc--; argv++; diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c index 43cb7d35c..7fdd841ed 100644 --- a/cmds/servicemanager/binder.c +++ b/cmds/servicemanager/binder.c @@ -94,6 +94,7 @@ struct binder_state struct binder_state *binder_open(unsigned mapsize) { struct binder_state *bs; + struct binder_version vers; bs = malloc(sizeof(*bs)); if (!bs) { @@ -108,6 +109,12 @@ struct binder_state *binder_open(unsigned mapsize) goto fail_open; } + if ((ioctl(bs->fd, BINDER_VERSION, &vers) == -1) || + (vers.protocol_version != BINDER_CURRENT_PROTOCOL_VERSION)) { + fprintf(stderr, "binder: driver version differs from user space\n"); + goto fail_open; + } + bs->mapsize = mapsize; bs->mapped = mmap(NULL, mapsize, PROT_READ, MAP_PRIVATE, bs->fd, 0); if (bs->mapped == MAP_FAILED) { @@ -116,8 +123,6 @@ struct binder_state *binder_open(unsigned mapsize) goto fail_map; } - /* TODO: check version */ - return bs; fail_map: diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index 3625e29f1..c8b219baf 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -275,6 +275,10 @@ int main(int argc, char **argv) void *svcmgr = BINDER_SERVICE_MANAGER; bs = binder_open(128*1024); + if (!bs) { + ALOGE("failed to open binder driver\n"); + return -1; + } if (binder_become_context_manager(bs)) { ALOGE("cannot become context manager (%s)\n", strerror(errno)); From 9b738bb4110926b85da65d36b2e6f1a50199ec4c Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Fri, 10 Jan 2014 15:48:29 +0000 Subject: [PATCH 04/11] ServiceManager: Generic Fixes This patch fixes some of the ServiceManager issues. The following patches of the series add fixes to the ABI. Change-Id: Ib479234c8704e12592f1b149ddec67881bc50230 Signed-off-by: Serban Constantinescu --- cmds/servicemanager/binder.c | 62 ++++++++++++++------------- cmds/servicemanager/binder.h | 19 ++++---- cmds/servicemanager/service_manager.c | 49 ++++++++++----------- 3 files changed, 66 insertions(+), 64 deletions(-) diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c index 7fdd841ed..d9539811f 100644 --- a/cmds/servicemanager/binder.c +++ b/cmds/servicemanager/binder.c @@ -20,14 +20,14 @@ void bio_init_from_txn(struct binder_io *io, struct binder_transaction_data *txn); #if TRACE -void hexdump(void *_data, unsigned len) +void hexdump(void *_data, size_t len) { unsigned char *data = _data; - unsigned count; + size_t count; for (count = 0; count < len; count++) { if ((count & 15) == 0) - fprintf(stderr,"%04x:", count); + fprintf(stderr,"%04zu:", count); fprintf(stderr," %02x %c", *data, (*data < 32) || (*data > 126) ? '.' : *data); data++; @@ -41,8 +41,8 @@ void hexdump(void *_data, unsigned len) void binder_dump_txn(struct binder_transaction_data *txn) { struct flat_binder_object *obj; - unsigned *offs = txn->data.ptr.offsets; - unsigned count = txn->offsets_size / 4; + size_t *offs = txn->data.ptr.offsets; + size_t count = txn->offsets_size / sizeof(size_t); fprintf(stderr," target %p cookie %p code %08x flags %08x\n", txn->target.ptr, txn->cookie, txn->code, txn->flags); @@ -88,10 +88,10 @@ struct binder_state { int fd; void *mapped; - unsigned mapsize; + size_t mapsize; }; -struct binder_state *binder_open(unsigned mapsize) +struct binder_state *binder_open(size_t mapsize) { struct binder_state *bs; struct binder_version vers; @@ -99,7 +99,7 @@ struct binder_state *binder_open(unsigned mapsize) bs = malloc(sizeof(*bs)); if (!bs) { errno = ENOMEM; - return 0; + return NULL; } bs->fd = open("/dev/binder", O_RDWR); @@ -129,7 +129,7 @@ fail_map: close(bs->fd); fail_open: free(bs); - return 0; + return NULL; } void binder_close(struct binder_state *bs) @@ -144,7 +144,7 @@ int binder_become_context_manager(struct binder_state *bs) return ioctl(bs->fd, BINDER_SET_CONTEXT_MGR, 0); } -int binder_write(struct binder_state *bs, void *data, unsigned len) +int binder_write(struct binder_state *bs, void *data, size_t len) { struct binder_write_read bwr; int res; @@ -164,7 +164,7 @@ int binder_write(struct binder_state *bs, void *data, unsigned len) void binder_send_reply(struct binder_state *bs, struct binder_io *reply, - void *buffer_to_free, + const void *buffer_to_free, int status) { struct { @@ -334,7 +334,7 @@ int binder_call(struct binder_state *bs, bwr.write_size = sizeof(writebuf); bwr.write_consumed = 0; bwr.write_buffer = (unsigned) &writebuf; - + hexdump(msg->data0, msg->data - msg->data0); for (;;) { bwr.read_size = sizeof(readbuf); @@ -368,7 +368,7 @@ void binder_loop(struct binder_state *bs, binder_handler func) bwr.write_size = 0; bwr.write_consumed = 0; bwr.write_buffer = 0; - + readbuf[0] = BC_ENTER_LOOPER; binder_write(bs, readbuf, sizeof(unsigned)); @@ -406,9 +406,9 @@ void bio_init_from_txn(struct binder_io *bio, struct binder_transaction_data *tx } void bio_init(struct binder_io *bio, void *data, - uint32_t maxdata, uint32_t maxoffs) + size_t maxdata, size_t maxoffs) { - uint32_t n = maxoffs * sizeof(uint32_t); + size_t n = maxoffs * sizeof(size_t); if (n > maxdata) { bio->flags = BIO_F_OVERFLOW; @@ -429,7 +429,7 @@ static void *bio_alloc(struct binder_io *bio, uint32_t size) size = (size + 3) & (~3); if (size > bio->data_avail) { bio->flags |= BIO_F_OVERFLOW; - return 0; + return NULL; } else { void *ptr = bio->data; bio->data += size; @@ -456,7 +456,7 @@ static struct flat_binder_object *bio_alloc_obj(struct binder_io *bio) struct flat_binder_object *obj; obj = bio_alloc(bio, sizeof(*obj)); - + if (obj && bio->offs_avail) { bio->offs_avail--; *bio->offs++ = ((char*) obj) - ((char*) bio->data0); @@ -464,7 +464,7 @@ static struct flat_binder_object *bio_alloc_obj(struct binder_io *bio) } bio->flags |= BIO_F_OVERFLOW; - return 0; + return NULL; } void bio_put_uint32(struct binder_io *bio, uint32_t n) @@ -508,7 +508,7 @@ void bio_put_ref(struct binder_io *bio, void *ptr) void bio_put_string16(struct binder_io *bio, const uint16_t *str) { - uint32_t len; + size_t len; uint16_t *ptr; if (!str) { @@ -524,7 +524,8 @@ void bio_put_string16(struct binder_io *bio, const uint16_t *str) return; } - bio_put_uint32(bio, len); + /* Note: The payload will carry 32bit size instead of size_t */ + bio_put_uint32(bio, (uint32_t) len); len = (len + 1) * sizeof(uint16_t); ptr = bio_alloc(bio, len); if (ptr) @@ -534,7 +535,7 @@ void bio_put_string16(struct binder_io *bio, const uint16_t *str) void bio_put_string16_x(struct binder_io *bio, const char *_str) { unsigned char *str = (unsigned char*) _str; - uint32_t len; + size_t len; uint16_t *ptr; if (!str) { @@ -549,6 +550,7 @@ void bio_put_string16_x(struct binder_io *bio, const char *_str) return; } + /* Note: The payload will carry 32bit size instead of size_t */ bio_put_uint32(bio, len); ptr = bio_alloc(bio, (len + 1) * sizeof(uint16_t)); if (!ptr) @@ -559,14 +561,14 @@ void bio_put_string16_x(struct binder_io *bio, const char *_str) *ptr++ = 0; } -static void *bio_get(struct binder_io *bio, uint32_t size) +static void *bio_get(struct binder_io *bio, size_t size) { size = (size + 3) & (~3); if (bio->data_avail < size){ bio->data_avail = 0; bio->flags |= BIO_F_OVERFLOW; - return 0; + return NULL; } else { void *ptr = bio->data; bio->data += size; @@ -581,10 +583,12 @@ uint32_t bio_get_uint32(struct binder_io *bio) return ptr ? *ptr : 0; } -uint16_t *bio_get_string16(struct binder_io *bio, unsigned *sz) +uint16_t *bio_get_string16(struct binder_io *bio, size_t *sz) { - unsigned len; - len = bio_get_uint32(bio); + size_t len; + + /* Note: The payload will carry 32bit size instead of size_t */ + len = (size_t) bio_get_uint32(bio); if (sz) *sz = len; return bio_get(bio, (len + 1) * sizeof(uint16_t)); @@ -592,8 +596,8 @@ uint16_t *bio_get_string16(struct binder_io *bio, unsigned *sz) static struct flat_binder_object *_bio_get_obj(struct binder_io *bio) { - unsigned n; - unsigned off = bio->data - bio->data0; + size_t n; + size_t off = bio->data - bio->data0; /* TODO: be smarter about this? */ for (n = 0; n < bio->offs_avail; n++) { @@ -603,7 +607,7 @@ static struct flat_binder_object *_bio_get_obj(struct binder_io *bio) bio->data_avail = 0; bio->flags |= BIO_F_OVERFLOW; - return 0; + return NULL; } void *bio_get_ref(struct binder_io *bio) diff --git a/cmds/servicemanager/binder.h b/cmds/servicemanager/binder.h index 07d74a0b2..4f1b4a5f2 100644 --- a/cmds/servicemanager/binder.h +++ b/cmds/servicemanager/binder.h @@ -12,12 +12,12 @@ struct binder_state; struct binder_io { char *data; /* pointer to read/write from */ - uint32_t *offs; /* array of offsets */ - uint32_t data_avail; /* bytes available in data buffer */ - uint32_t offs_avail; /* entries available in offsets array */ + size_t *offs; /* array of offsets */ + size_t data_avail; /* bytes available in data buffer */ + size_t offs_avail; /* entries available in offsets array */ char *data0; /* start of data buffer */ - uint32_t *offs0; /* start of offsets buffer */ + size_t *offs0; /* start of offsets buffer */ uint32_t flags; uint32_t unused; }; @@ -25,7 +25,7 @@ struct binder_io struct binder_death { void (*func)(struct binder_state *bs, void *ptr); void *ptr; -}; +}; /* the one magic object */ #define BINDER_SERVICE_MANAGER ((void*) 0) @@ -44,7 +44,7 @@ typedef int (*binder_handler)(struct binder_state *bs, struct binder_io *msg, struct binder_io *reply); -struct binder_state *binder_open(unsigned mapsize); +struct binder_state *binder_open(size_t mapsize); void binder_close(struct binder_state *bs); /* initiate a blocking binder call @@ -77,9 +77,7 @@ int binder_become_context_manager(struct binder_state *bs); * offset entries to reserve from the buffer */ void bio_init(struct binder_io *bio, void *data, - uint32_t maxdata, uint32_t maxobjects); - -void bio_destroy(struct binder_io *bio); + size_t maxdata, size_t maxobjects); void bio_put_obj(struct binder_io *bio, void *ptr); void bio_put_ref(struct binder_io *bio, void *ptr); @@ -88,8 +86,7 @@ void bio_put_string16(struct binder_io *bio, const uint16_t *str); void bio_put_string16_x(struct binder_io *bio, const char *_str); uint32_t bio_get_uint32(struct binder_io *bio); -uint16_t *bio_get_string16(struct binder_io *bio, uint32_t *sz); -void *bio_get_obj(struct binder_io *bio); +uint16_t *bio_get_string16(struct binder_io *bio, size_t *sz); void *bio_get_ref(struct binder_io *bio); #endif diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index c8b219baf..1e32133b1 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -24,7 +24,7 @@ * uid can register media.*, etc) */ static struct { - unsigned uid; + uid_t uid; const char *name; } allowed[] = { { AID_MEDIA, "media.audio_flinger" }, @@ -52,7 +52,7 @@ static struct { void *svcmgr_handle; -const char *str8(uint16_t *x) +const char *str8(const uint16_t *x) { static char buf[128]; unsigned max = 127; @@ -67,7 +67,7 @@ const char *str8(uint16_t *x) return buf; } -int str16eq(uint16_t *a, const char *b) +int str16eq(const uint16_t *a, const char *b) { while (*a && *b) if (*a++ != *b++) return 0; @@ -76,10 +76,10 @@ int str16eq(uint16_t *a, const char *b) return 1; } -int svc_can_register(unsigned uid, uint16_t *name) +int svc_can_register(uid_t uid, const uint16_t *name) { - unsigned n; - + size_t n; + if ((uid == 0) || (uid == AID_SYSTEM)) return 1; @@ -90,19 +90,19 @@ int svc_can_register(unsigned uid, uint16_t *name) return 0; } -struct svcinfo +struct svcinfo { struct svcinfo *next; void *ptr; struct binder_death death; int allow_isolated; - unsigned len; + size_t len; uint16_t name[0]; }; -struct svcinfo *svclist = 0; +struct svcinfo *svclist = NULL; -struct svcinfo *find_svc(uint16_t *s16, unsigned len) +struct svcinfo *find_svc(const uint16_t *s16, size_t len) { struct svcinfo *si; @@ -112,26 +112,27 @@ struct svcinfo *find_svc(uint16_t *s16, unsigned len) return si; } } - return 0; + return NULL; } void svcinfo_death(struct binder_state *bs, void *ptr) { - struct svcinfo *si = ptr; + struct svcinfo *si = (struct svcinfo* ) ptr; + ALOGI("service '%s' died\n", str8(si->name)); if (si->ptr) { binder_release(bs, si->ptr); si->ptr = 0; - } + } } -uint16_t svcmgr_id[] = { +uint16_t svcmgr_id[] = { 'a','n','d','r','o','i','d','.','o','s','.', - 'I','S','e','r','v','i','c','e','M','a','n','a','g','e','r' + 'I','S','e','r','v','i','c','e','M','a','n','a','g','e','r' }; - -void *do_find_service(struct binder_state *bs, uint16_t *s, unsigned len, unsigned uid) + +void *do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, uid_t uid) { struct svcinfo *si; si = find_svc(s, len); @@ -141,7 +142,7 @@ void *do_find_service(struct binder_state *bs, uint16_t *s, unsigned len, unsign if (!si->allow_isolated) { // If this service doesn't allow access from isolated processes, // then check the uid to see if it is isolated. - unsigned appid = uid % AID_USER; + uid_t appid = uid % AID_USER; if (appid >= AID_ISOLATED_START && appid <= AID_ISOLATED_END) { return 0; } @@ -153,8 +154,8 @@ void *do_find_service(struct binder_state *bs, uint16_t *s, unsigned len, unsign } int do_add_service(struct binder_state *bs, - uint16_t *s, unsigned len, - void *ptr, unsigned uid, int allow_isolated) + const uint16_t *s, size_t len, + void *ptr, uid_t uid, int allow_isolated) { struct svcinfo *si; //ALOGI("add_service('%s',%p,%s) uid=%d\n", str8(s), ptr, @@ -188,7 +189,7 @@ int do_add_service(struct binder_state *bs, si->len = len; memcpy(si->name, s, (len + 1) * sizeof(uint16_t)); si->name[len] = '\0'; - si->death.func = svcinfo_death; + si->death.func = (void*) svcinfo_death; si->death.ptr = si; si->allow_isolated = allow_isolated; si->next = svclist; @@ -207,7 +208,7 @@ int svcmgr_handler(struct binder_state *bs, { struct svcinfo *si; uint16_t *s; - unsigned len; + size_t len; void *ptr; uint32_t strict_policy; int allow_isolated; @@ -272,7 +273,6 @@ int svcmgr_handler(struct binder_state *bs, int main(int argc, char **argv) { struct binder_state *bs; - void *svcmgr = BINDER_SERVICE_MANAGER; bs = binder_open(128*1024); if (!bs) { @@ -285,7 +285,8 @@ int main(int argc, char **argv) return -1; } - svcmgr_handle = svcmgr; + svcmgr_handle = BINDER_SERVICE_MANAGER; binder_loop(bs, svcmgr_handler); + return 0; } From 5fb1b8836aa5cf0f38b49bc7bfb8343b84fdf9bf Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 30 Jan 2014 14:07:34 +0000 Subject: [PATCH 05/11] ServiceManager: Store handles in uint32_t instead of void * This patch corrects the types used for storing handles. Change-Id: If9c10782345f1de9e12b4b3fd6be9e02e6b568cd Signed-off-by: Serban Constantinescu --- cmds/servicemanager/bctest.c | 30 +++++++------- cmds/servicemanager/binder.c | 22 +++++----- cmds/servicemanager/binder.h | 16 ++++---- cmds/servicemanager/service_manager.c | 58 +++++++++++++-------------- 4 files changed, 63 insertions(+), 63 deletions(-) diff --git a/cmds/servicemanager/bctest.c b/cmds/servicemanager/bctest.c index ee49679fd..3bd18a710 100644 --- a/cmds/servicemanager/bctest.c +++ b/cmds/servicemanager/bctest.c @@ -7,9 +7,9 @@ #include "binder.h" -void *svcmgr_lookup(struct binder_state *bs, void *target, const char *name) +uint32_t svcmgr_lookup(struct binder_state *bs, uint32_t target, const char *name) { - void *ptr; + uint32_t handle; unsigned iodata[512/4]; struct binder_io msg, reply; @@ -21,17 +21,17 @@ void *svcmgr_lookup(struct binder_state *bs, void *target, const char *name) if (binder_call(bs, &msg, &reply, target, SVC_MGR_CHECK_SERVICE)) return 0; - ptr = bio_get_ref(&reply); + handle = bio_get_ref(&reply); - if (ptr) - binder_acquire(bs, ptr); + if (handle) + binder_acquire(bs, handle); binder_done(bs, &msg, &reply); - return ptr; + return handle; } -int svcmgr_publish(struct binder_state *bs, void *target, const char *name, void *ptr) +int svcmgr_publish(struct binder_state *bs, uint32_t target, const char *name, void *ptr) { unsigned status; unsigned iodata[512/4]; @@ -59,7 +59,8 @@ int main(int argc, char **argv) { int fd; struct binder_state *bs; - void *svcmgr = BINDER_SERVICE_MANAGER; + uint32_t svcmgr = BINDER_SERVICE_MANAGER; + uint32_t handle; bs = binder_open(128*1024); if (!bs) { @@ -71,21 +72,20 @@ int main(int argc, char **argv) argv++; while (argc > 0) { if (!strcmp(argv[0],"alt")) { - void *ptr = svcmgr_lookup(bs, svcmgr, "alt_svc_mgr"); - if (!ptr) { + handle = svcmgr_lookup(bs, svcmgr, "alt_svc_mgr"); + if (!handle) { fprintf(stderr,"cannot find alt_svc_mgr\n"); return -1; } - svcmgr = ptr; - fprintf(stderr,"svcmgr is via %p\n", ptr); + svcmgr = handle; + fprintf(stderr,"svcmgr is via %x\n", handle); } else if (!strcmp(argv[0],"lookup")) { - void *ptr; if (argc < 2) { fprintf(stderr,"argument required\n"); return -1; } - ptr = svcmgr_lookup(bs, svcmgr, argv[1]); - fprintf(stderr,"lookup(%s) = %p\n", argv[1], ptr); + handle = svcmgr_lookup(bs, svcmgr, argv[1]); + fprintf(stderr,"lookup(%s) = %x\n", argv[1], handle); argc--; argv++; } else if (!strcmp(argv[0],"publish")) { diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c index d9539811f..5f206af25 100644 --- a/cmds/servicemanager/binder.c +++ b/cmds/servicemanager/binder.c @@ -279,27 +279,27 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, return r; } -void binder_acquire(struct binder_state *bs, void *ptr) +void binder_acquire(struct binder_state *bs, uint32_t target) { uint32_t cmd[2]; cmd[0] = BC_ACQUIRE; - cmd[1] = (uint32_t) ptr; + cmd[1] = target; binder_write(bs, cmd, sizeof(cmd)); } -void binder_release(struct binder_state *bs, void *ptr) +void binder_release(struct binder_state *bs, uint32_t target) { uint32_t cmd[2]; cmd[0] = BC_RELEASE; - cmd[1] = (uint32_t) ptr; + cmd[1] = target; binder_write(bs, cmd, sizeof(cmd)); } -void binder_link_to_death(struct binder_state *bs, void *ptr, struct binder_death *death) +void binder_link_to_death(struct binder_state *bs, uint32_t target, struct binder_death *death) { uint32_t cmd[3]; cmd[0] = BC_REQUEST_DEATH_NOTIFICATION; - cmd[1] = (uint32_t) ptr; + cmd[1] = (uint32_t) target; cmd[2] = (uint32_t) death; binder_write(bs, cmd, sizeof(cmd)); } @@ -307,7 +307,7 @@ void binder_link_to_death(struct binder_state *bs, void *ptr, struct binder_deat int binder_call(struct binder_state *bs, struct binder_io *msg, struct binder_io *reply, - void *target, uint32_t code) + uint32_t target, uint32_t code) { int res; struct binder_write_read bwr; @@ -488,11 +488,11 @@ void bio_put_obj(struct binder_io *bio, void *ptr) obj->cookie = 0; } -void bio_put_ref(struct binder_io *bio, void *ptr) +void bio_put_ref(struct binder_io *bio, uint32_t handle) { struct flat_binder_object *obj; - if (ptr) + if (handle) obj = bio_alloc_obj(bio); else obj = bio_alloc(bio, sizeof(*obj)); @@ -502,7 +502,7 @@ void bio_put_ref(struct binder_io *bio, void *ptr) obj->flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; obj->type = BINDER_TYPE_HANDLE; - obj->handle = ptr; + obj->handle = handle; obj->cookie = 0; } @@ -610,7 +610,7 @@ static struct flat_binder_object *_bio_get_obj(struct binder_io *bio) return NULL; } -void *bio_get_ref(struct binder_io *bio) +uint32_t bio_get_ref(struct binder_io *bio) { struct flat_binder_object *obj; diff --git a/cmds/servicemanager/binder.h b/cmds/servicemanager/binder.h index 4f1b4a5f2..24b77f662 100644 --- a/cmds/servicemanager/binder.h +++ b/cmds/servicemanager/binder.h @@ -27,8 +27,8 @@ struct binder_death { void *ptr; }; -/* the one magic object */ -#define BINDER_SERVICE_MANAGER ((void*) 0) +/* the one magic handle */ +#define BINDER_SERVICE_MANAGER 0U #define SVC_MGR_NAME "android.os.IServiceManager" @@ -52,7 +52,7 @@ void binder_close(struct binder_state *bs); */ int binder_call(struct binder_state *bs, struct binder_io *msg, struct binder_io *reply, - void *target, uint32_t code); + uint32_t target, uint32_t code); /* release any state associate with the binder_io * - call once any necessary data has been extracted from the @@ -63,10 +63,10 @@ void binder_done(struct binder_state *bs, struct binder_io *msg, struct binder_io *reply); /* manipulate strong references */ -void binder_acquire(struct binder_state *bs, void *ptr); -void binder_release(struct binder_state *bs, void *ptr); +void binder_acquire(struct binder_state *bs, uint32_t target); +void binder_release(struct binder_state *bs, uint32_t target); -void binder_link_to_death(struct binder_state *bs, void *ptr, struct binder_death *death); +void binder_link_to_death(struct binder_state *bs, uint32_t target, struct binder_death *death); void binder_loop(struct binder_state *bs, binder_handler func); @@ -80,13 +80,13 @@ void bio_init(struct binder_io *bio, void *data, size_t maxdata, size_t maxobjects); void bio_put_obj(struct binder_io *bio, void *ptr); -void bio_put_ref(struct binder_io *bio, void *ptr); +void bio_put_ref(struct binder_io *bio, uint32_t handle); void bio_put_uint32(struct binder_io *bio, uint32_t n); void bio_put_string16(struct binder_io *bio, const uint16_t *str); void bio_put_string16_x(struct binder_io *bio, const char *_str); uint32_t bio_get_uint32(struct binder_io *bio); uint16_t *bio_get_string16(struct binder_io *bio, size_t *sz); -void *bio_get_ref(struct binder_io *bio); +uint32_t bio_get_ref(struct binder_io *bio); #endif diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index 1e32133b1..c6735ee98 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -50,7 +50,7 @@ static struct { { AID_KEYSTORE, "android.security.keystore" }, }; -void *svcmgr_handle; +uint32_t svcmgr_handle; const char *str8(const uint16_t *x) { @@ -93,7 +93,7 @@ int svc_can_register(uid_t uid, const uint16_t *name) struct svcinfo { struct svcinfo *next; - void *ptr; + uint32_t handle; struct binder_death death; int allow_isolated; size_t len; @@ -120,9 +120,9 @@ void svcinfo_death(struct binder_state *bs, void *ptr) struct svcinfo *si = (struct svcinfo* ) ptr; ALOGI("service '%s' died\n", str8(si->name)); - if (si->ptr) { - binder_release(bs, si->ptr); - si->ptr = 0; + if (si->handle) { + binder_release(bs, si->handle); + si->handle = 0; } } @@ -132,13 +132,13 @@ uint16_t svcmgr_id[] = { }; -void *do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, uid_t uid) +uint32_t do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, uid_t uid) { struct svcinfo *si; si = find_svc(s, len); -// ALOGI("check_service('%s') ptr = %p\n", str8(s), si ? si->ptr : 0); - if (si && si->ptr) { + //ALOGI("check_service('%s') handle = %x\n", str8(s), si ? si->handle : 0); + if (si && si->handle) { if (!si->allow_isolated) { // If this service doesn't allow access from isolated processes, // then check the uid to see if it is isolated. @@ -147,7 +147,7 @@ void *do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, ui return 0; } } - return si->ptr; + return si->handle; } else { return 0; } @@ -155,37 +155,37 @@ void *do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, ui int do_add_service(struct binder_state *bs, const uint16_t *s, size_t len, - void *ptr, uid_t uid, int allow_isolated) + uint32_t handle, uid_t uid, int allow_isolated) { struct svcinfo *si; - //ALOGI("add_service('%s',%p,%s) uid=%d\n", str8(s), ptr, + //ALOGI("add_service('%s',%x,%s) uid=%d\n", str8(s), handle, // allow_isolated ? "allow_isolated" : "!allow_isolated", uid); - if (!ptr || (len == 0) || (len > 127)) + if (!handle || (len == 0) || (len > 127)) return -1; if (!svc_can_register(uid, s)) { - ALOGE("add_service('%s',%p) uid=%d - PERMISSION DENIED\n", - str8(s), ptr, uid); + ALOGE("add_service('%s',%x) uid=%d - PERMISSION DENIED\n", + str8(s), handle, uid); return -1; } si = find_svc(s, len); if (si) { - if (si->ptr) { - ALOGE("add_service('%s',%p) uid=%d - ALREADY REGISTERED, OVERRIDE\n", - str8(s), ptr, uid); + if (si->handle) { + ALOGE("add_service('%s',%x) uid=%d - ALREADY REGISTERED, OVERRIDE\n", + str8(s), handle, uid); svcinfo_death(bs, si); } - si->ptr = ptr; + si->handle = handle; } else { si = malloc(sizeof(*si) + (len + 1) * sizeof(uint16_t)); if (!si) { - ALOGE("add_service('%s',%p) uid=%d - OUT OF MEMORY\n", - str8(s), ptr, uid); + ALOGE("add_service('%s',%x) uid=%d - OUT OF MEMORY\n", + str8(s), handle, uid); return -1; } - si->ptr = ptr; + si->handle = handle; si->len = len; memcpy(si->name, s, (len + 1) * sizeof(uint16_t)); si->name[len] = '\0'; @@ -196,8 +196,8 @@ int do_add_service(struct binder_state *bs, svclist = si; } - binder_acquire(bs, ptr); - binder_link_to_death(bs, ptr, &si->death); + binder_acquire(bs, handle); + binder_link_to_death(bs, handle, &si->death); return 0; } @@ -209,7 +209,7 @@ int svcmgr_handler(struct binder_state *bs, struct svcinfo *si; uint16_t *s; size_t len; - void *ptr; + uint32_t handle; uint32_t strict_policy; int allow_isolated; @@ -235,17 +235,17 @@ int svcmgr_handler(struct binder_state *bs, case SVC_MGR_GET_SERVICE: case SVC_MGR_CHECK_SERVICE: s = bio_get_string16(msg, &len); - ptr = do_find_service(bs, s, len, txn->sender_euid); - if (!ptr) + handle = do_find_service(bs, s, len, txn->sender_euid); + if (!handle) break; - bio_put_ref(reply, ptr); + bio_put_ref(reply, handle); return 0; case SVC_MGR_ADD_SERVICE: s = bio_get_string16(msg, &len); - ptr = bio_get_ref(msg); + handle = bio_get_ref(msg); allow_isolated = bio_get_uint32(msg) ? 1 : 0; - if (do_add_service(bs, s, len, ptr, txn->sender_euid, allow_isolated)) + if (do_add_service(bs, s, len, handle, txn->sender_euid, allow_isolated)) return -1; break; From 3a345f0df5f62d77e875a289e9aee89f0d1b526e Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Thu, 19 Dec 2013 09:11:41 +0000 Subject: [PATCH 06/11] ServiceManager: Fix the binder interface This patch adds support for binder transactions on 64bit systems without breaking the existing 32bit ABI. It has been tested on the Android emulator and ARMv8 Model. Most of the changes in this patch just follow the binder ABI. Change-Id: I8c37b847ea65008d56554d34d4696fe3d22f7533 Signed-off-by: Serban Constantinescu --- cmds/servicemanager/bctest.c | 2 +- cmds/servicemanager/binder.c | 80 +++++++++++++++------------ cmds/servicemanager/service_manager.c | 9 +-- 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/cmds/servicemanager/bctest.c b/cmds/servicemanager/bctest.c index 3bd18a710..e02b45d10 100644 --- a/cmds/servicemanager/bctest.c +++ b/cmds/servicemanager/bctest.c @@ -33,7 +33,7 @@ uint32_t svcmgr_lookup(struct binder_state *bs, uint32_t target, const char *nam int svcmgr_publish(struct binder_state *bs, uint32_t target, const char *name, void *ptr) { - unsigned status; + int status; unsigned iodata[512/4]; struct binder_io msg, reply; diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c index 5f206af25..7f8d0e0ed 100644 --- a/cmds/servicemanager/binder.c +++ b/cmds/servicemanager/binder.c @@ -148,9 +148,10 @@ int binder_write(struct binder_state *bs, void *data, size_t len) { struct binder_write_read bwr; int res; + bwr.write_size = len; bwr.write_consumed = 0; - bwr.write_buffer = (unsigned) data; + bwr.write_buffer = (uintptr_t) data; bwr.read_size = 0; bwr.read_consumed = 0; bwr.read_buffer = 0; @@ -169,13 +170,13 @@ void binder_send_reply(struct binder_state *bs, { struct { uint32_t cmd_free; - void *buffer; + uintptr_t buffer; uint32_t cmd_reply; struct binder_transaction_data txn; } __attribute__((packed)) data; data.cmd_free = BC_FREE_BUFFER; - data.buffer = buffer_to_free; + data.buffer = (uintptr_t) buffer_to_free; data.cmd_reply = BC_REPLY; data.txn.target.ptr = 0; data.txn.cookie = 0; @@ -197,13 +198,14 @@ void binder_send_reply(struct binder_state *bs, } int binder_parse(struct binder_state *bs, struct binder_io *bio, - uint32_t *ptr, uint32_t size, binder_handler func) + uintptr_t ptr, size_t size, binder_handler func) { int r = 1; - uint32_t *end = ptr + (size / 4); + uintptr_t end = ptr + (uintptr_t) size; while (ptr < end) { - uint32_t cmd = *ptr++; + uint32_t cmd = *(uint32_t *) ptr; + ptr += sizeof(uint32_t); #if TRACE fprintf(stderr,"%s:\n", cmd_name(cmd)); #endif @@ -217,13 +219,13 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, case BR_RELEASE: case BR_DECREFS: #if TRACE - fprintf(stderr," %08x %08x\n", ptr[0], ptr[1]); + fprintf(stderr," %p, %p\n", ptr, (ptr + sizeof(void *))); #endif - ptr += 2; + ptr += sizeof(struct binder_ptr_cookie); break; case BR_TRANSACTION: { struct binder_transaction_data *txn = (struct binder_transaction_data *) ptr; - if ((end - ptr) * sizeof(uint32_t) < sizeof(*txn)) { + if ((end - ptr) < sizeof(*txn)) { ALOGE("parse: txn too small!\n"); return -1; } @@ -239,12 +241,12 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, res = func(bs, txn, &msg, &reply); binder_send_reply(bs, &reply, txn->data.ptr.buffer, res); } - ptr += sizeof(*txn) / sizeof(uint32_t); + ptr += sizeof(*txn); break; } case BR_REPLY: { struct binder_transaction_data *txn = (struct binder_transaction_data *) ptr; - if ((end - ptr) * sizeof(uint32_t) < sizeof(*txn)) { + if ((end - ptr) < sizeof(*txn)) { ALOGE("parse: reply too small!\n"); return -1; } @@ -253,14 +255,15 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, bio_init_from_txn(bio, txn); bio = 0; } else { - /* todo FREE BUFFER */ + /* todo FREE BUFFER */ } - ptr += (sizeof(*txn) / sizeof(uint32_t)); + ptr += sizeof(*txn); r = 0; break; } case BR_DEAD_BINDER: { - struct binder_death *death = (void*) *ptr++; + struct binder_death *death = (struct binder_death *) *(intptr_t *)ptr; + ptr += sizeof(void *); death->func(bs, death->ptr); break; } @@ -297,13 +300,16 @@ void binder_release(struct binder_state *bs, uint32_t target) void binder_link_to_death(struct binder_state *bs, uint32_t target, struct binder_death *death) { - uint32_t cmd[3]; - cmd[0] = BC_REQUEST_DEATH_NOTIFICATION; - cmd[1] = (uint32_t) target; - cmd[2] = (uint32_t) death; - binder_write(bs, cmd, sizeof(cmd)); -} + struct { + uint32_t cmd; + struct binder_handle_cookie payload; + } __attribute__((packed)) data; + data.cmd = BC_REQUEST_DEATH_NOTIFICATION; + data.payload.handle = target; + data.payload.cookie = (uintptr_t) death; + binder_write(bs, &data, sizeof(data)); +} int binder_call(struct binder_state *bs, struct binder_io *msg, struct binder_io *reply, @@ -314,7 +320,7 @@ int binder_call(struct binder_state *bs, struct { uint32_t cmd; struct binder_transaction_data txn; - } writebuf; + } __attribute__((packed)) writebuf; unsigned readbuf[32]; if (msg->flags & BIO_F_OVERFLOW) { @@ -333,13 +339,13 @@ int binder_call(struct binder_state *bs, bwr.write_size = sizeof(writebuf); bwr.write_consumed = 0; - bwr.write_buffer = (unsigned) &writebuf; + bwr.write_buffer = (uintptr_t) &writebuf; hexdump(msg->data0, msg->data - msg->data0); for (;;) { bwr.read_size = sizeof(readbuf); bwr.read_consumed = 0; - bwr.read_buffer = (unsigned) readbuf; + bwr.read_buffer = (uintptr_t) readbuf; res = ioctl(bs->fd, BINDER_WRITE_READ, &bwr); @@ -348,7 +354,7 @@ int binder_call(struct binder_state *bs, goto fail; } - res = binder_parse(bs, reply, readbuf, bwr.read_consumed, 0); + res = binder_parse(bs, reply, (uintptr_t) readbuf, bwr.read_consumed, 0); if (res == 0) return 0; if (res < 0) goto fail; } @@ -363,19 +369,19 @@ void binder_loop(struct binder_state *bs, binder_handler func) { int res; struct binder_write_read bwr; - unsigned readbuf[32]; + uint32_t readbuf[32]; bwr.write_size = 0; bwr.write_consumed = 0; bwr.write_buffer = 0; readbuf[0] = BC_ENTER_LOOPER; - binder_write(bs, readbuf, sizeof(unsigned)); + binder_write(bs, readbuf, sizeof(uint32_t)); for (;;) { bwr.read_size = sizeof(readbuf); bwr.read_consumed = 0; - bwr.read_buffer = (unsigned) readbuf; + bwr.read_buffer = (uintptr_t) readbuf; res = ioctl(bs->fd, BINDER_WRITE_READ, &bwr); @@ -384,7 +390,7 @@ void binder_loop(struct binder_state *bs, binder_handler func) break; } - res = binder_parse(bs, 0, readbuf, bwr.read_consumed, func); + res = binder_parse(bs, 0, (uintptr_t) readbuf, bwr.read_consumed, func); if (res == 0) { ALOGE("binder_loop: unexpected reply?!\n"); break; @@ -401,7 +407,7 @@ void bio_init_from_txn(struct binder_io *bio, struct binder_transaction_data *tx bio->data = bio->data0 = txn->data.ptr.buffer; bio->offs = bio->offs0 = txn->data.ptr.offsets; bio->data_avail = txn->data_size; - bio->offs_avail = txn->offsets_size / 4; + bio->offs_avail = txn->offsets_size / sizeof(size_t); bio->flags = BIO_F_SHARED; } @@ -424,7 +430,7 @@ void bio_init(struct binder_io *bio, void *data, bio->flags = 0; } -static void *bio_alloc(struct binder_io *bio, uint32_t size) +static void *bio_alloc(struct binder_io *bio, size_t size) { size = (size + 3) & (~3); if (size > bio->data_avail) { @@ -442,11 +448,15 @@ void binder_done(struct binder_state *bs, struct binder_io *msg, struct binder_io *reply) { + struct { + uint32_t cmd; + uintptr_t buffer; + } __attribute__((packed)) data; + if (reply->flags & BIO_F_SHARED) { - uint32_t cmd[2]; - cmd[0] = BC_FREE_BUFFER; - cmd[1] = (uint32_t) reply->data0; - binder_write(bs, cmd, sizeof(cmd)); + data.cmd = BC_FREE_BUFFER; + data.buffer = (uintptr_t) reply->data0; + binder_write(bs, &data, sizeof(data)); reply->flags = 0; } } @@ -599,7 +609,7 @@ static struct flat_binder_object *_bio_get_obj(struct binder_io *bio) size_t n; size_t off = bio->data - bio->data0; - /* TODO: be smarter about this? */ + /* TODO: be smarter about this? */ for (n = 0; n < bio->offs_avail; n++) { if (bio->offs[n] == off) return bio_get(bio, sizeof(struct flat_binder_object)); diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index c6735ee98..f8212e8ba 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -135,8 +135,8 @@ uint16_t svcmgr_id[] = { uint32_t do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, uid_t uid) { struct svcinfo *si; - si = find_svc(s, len); + si = find_svc(s, len); //ALOGI("check_service('%s') handle = %x\n", str8(s), si ? si->handle : 0); if (si && si->handle) { if (!si->allow_isolated) { @@ -158,6 +158,7 @@ int do_add_service(struct binder_state *bs, uint32_t handle, uid_t uid, int allow_isolated) { struct svcinfo *si; + //ALOGI("add_service('%s',%x,%s) uid=%d\n", str8(s), handle, // allow_isolated ? "allow_isolated" : "!allow_isolated", uid); @@ -213,8 +214,8 @@ int svcmgr_handler(struct binder_state *bs, uint32_t strict_policy; int allow_isolated; -// ALOGI("target=%p code=%d pid=%d uid=%d\n", -// txn->target, txn->code, txn->sender_pid, txn->sender_euid); + //ALOGI("target=%x code=%d pid=%d uid=%d\n", + // txn->target.handle, txn->code, txn->sender_pid, txn->sender_euid); if (txn->target.handle != svcmgr_handle) return -1; @@ -250,7 +251,7 @@ int svcmgr_handler(struct binder_state *bs, break; case SVC_MGR_LIST_SERVICES: { - unsigned n = bio_get_uint32(msg); + uint32_t n = bio_get_uint32(msg); si = svclist; while ((n-- > 0) && si) From f683e0163a84d93448b9388126902242367cd961 Mon Sep 17 00:00:00 2001 From: Serban Constantinescu Date: Tue, 5 Nov 2013 16:53:55 +0000 Subject: [PATCH 07/11] Binder: Make binder portable Changes include - Binder attempts to cast pointers to a int datatype which is not sufficient on a 64-bit platform. - This patch introduces new read/write functions into Parcel that allow pointers to be written using the uintptr_t datatype for compile-time data type size selection. - Change access specifier for the methods above. - Binder uses the 64bit android_atomic_release_cas64 (aka cmpxchg) Change-Id: I595280541e0ba1d19c94b2ca2127bf9d96efabf1 Signed-off-by: Matthew Leach Signed-off-by: Serban Constantinescu --- include/binder/Parcel.h | 4 ++++ libs/binder/Binder.cpp | 5 +++++ libs/binder/IPCThreadState.cpp | 40 +++++++++++++++++----------------- libs/binder/MemoryDealer.cpp | 4 ++-- libs/binder/Parcel.cpp | 16 ++++++++++++++ 5 files changed, 47 insertions(+), 22 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index c95f297b4..fa13ff5f1 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -38,6 +38,7 @@ class TextOutput; struct flat_binder_object; // defined in support_p/binder_module.h class Parcel { + friend class IPCThreadState; public: class ReadableBlob; class WritableBlob; @@ -218,6 +219,9 @@ private: status_t growData(size_t len); status_t restartWrite(size_t desired); status_t continueWrite(size_t desired); + status_t writePointer(uintptr_t val); + status_t readPointer(uintptr_t *pArg) const; + uintptr_t readPointer() const; void freeDataNoInit(); void initState(); void scanForFds() const; diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 1f21f9c32..320bdea8f 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -142,8 +142,13 @@ void BBinder::attachObject( if (!e) { e = new Extras; +#ifdef __LP64__ + if (android_atomic_release_cas64(0, reinterpret_cast(e), + reinterpret_cast(&mExtras)) != 0) { +#else if (android_atomic_cmpxchg(0, reinterpret_cast(e), reinterpret_cast(&mExtras)) != 0) { +#endif delete e; e = mExtras; } diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 5951a3ff4..84bb94d49 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -663,7 +663,7 @@ status_t IPCThreadState::requestDeathNotification(int32_t handle, BpBinder* prox { mOut.writeInt32(BC_REQUEST_DEATH_NOTIFICATION); mOut.writeInt32((int32_t)handle); - mOut.writeInt32((int32_t)proxy); + mOut.writePointer((uintptr_t)proxy); return NO_ERROR; } @@ -671,7 +671,7 @@ status_t IPCThreadState::clearDeathNotification(int32_t handle, BpBinder* proxy) { mOut.writeInt32(BC_CLEAR_DEATH_NOTIFICATION); mOut.writeInt32((int32_t)handle); - mOut.writeInt32((int32_t)proxy); + mOut.writePointer((uintptr_t)proxy); return NO_ERROR; } @@ -950,8 +950,8 @@ status_t IPCThreadState::executeCommand(int32_t cmd) break; case BR_ACQUIRE: - refs = (RefBase::weakref_type*)mIn.readInt32(); - obj = (BBinder*)mIn.readInt32(); + refs = (RefBase::weakref_type*)mIn.readPointer(); + obj = (BBinder*)mIn.readPointer(); ALOG_ASSERT(refs->refBase() == obj, "BR_ACQUIRE: object %p does not match cookie %p (expected %p)", refs, obj, refs->refBase()); @@ -961,13 +961,13 @@ status_t IPCThreadState::executeCommand(int32_t cmd) obj->printRefs(); } mOut.writeInt32(BC_ACQUIRE_DONE); - mOut.writeInt32((int32_t)refs); - mOut.writeInt32((int32_t)obj); + mOut.writePointer((uintptr_t)refs); + mOut.writePointer((uintptr_t)obj); break; case BR_RELEASE: - refs = (RefBase::weakref_type*)mIn.readInt32(); - obj = (BBinder*)mIn.readInt32(); + refs = (RefBase::weakref_type*)mIn.readPointer(); + obj = (BBinder*)mIn.readPointer(); ALOG_ASSERT(refs->refBase() == obj, "BR_RELEASE: object %p does not match cookie %p (expected %p)", refs, obj, refs->refBase()); @@ -979,17 +979,17 @@ status_t IPCThreadState::executeCommand(int32_t cmd) break; case BR_INCREFS: - refs = (RefBase::weakref_type*)mIn.readInt32(); - obj = (BBinder*)mIn.readInt32(); + refs = (RefBase::weakref_type*)mIn.readPointer(); + obj = (BBinder*)mIn.readPointer(); refs->incWeak(mProcess.get()); mOut.writeInt32(BC_INCREFS_DONE); - mOut.writeInt32((int32_t)refs); - mOut.writeInt32((int32_t)obj); + mOut.writePointer((uintptr_t)refs); + mOut.writePointer((uintptr_t)obj); break; case BR_DECREFS: - refs = (RefBase::weakref_type*)mIn.readInt32(); - obj = (BBinder*)mIn.readInt32(); + refs = (RefBase::weakref_type*)mIn.readPointer(); + obj = (BBinder*)mIn.readPointer(); // NOTE: This assertion is not valid, because the object may no // longer exist (thus the (BBinder*)cast above resulting in a different // memory address). @@ -1000,8 +1000,8 @@ status_t IPCThreadState::executeCommand(int32_t cmd) break; case BR_ATTEMPT_ACQUIRE: - refs = (RefBase::weakref_type*)mIn.readInt32(); - obj = (BBinder*)mIn.readInt32(); + refs = (RefBase::weakref_type*)mIn.readPointer(); + obj = (BBinder*)mIn.readPointer(); { const bool success = refs->attemptIncStrong(mProcess.get()); @@ -1103,15 +1103,15 @@ status_t IPCThreadState::executeCommand(int32_t cmd) case BR_DEAD_BINDER: { - BpBinder *proxy = (BpBinder*)mIn.readInt32(); + BpBinder *proxy = (BpBinder*)mIn.readPointer(); proxy->sendObituary(); mOut.writeInt32(BC_DEAD_BINDER_DONE); - mOut.writeInt32((int32_t)proxy); + mOut.writePointer((uintptr_t)proxy); } break; case BR_CLEAR_DEATH_NOTIFICATION_DONE: { - BpBinder *proxy = (BpBinder*)mIn.readInt32(); + BpBinder *proxy = (BpBinder*)mIn.readPointer(); proxy->getWeakRefs()->decWeak(proxy); } break; @@ -1166,7 +1166,7 @@ void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data, size_t data if (parcel != NULL) parcel->closeFileDescriptors(); IPCThreadState* state = self(); state->mOut.writeInt32(BC_FREE_BUFFER); - state->mOut.writeInt32((int32_t)data); + state->mOut.writePointer((uintptr_t)data); } }; // namespace android diff --git a/libs/binder/MemoryDealer.cpp b/libs/binder/MemoryDealer.cpp index 8d0e0a741..2c9f2442c 100644 --- a/libs/binder/MemoryDealer.cpp +++ b/libs/binder/MemoryDealer.cpp @@ -445,8 +445,8 @@ void SimpleBestFitAllocator::dump_l(String8& result, int np = ((cur->next) && cur->next->prev != cur) ? 1 : 0; int pn = ((cur->prev) && cur->prev->next != cur) ? 2 : 0; - snprintf(buffer, SIZE, " %3u: %08x | 0x%08X | 0x%08X | %s %s\n", - i, int(cur), int(cur->start*kMemoryAlign), + snprintf(buffer, SIZE, " %3u: %p | 0x%08X | 0x%08X | %s %s\n", + i, cur, int(cur->start*kMemoryAlign), int(cur->size*kMemoryAlign), int(cur->free) ? "F" : "A", errs[np|pn]); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 38e019cb9..20cdb9ae9 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -633,6 +633,11 @@ status_t Parcel::writeInt64(int64_t val) return writeAligned(val); } +status_t Parcel::writePointer(uintptr_t val) +{ + return writeAligned(val); +} + status_t Parcel::writeFloat(float val) { return writeAligned(val); @@ -978,6 +983,17 @@ int64_t Parcel::readInt64() const return readAligned(); } +status_t Parcel::readPointer(uintptr_t *pArg) const +{ + return readAligned(pArg); +} + +uintptr_t Parcel::readPointer() const +{ + return readAligned(); +} + + status_t Parcel::readFloat(float *pArg) const { return readAligned(pArg); From e91fff0a2dfe7d312286b140c8069c820627da8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= Date: Tue, 28 Jan 2014 19:57:02 -0800 Subject: [PATCH 08/11] Add BINDER_IPC_32BIT to CFLAGS unless TARGET_USES_64_BIT_BINDER is true Change-Id: I96c643123b0314c361b7f48a18d5c22c660d4ff5 --- cmds/servicemanager/Android.mk | 4 ++++ libs/binder/Android.mk | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/cmds/servicemanager/Android.mk b/cmds/servicemanager/Android.mk index 138be5232..12149f621 100644 --- a/cmds/servicemanager/Android.mk +++ b/cmds/servicemanager/Android.mk @@ -3,6 +3,10 @@ LOCAL_PATH:= $(call my-dir) svc_c_flags = \ -Wall -Wextra \ +ifneq ($(TARGET_USES_64_BIT_BINDER),true) +svc_c_flags += -DBINDER_IPC_32BIT=1 +endif + include $(CLEAR_VARS) LOCAL_SHARED_LIBRARIES := liblog LOCAL_SRC_FILES := bctest.c binder.c diff --git a/libs/binder/Android.mk b/libs/binder/Android.mk index f3f8dafc2..3c6b8ad90 100644 --- a/libs/binder/Android.mk +++ b/libs/binder/Android.mk @@ -42,6 +42,9 @@ LOCAL_LDLIBS += -lpthread LOCAL_MODULE := libbinder LOCAL_SHARED_LIBRARIES := liblog libcutils libutils LOCAL_SRC_FILES := $(sources) +ifneq ($(TARGET_USES_64_BIT_BINDER),true) +LOCAL_CFLAGS += -DBINDER_IPC_32BIT=1 +endif include $(BUILD_SHARED_LIBRARY) include $(CLEAR_VARS) @@ -49,4 +52,7 @@ LOCAL_LDLIBS += -lpthread LOCAL_MODULE := libbinder LOCAL_STATIC_LIBRARIES += libutils LOCAL_SRC_FILES := $(sources) +ifneq ($(TARGET_USES_64_BIT_BINDER),true) +LOCAL_CFLAGS += -DBINDER_IPC_32BIT=1 +endif include $(BUILD_STATIC_LIBRARY) From 84e625ac1e01f5a9c3ed16664da6498212ed662b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= Date: Tue, 28 Jan 2014 20:12:59 -0800 Subject: [PATCH 09/11] Binder: Use 64 bit pointers in 32 processes if selected by the target Uses new kernel header where void * has been replaced by binder_uintptr_t Change-Id: Icfc67c2a279269f700343bd9246fd7cb94efe2c1 --- include/binder/IPCThreadState.h | 2 +- include/binder/Parcel.h | 20 +++--- libs/binder/IPCThreadState.cpp | 32 +++++----- libs/binder/Parcel.cpp | 105 +++++++++++++++++--------------- 4 files changed, 84 insertions(+), 75 deletions(-) diff --git a/include/binder/IPCThreadState.h b/include/binder/IPCThreadState.h index 5bc123e3c..6e0c01bea 100644 --- a/include/binder/IPCThreadState.h +++ b/include/binder/IPCThreadState.h @@ -107,7 +107,7 @@ private: static void threadDestructor(void *st); static void freeBuffer(Parcel* parcel, const uint8_t* data, size_t dataSize, - const size_t* objects, size_t objectsSize, + const binder_size_t* objects, size_t objectsSize, void* cookie); const sp mProcess; diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index fa13ff5f1..ed2e7df2c 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -23,6 +23,7 @@ #include #include #include +#include // --------------------------------------------------------------------------- namespace android { @@ -35,8 +36,6 @@ class ProcessState; class String8; class TextOutput; -struct flat_binder_object; // defined in support_p/binder_module.h - class Parcel { friend class IPCThreadState; public: @@ -82,7 +81,10 @@ public: void freeData(); - const size_t* objects() const; +private: + const binder_size_t* objects() const; + +public: size_t objectsCount() const; status_t errorCheck() const; @@ -194,19 +196,21 @@ public: // Explicitly close all file descriptors in the parcel. void closeFileDescriptors(); +private: typedef void (*release_func)(Parcel* parcel, const uint8_t* data, size_t dataSize, - const size_t* objects, size_t objectsSize, + const binder_size_t* objects, size_t objectsSize, void* cookie); - const uint8_t* ipcData() const; + uintptr_t ipcData() const; size_t ipcDataSize() const; - const size_t* ipcObjects() const; + uintptr_t ipcObjects() const; size_t ipcObjectsCount() const; void ipcSetDataReference(const uint8_t* data, size_t dataSize, - const size_t* objects, size_t objectsCount, + const binder_size_t* objects, size_t objectsCount, release_func relFunc, void* relCookie); +public: void print(TextOutput& to, uint32_t flags = 0) const; private: @@ -239,7 +243,7 @@ private: size_t mDataSize; size_t mDataCapacity; mutable size_t mDataPos; - size_t* mObjects; + binder_size_t* mObjects; size_t mObjectsSize; size_t mObjectsCapacity; mutable size_t mNextObjectHint; diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 84bb94d49..53f098735 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -753,23 +753,23 @@ status_t IPCThreadState::waitForResponse(Parcel *reply, status_t *acquireResult) reply->ipcSetDataReference( reinterpret_cast(tr.data.ptr.buffer), tr.data_size, - reinterpret_cast(tr.data.ptr.offsets), - tr.offsets_size/sizeof(size_t), + reinterpret_cast(tr.data.ptr.offsets), + tr.offsets_size/sizeof(binder_size_t), freeBuffer, this); } else { - err = *static_cast(tr.data.ptr.buffer); + err = *reinterpret_cast(tr.data.ptr.buffer); freeBuffer(NULL, reinterpret_cast(tr.data.ptr.buffer), tr.data_size, - reinterpret_cast(tr.data.ptr.offsets), - tr.offsets_size/sizeof(size_t), this); + reinterpret_cast(tr.data.ptr.offsets), + tr.offsets_size/sizeof(binder_size_t), this); } } else { freeBuffer(NULL, reinterpret_cast(tr.data.ptr.buffer), tr.data_size, - reinterpret_cast(tr.data.ptr.offsets), - tr.offsets_size/sizeof(size_t), this); + reinterpret_cast(tr.data.ptr.offsets), + tr.offsets_size/sizeof(binder_size_t), this); continue; } } @@ -809,12 +809,12 @@ status_t IPCThreadState::talkWithDriver(bool doReceive) const size_t outAvail = (!doReceive || needRead) ? mOut.dataSize() : 0; bwr.write_size = outAvail; - bwr.write_buffer = (long unsigned int)mOut.data(); + bwr.write_buffer = (uintptr_t)mOut.data(); // This is what we'll read. if (doReceive && needRead) { bwr.read_size = mIn.dataCapacity(); - bwr.read_buffer = (long unsigned int)mIn.data(); + bwr.read_buffer = (uintptr_t)mIn.data(); } else { bwr.read_size = 0; bwr.read_buffer = 0; @@ -868,7 +868,7 @@ status_t IPCThreadState::talkWithDriver(bool doReceive) if (err >= NO_ERROR) { if (bwr.write_consumed > 0) { - if (bwr.write_consumed < (ssize_t)mOut.dataSize()) + if (bwr.write_consumed < mOut.dataSize()) mOut.remove(0, bwr.write_consumed); else mOut.setDataSize(0); @@ -909,15 +909,15 @@ status_t IPCThreadState::writeTransactionData(int32_t cmd, uint32_t binderFlags, if (err == NO_ERROR) { tr.data_size = data.ipcDataSize(); tr.data.ptr.buffer = data.ipcData(); - tr.offsets_size = data.ipcObjectsCount()*sizeof(size_t); + tr.offsets_size = data.ipcObjectsCount()*sizeof(binder_size_t); tr.data.ptr.offsets = data.ipcObjects(); } else if (statusBuffer) { tr.flags |= TF_STATUS_CODE; *statusBuffer = err; tr.data_size = sizeof(status_t); - tr.data.ptr.buffer = statusBuffer; + tr.data.ptr.buffer = reinterpret_cast(statusBuffer); tr.offsets_size = 0; - tr.data.ptr.offsets = NULL; + tr.data.ptr.offsets = 0; } else { return (mLastError = err); } @@ -1026,8 +1026,8 @@ status_t IPCThreadState::executeCommand(int32_t cmd) buffer.ipcSetDataReference( reinterpret_cast(tr.data.ptr.buffer), tr.data_size, - reinterpret_cast(tr.data.ptr.offsets), - tr.offsets_size/sizeof(size_t), freeBuffer, this); + reinterpret_cast(tr.data.ptr.offsets), + tr.offsets_size/sizeof(binder_size_t), freeBuffer, this); const pid_t origPid = mCallingPid; const uid_t origUid = mCallingUid; @@ -1155,7 +1155,7 @@ void IPCThreadState::threadDestructor(void *st) void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data, size_t dataSize, - const size_t* objects, size_t objectsSize, + const binder_size_t* objects, size_t objectsSize, void* cookie) { //ALOGI("Freeing parcel %p", &parcel); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 20cdb9ae9..ee453b657 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -77,12 +77,12 @@ void acquire_object(const sp& proc, case BINDER_TYPE_BINDER: if (obj.binder) { LOG_REFS("Parcel %p acquiring reference on local %p", who, obj.cookie); - static_cast(obj.cookie)->incStrong(who); + reinterpret_cast(obj.cookie)->incStrong(who); } return; case BINDER_TYPE_WEAK_BINDER: if (obj.binder) - static_cast(obj.binder)->incWeak(who); + reinterpret_cast(obj.binder)->incWeak(who); return; case BINDER_TYPE_HANDLE: { const sp b = proc->getStrongProxyForHandle(obj.handle); @@ -114,12 +114,12 @@ void release_object(const sp& proc, case BINDER_TYPE_BINDER: if (obj.binder) { LOG_REFS("Parcel %p releasing reference on local %p", who, obj.cookie); - static_cast(obj.cookie)->decStrong(who); + reinterpret_cast(obj.cookie)->decStrong(who); } return; case BINDER_TYPE_WEAK_BINDER: if (obj.binder) - static_cast(obj.binder)->decWeak(who); + reinterpret_cast(obj.binder)->decWeak(who); return; case BINDER_TYPE_HANDLE: { const sp b = proc->getStrongProxyForHandle(obj.handle); @@ -135,7 +135,7 @@ void release_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if (obj.cookie != (void*)0) close(obj.handle); + if (obj.cookie != 0) close(obj.handle); return; } } @@ -165,16 +165,16 @@ status_t flatten_binder(const sp& proc, const int32_t handle = proxy ? proxy->handle() : 0; obj.type = BINDER_TYPE_HANDLE; obj.handle = handle; - obj.cookie = NULL; + obj.cookie = 0; } else { obj.type = BINDER_TYPE_BINDER; - obj.binder = local->getWeakRefs(); - obj.cookie = local; + obj.binder = reinterpret_cast(local->getWeakRefs()); + obj.cookie = reinterpret_cast(local); } } else { obj.type = BINDER_TYPE_BINDER; - obj.binder = NULL; - obj.cookie = NULL; + obj.binder = 0; + obj.cookie = 0; } return finish_flatten_binder(binder, obj, out); @@ -198,11 +198,11 @@ status_t flatten_binder(const sp& proc, const int32_t handle = proxy ? proxy->handle() : 0; obj.type = BINDER_TYPE_WEAK_HANDLE; obj.handle = handle; - obj.cookie = NULL; + obj.cookie = 0; } else { obj.type = BINDER_TYPE_WEAK_BINDER; - obj.binder = binder.get_refs(); - obj.cookie = binder.unsafe_get(); + obj.binder = reinterpret_cast(binder.get_refs()); + obj.cookie = reinterpret_cast(binder.unsafe_get()); } return finish_flatten_binder(real, obj, out); } @@ -216,14 +216,14 @@ status_t flatten_binder(const sp& proc, // implementation we are using. ALOGE("Unable to unflatten Binder weak reference!"); obj.type = BINDER_TYPE_BINDER; - obj.binder = NULL; - obj.cookie = NULL; + obj.binder = 0; + obj.cookie = 0; return finish_flatten_binder(NULL, obj, out); } else { obj.type = BINDER_TYPE_BINDER; - obj.binder = NULL; - obj.cookie = NULL; + obj.binder = 0; + obj.cookie = 0; return finish_flatten_binder(NULL, obj, out); } } @@ -242,7 +242,7 @@ status_t unflatten_binder(const sp& proc, if (flat) { switch (flat->type) { case BINDER_TYPE_BINDER: - *out = static_cast(flat->cookie); + *out = reinterpret_cast(flat->cookie); return finish_unflatten_binder(NULL, *flat, in); case BINDER_TYPE_HANDLE: *out = proc->getStrongProxyForHandle(flat->handle); @@ -261,13 +261,13 @@ status_t unflatten_binder(const sp& proc, if (flat) { switch (flat->type) { case BINDER_TYPE_BINDER: - *out = static_cast(flat->cookie); + *out = reinterpret_cast(flat->cookie); return finish_unflatten_binder(NULL, *flat, in); case BINDER_TYPE_WEAK_BINDER: - if (flat->binder != NULL) { + if (flat->binder != 0) { out->set_object_and_refs( - static_cast(flat->cookie), - static_cast(flat->binder)); + reinterpret_cast(flat->cookie), + reinterpret_cast(flat->binder)); } else { *out = NULL; } @@ -364,7 +364,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) const sp proc(ProcessState::self()); status_t err; const uint8_t *data = parcel->mData; - const size_t *objects = parcel->mObjects; + const binder_size_t *objects = parcel->mObjects; size_t size = parcel->mObjectsSize; int startPos = mDataPos; int firstIndex = -1, lastIndex = -2; @@ -411,9 +411,9 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) // grow objects if (mObjectsCapacity < mObjectsSize + numObjects) { int newSize = ((mObjectsSize + numObjects)*3)/2; - size_t *objects = - (size_t*)realloc(mObjects, newSize*sizeof(size_t)); - if (objects == (size_t*)0) { + binder_size_t *objects = + (binder_size_t*)realloc(mObjects, newSize*sizeof(binder_size_t)); + if (objects == (binder_size_t*)0) { return NO_MEMORY; } mObjects = objects; @@ -436,7 +436,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) // new Parcel now owns its own fd, and can declare that we // officially know we have fds. flat->handle = dup(flat->handle); - flat->cookie = (void*)1; + flat->cookie = 1; mHasFds = mFdsKnown = true; if (!mAllowFds) { err = FDS_NOT_ALLOWED; @@ -511,7 +511,7 @@ bool Parcel::enforceInterface(const String16& interface, } } -const size_t* Parcel::objects() const +const binder_size_t* Parcel::objects() const { return mObjects; } @@ -635,7 +635,7 @@ status_t Parcel::writeInt64(int64_t val) status_t Parcel::writePointer(uintptr_t val) { - return writeAligned(val); + return writeAligned(val); } status_t Parcel::writeFloat(float val) @@ -748,7 +748,7 @@ status_t Parcel::writeFileDescriptor(int fd, bool takeOwnership) obj.type = BINDER_TYPE_FD; obj.flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; obj.handle = fd; - obj.cookie = (void*) (takeOwnership ? 1 : 0); + obj.cookie = takeOwnership ? 1 : 0; return writeObject(obj, true); } @@ -858,7 +858,7 @@ restart_write: *reinterpret_cast(mData+mDataPos) = val; // Need to write meta-data? - if (nullMetaData || val.binder != NULL) { + if (nullMetaData || val.binder != 0) { mObjects[mObjectsSize] = mDataPos; acquire_object(ProcessState::self(), val, this); mObjectsSize++; @@ -881,7 +881,7 @@ restart_write: } if (!enoughObjects) { size_t newSize = ((mObjectsSize+2)*3)/2; - size_t* objects = (size_t*)realloc(mObjects, newSize*sizeof(size_t)); + binder_size_t* objects = (binder_size_t*)realloc(mObjects, newSize*sizeof(binder_size_t)); if (objects == NULL) return NO_MEMORY; mObjects = objects; mObjectsCapacity = newSize; @@ -985,12 +985,17 @@ int64_t Parcel::readInt64() const status_t Parcel::readPointer(uintptr_t *pArg) const { - return readAligned(pArg); + status_t ret; + binder_uintptr_t ptr; + ret = readAligned(&ptr); + if (!ret) + *pArg = ptr; + return ret; } uintptr_t Parcel::readPointer() const { - return readAligned(); + return readAligned(); } @@ -1239,7 +1244,7 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const const flat_binder_object* obj = reinterpret_cast(mData+DPOS); mDataPos = DPOS + sizeof(flat_binder_object); - if (!nullMetaData && (obj->cookie == NULL && obj->binder == NULL)) { + if (!nullMetaData && (obj->cookie == 0 && obj->binder == 0)) { // When transferring a NULL object, we don't write it into // the object list, so we don't want to check for it when // reading. @@ -1248,7 +1253,7 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const } // Ensure that this object is valid... - size_t* const OBJS = mObjects; + binder_size_t* const OBJS = mObjects; const size_t N = mObjectsSize; size_t opos = mNextObjectHint; @@ -1310,9 +1315,9 @@ void Parcel::closeFileDescriptors() } } -const uint8_t* Parcel::ipcData() const +uintptr_t Parcel::ipcData() const { - return mData; + return reinterpret_cast(mData); } size_t Parcel::ipcDataSize() const @@ -1320,9 +1325,9 @@ size_t Parcel::ipcDataSize() const return (mDataSize > mDataPos ? mDataSize : mDataPos); } -const size_t* Parcel::ipcObjects() const +uintptr_t Parcel::ipcObjects() const { - return mObjects; + return reinterpret_cast(mObjects); } size_t Parcel::ipcObjectsCount() const @@ -1331,7 +1336,7 @@ size_t Parcel::ipcObjectsCount() const } void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, - const size_t* objects, size_t objectsCount, release_func relFunc, void* relCookie) + const binder_size_t* objects, size_t objectsCount, release_func relFunc, void* relCookie) { freeDataNoInit(); mError = NO_ERROR; @@ -1340,7 +1345,7 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, //ALOGI("setDataReference Setting data size of %p to %lu (pid=%d)\n", this, mDataSize, getpid()); mDataPos = 0; ALOGV("setDataReference Setting data pos of %p to %d\n", this, mDataPos); - mObjects = const_cast(objects); + mObjects = const_cast(objects); mObjectsSize = mObjectsCapacity = objectsCount; mNextObjectHint = 0; mOwner = relFunc; @@ -1358,7 +1363,7 @@ void Parcel::print(TextOutput& to, uint32_t flags) const } else if (dataSize() > 0) { const uint8_t* DATA = data(); to << indent << HexDump(DATA, dataSize()) << dedent; - const size_t* OBJS = objects(); + const binder_size_t* OBJS = objects(); const size_t N = objectsCount(); for (size_t i=0; i proc(ProcessState::self()); size_t i = mObjectsSize; uint8_t* const data = mData; - size_t* const objects = mObjects; + binder_size_t* const objects = mObjects; while (i > 0) { i--; const flat_binder_object* flat @@ -1393,7 +1398,7 @@ void Parcel::acquireObjects() const sp proc(ProcessState::self()); size_t i = mObjectsSize; uint8_t* const data = mData; - size_t* const objects = mObjects; + binder_size_t* const objects = mObjects; while (i > 0) { i--; const flat_binder_object* flat @@ -1494,10 +1499,10 @@ status_t Parcel::continueWrite(size_t desired) mError = NO_MEMORY; return NO_MEMORY; } - size_t* objects = NULL; + binder_size_t* objects = NULL; if (objectsSize) { - objects = (size_t*)malloc(objectsSize*sizeof(size_t)); + objects = (binder_size_t*)malloc(objectsSize*sizeof(binder_size_t)); if (!objects) { free(data); @@ -1517,7 +1522,7 @@ status_t Parcel::continueWrite(size_t desired) memcpy(data, mData, mDataSize < desired ? mDataSize : desired); } if (objects && mObjects) { - memcpy(objects, mObjects, objectsSize*sizeof(size_t)); + memcpy(objects, mObjects, objectsSize*sizeof(binder_size_t)); } //ALOGI("Freeing data ref of %p (pid=%d)\n", this, getpid()); mOwner(this, mData, mDataSize, mObjects, mObjectsSize, mOwnerCookie); @@ -1544,8 +1549,8 @@ status_t Parcel::continueWrite(size_t desired) } release_object(proc, *flat, this); } - size_t* objects = - (size_t*)realloc(mObjects, objectsSize*sizeof(size_t)); + binder_size_t* objects = + (binder_size_t*)realloc(mObjects, objectsSize*sizeof(binder_size_t)); if (objects) { mObjects = objects; } From 399b6c3bbc0d887ab016a8bb686ff16d36edc6c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= Date: Tue, 28 Jan 2014 21:25:12 -0800 Subject: [PATCH 10/11] ServiceManager: Use 32/64 bit types from new binder header Change-Id: I1bd7c38ed9f43125cf9c63aa533434ee7ca06f80 --- cmds/servicemanager/binder.c | 49 ++++++++++++++++++------------------ cmds/servicemanager/binder.h | 4 +-- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c index 7f8d0e0ed..db7632d36 100644 --- a/cmds/servicemanager/binder.c +++ b/cmds/servicemanager/binder.c @@ -1,6 +1,7 @@ /* Copyright 2008 The Android Open Source Project */ +#include #include #include #include @@ -41,18 +42,18 @@ void hexdump(void *_data, size_t len) void binder_dump_txn(struct binder_transaction_data *txn) { struct flat_binder_object *obj; - size_t *offs = txn->data.ptr.offsets; - size_t count = txn->offsets_size / sizeof(size_t); + binder_size_t *offs = (binder_size_t *)(uintptr_t)txn->data.ptr.offsets; + size_t count = txn->offsets_size / sizeof(binder_size_t); - fprintf(stderr," target %p cookie %p code %08x flags %08x\n", - txn->target.ptr, txn->cookie, txn->code, txn->flags); - fprintf(stderr," pid %8d uid %8d data %zu offs %zu\n", - txn->sender_pid, txn->sender_euid, txn->data_size, txn->offsets_size); - hexdump(txn->data.ptr.buffer, txn->data_size); + fprintf(stderr," target %016"PRIx64" cookie %016"PRIx64" code %08x flags %08x\n", + (uint64_t)txn->target.ptr, (uint64_t)txn->cookie, txn->code, txn->flags); + fprintf(stderr," pid %8d uid %8d data %"PRIu64" offs %"PRIu64"\n", + txn->sender_pid, txn->sender_euid, (uint64_t)txn->data_size, (uint64_t)txn->offsets_size); + hexdump((void *)(uintptr_t)txn->data.ptr.buffer, txn->data_size); while (count--) { - obj = (struct flat_binder_object *) (((char*) txn->data.ptr.buffer) + *offs++); - fprintf(stderr," - type %08x flags %08x ptr %p cookie %p\n", - obj->type, obj->flags, obj->binder, obj->cookie); + obj = (struct flat_binder_object *) (((char*)(uintptr_t)txn->data.ptr.buffer) + *offs++); + fprintf(stderr," - type %08x flags %08x ptr %016"PRIx64" cookie %016"PRIx64"\n", + obj->type, obj->flags, (uint64_t)obj->binder, (uint64_t)obj->cookie); } } @@ -165,18 +166,18 @@ int binder_write(struct binder_state *bs, void *data, size_t len) void binder_send_reply(struct binder_state *bs, struct binder_io *reply, - const void *buffer_to_free, + binder_uintptr_t buffer_to_free, int status) { struct { uint32_t cmd_free; - uintptr_t buffer; + binder_uintptr_t buffer; uint32_t cmd_reply; struct binder_transaction_data txn; } __attribute__((packed)) data; data.cmd_free = BC_FREE_BUFFER; - data.buffer = (uintptr_t) buffer_to_free; + data.buffer = buffer_to_free; data.cmd_reply = BC_REPLY; data.txn.target.ptr = 0; data.txn.cookie = 0; @@ -185,14 +186,14 @@ void binder_send_reply(struct binder_state *bs, data.txn.flags = TF_STATUS_CODE; data.txn.data_size = sizeof(int); data.txn.offsets_size = 0; - data.txn.data.ptr.buffer = &status; + data.txn.data.ptr.buffer = (uintptr_t)&status; data.txn.data.ptr.offsets = 0; } else { data.txn.flags = 0; data.txn.data_size = reply->data - reply->data0; data.txn.offsets_size = ((char*) reply->offs) - ((char*) reply->offs0); - data.txn.data.ptr.buffer = reply->data0; - data.txn.data.ptr.offsets = reply->offs0; + data.txn.data.ptr.buffer = (uintptr_t)reply->data0; + data.txn.data.ptr.offsets = (uintptr_t)reply->offs0; } binder_write(bs, &data, sizeof(data)); } @@ -219,7 +220,7 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, case BR_RELEASE: case BR_DECREFS: #if TRACE - fprintf(stderr," %p, %p\n", ptr, (ptr + sizeof(void *))); + fprintf(stderr," %p, %p\n", (void *)ptr, (void *)(ptr + sizeof(void *))); #endif ptr += sizeof(struct binder_ptr_cookie); break; @@ -262,8 +263,8 @@ int binder_parse(struct binder_state *bs, struct binder_io *bio, break; } case BR_DEAD_BINDER: { - struct binder_death *death = (struct binder_death *) *(intptr_t *)ptr; - ptr += sizeof(void *); + struct binder_death *death = (struct binder_death *)(uintptr_t) *(binder_uintptr_t *)ptr; + ptr += sizeof(binder_uintptr_t); death->func(bs, death->ptr); break; } @@ -334,8 +335,8 @@ int binder_call(struct binder_state *bs, writebuf.txn.flags = 0; writebuf.txn.data_size = msg->data - msg->data0; writebuf.txn.offsets_size = ((char*) msg->offs) - ((char*) msg->offs0); - writebuf.txn.data.ptr.buffer = msg->data0; - writebuf.txn.data.ptr.offsets = msg->offs0; + writebuf.txn.data.ptr.buffer = (uintptr_t)msg->data0; + writebuf.txn.data.ptr.offsets = (uintptr_t)msg->offs0; bwr.write_size = sizeof(writebuf); bwr.write_consumed = 0; @@ -404,8 +405,8 @@ void binder_loop(struct binder_state *bs, binder_handler func) void bio_init_from_txn(struct binder_io *bio, struct binder_transaction_data *txn) { - bio->data = bio->data0 = txn->data.ptr.buffer; - bio->offs = bio->offs0 = txn->data.ptr.offsets; + bio->data = bio->data0 = (char *)(intptr_t)txn->data.ptr.buffer; + bio->offs = bio->offs0 = (binder_size_t *)(intptr_t)txn->data.ptr.offsets; bio->data_avail = txn->data_size; bio->offs_avail = txn->offsets_size / sizeof(size_t); bio->flags = BIO_F_SHARED; @@ -494,7 +495,7 @@ void bio_put_obj(struct binder_io *bio, void *ptr) obj->flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; obj->type = BINDER_TYPE_BINDER; - obj->binder = ptr; + obj->binder = (uintptr_t)ptr; obj->cookie = 0; } diff --git a/cmds/servicemanager/binder.h b/cmds/servicemanager/binder.h index 24b77f662..c20727972 100644 --- a/cmds/servicemanager/binder.h +++ b/cmds/servicemanager/binder.h @@ -12,12 +12,12 @@ struct binder_state; struct binder_io { char *data; /* pointer to read/write from */ - size_t *offs; /* array of offsets */ + binder_size_t *offs; /* array of offsets */ size_t data_avail; /* bytes available in data buffer */ size_t offs_avail; /* entries available in offsets array */ char *data0; /* start of data buffer */ - size_t *offs0; /* start of offsets buffer */ + binder_size_t *offs0; /* start of offsets buffer */ uint32_t flags; uint32_t unused; }; From e5245cbf5d4e830cf605ef07f5d284d7c5d2867e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= Date: Tue, 28 Jan 2014 21:35:03 -0800 Subject: [PATCH 11/11] ServiceManager: Implement PING_TRANSACTION Stop printing "invalid id " to stderr every time a process tries to connect to the servicemanager. Change-Id: Ib0e5a0375bfa2dec2c2f9cd668bd5dda46ed6588 --- cmds/servicemanager/binder.h | 2 ++ cmds/servicemanager/service_manager.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/cmds/servicemanager/binder.h b/cmds/servicemanager/binder.h index c20727972..7915fc268 100644 --- a/cmds/servicemanager/binder.h +++ b/cmds/servicemanager/binder.h @@ -33,6 +33,8 @@ struct binder_death { #define SVC_MGR_NAME "android.os.IServiceManager" enum { + /* Must match definitions in IBinder.h and IServiceManager.h */ + PING_TRANSACTION = B_PACK_CHARS('_','P','N','G'), SVC_MGR_GET_SERVICE = 1, SVC_MGR_CHECK_SERVICE, SVC_MGR_ADD_SERVICE, diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index f8212e8ba..79ce6eda5 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -220,6 +220,9 @@ int svcmgr_handler(struct binder_state *bs, if (txn->target.handle != svcmgr_handle) return -1; + if (txn->code == PING_TRANSACTION) + return 0; + // Equivalent to Parcel::enforceInterface(), reading the RPC // header with the strict mode policy mask and the interface name. // Note that we ignore the strict_policy and don't propagate it