From 2deedfe0b1ac86ebd62d19cf7da9e7dcb508ab09 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 28 Jan 2013 17:13:35 -0800 Subject: [PATCH 1/4] init: switch property_get to use __system_property_get Change-Id: I4fc0502a1a5b331087618a4d2e3d90948743d7bd --- init/init.c | 22 ++++++++++++---------- init/init_parser.c | 16 ++++++++-------- init/keychords.c | 7 +++---- init/property_service.c | 21 +++++++-------------- init/property_service.h | 3 ++- 5 files changed, 32 insertions(+), 37 deletions(-) diff --git a/init/init.c b/init/init.c index f8b21e64a..94ffe1b7e 100755 --- a/init/init.c +++ b/init/init.c @@ -625,7 +625,7 @@ static void import_kernel_nv(char *name, int for_emulator) static void export_kernel_boot_props(void) { char tmp[PROP_VALUE_MAX]; - const char *pval; + int ret; unsigned i; struct { const char *src_prop; @@ -639,22 +639,24 @@ static void export_kernel_boot_props(void) }; for (i = 0; i < ARRAY_SIZE(prop_map); i++) { - pval = property_get(prop_map[i].src_prop); - property_set(prop_map[i].dest_prop, pval ?: prop_map[i].def_val); + ret = property_get(prop_map[i].src_prop, tmp); + if (ret == 0) + property_set(prop_map[i].dest_prop, prop_map[i].def_val); } - pval = property_get("ro.boot.console"); - if (pval) - strlcpy(console, pval, sizeof(console)); + ret = property_get("ro.boot.console", tmp); + if (ret) + strlcpy(console, tmp, sizeof(console)); /* save a copy for init's usage during boot */ - strlcpy(bootmode, property_get("ro.bootmode"), sizeof(bootmode)); + property_get("ro.bootmode", tmp); + strlcpy(bootmode, tmp, sizeof(bootmode)); /* if this was given on kernel command line, override what we read * before (e.g. from /proc/cpuinfo), if anything */ - pval = property_get("ro.boot.hardware"); - if (pval) - strlcpy(hardware, pval, sizeof(hardware)); + ret = property_get("ro.boot.hardware", tmp); + if (ret) + strlcpy(hardware, tmp, sizeof(hardware)); property_set("ro.hardware", hardware); snprintf(tmp, PROP_VALUE_MAX, "%d", revision); diff --git a/init/init_parser.c b/init/init_parser.c index a1d242355..28bf30c14 100644 --- a/init/init_parser.c +++ b/init/init_parser.c @@ -208,8 +208,9 @@ int expand_props(char *dst, const char *src, int dst_size) while (*src_ptr && left > 0) { char *c; char prop[PROP_NAME_MAX + 1]; - const char *prop_val; + char prop_val[PROP_VALUE_MAX]; int prop_len = 0; + int prop_val_len; c = strchr(src_ptr, '$'); if (!c) { @@ -267,14 +268,14 @@ int expand_props(char *dst, const char *src, int dst_size) goto err; } - prop_val = property_get(prop); - if (!prop_val) { + prop_val_len = property_get(prop, prop_val); + if (!prop_val_len) { ERROR("property '%s' doesn't exist while expanding '%s'\n", prop, src); goto err; } - ret = push_chars(&dst_ptr, &left, prop_val, strlen(prop_val)); + ret = push_chars(&dst_ptr, &left, prop_val, prop_val_len); if (ret < 0) goto err_nospace; src_ptr = c; @@ -545,7 +546,7 @@ void queue_all_property_triggers() const char* equals = strchr(name, '='); if (equals) { char prop_name[PROP_NAME_MAX + 1]; - const char* value; + char value[PROP_VALUE_MAX]; int length = equals - name; if (length > PROP_NAME_MAX) { ERROR("property name too long in trigger %s", act->name); @@ -554,9 +555,8 @@ void queue_all_property_triggers() prop_name[length] = 0; /* does the property exist, and match the trigger value? */ - value = property_get(prop_name); - if (value && (!strcmp(equals + 1, value) || - !strcmp(equals + 1, "*"))) { + property_get(prop_name, value); + if (!strcmp(equals + 1, value) ||!strcmp(equals + 1, "*")) { action_add_queue_tail(act); } } diff --git a/init/keychords.c b/init/keychords.c index 061d15701..4a6404261 100644 --- a/init/keychords.c +++ b/init/keychords.c @@ -95,20 +95,19 @@ void keychord_init() void handle_keychord() { struct service *svc; - const char* debuggable; - const char* adb_enabled; + char adb_enabled[PROP_VALUE_MAX]; int ret; __u16 id; // Only handle keychords if adb is enabled. - adb_enabled = property_get("init.svc.adbd"); + property_get("init.svc.adbd", adb_enabled); ret = read(keychord_fd, &id, sizeof(id)); if (ret != sizeof(id)) { ERROR("could not read keychord id\n"); return; } - if ((adb_enabled && !strcmp(adb_enabled, "running"))) { + if (!strcmp(adb_enabled, "running")) { svc = service_find_by_keychord(id); if (svc) { INFO("starting service %s from keychord\n", svc->name); diff --git a/init/property_service.c b/init/property_service.c index 33ec14633..cd4717192 100644 --- a/init/property_service.c +++ b/init/property_service.c @@ -299,19 +299,9 @@ static int check_perms(const char *name, unsigned int uid, unsigned int gid, cha return 0; } -const char* property_get(const char *name) +int property_get(const char *name, char value[PROP_VALUE_MAX]) { - prop_info *pi; - - if(strlen(name) >= PROP_NAME_MAX) return 0; - - pi = (prop_info*) __system_property_find(name); - - if(pi != 0) { - return pi->value; - } else { - return 0; - } + return __system_property_get(name, value); } static void write_persistent_property(const char *name, const char *value) @@ -598,8 +588,11 @@ int properties_inited(void) static void load_override_properties() { #ifdef ALLOW_LOCAL_PROP_OVERRIDE - const char *debuggable = property_get("ro.debuggable"); - if (debuggable && (strcmp(debuggable, "1") == 0)) { + char debuggable[PROP_VALUE_MAX]; + int ret; + + ret = property_get("ro.debuggable", debuggable); + if (ret && (strcmp(debuggable, "1") == 0)) { load_properties_from_file(PROP_PATH_LOCAL_OVERRIDE); } #endif /* ALLOW_LOCAL_PROP_OVERRIDE */ diff --git a/init/property_service.h b/init/property_service.h index b9d1bf610..b08c11854 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -18,6 +18,7 @@ #define _INIT_PROPERTY_H #include +#include extern void handle_property_set_fd(void); extern void property_init(void); @@ -25,7 +26,7 @@ extern void property_load_boot_defaults(void); extern void load_persist_props(void); extern void start_property_service(void); void get_property_workspace(int *fd, int *sz); -extern const char* property_get(const char *name); +extern int property_get(const char *name, char value[PROP_VALUE_MAX]); extern int property_set(const char *name, const char *value); extern int properties_inited(); int get_property_set_fd(void); From 9f5af635010a7ba92edf1fca543f7271cc9d75c8 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 23 Jan 2013 23:09:48 -0800 Subject: [PATCH 2/4] init: move the system property writer implementation Move the system property writer implementation into bionic to keep it next to the reader implementation and allow for better testing. Change-Id: I9026e604109e30546b2849b60cab2e7e5ff00ba5 --- init/property_service.c | 59 ++++++----------------------------------- 1 file changed, 8 insertions(+), 51 deletions(-) diff --git a/init/property_service.c b/init/property_service.c index cd4717192..79ff6c088 100644 --- a/init/property_service.c +++ b/init/property_service.c @@ -152,23 +152,11 @@ out: return -1; } -/* (8 header words + 247 toc words) = 1020 bytes */ -/* 1024 bytes header and toc + 247 prop_infos @ 128 bytes = 32640 bytes */ - -#define PA_COUNT_MAX 247 -#define PA_INFO_START 1024 -#define PA_SIZE 32768 - static workspace pa_workspace; -static prop_info *pa_info_array; - -extern prop_area *__system_property_area__; static int init_property_area(void) { - prop_area *pa; - - if(pa_info_array) + if (property_area_inited) return -1; if(init_workspace(&pa_workspace, PA_SIZE)) @@ -176,27 +164,12 @@ static int init_property_area(void) fcntl(pa_workspace.fd, F_SETFD, FD_CLOEXEC); - pa_info_array = (void*) (((char*) pa_workspace.data) + PA_INFO_START); + __system_property_area_init(pa_workspace.data); - pa = pa_workspace.data; - memset(pa, 0, PA_SIZE); - pa->magic = PROP_AREA_MAGIC; - pa->version = PROP_AREA_VERSION; - - /* plug into the lib property services */ - __system_property_area__ = pa; property_area_inited = 1; return 0; } -static void update_prop_info(prop_info *pi, const char *value, unsigned len) -{ - pi->serial = pi->serial | 1; - memcpy(pi->value, value, len + 1); - pi->serial = (len << 24) | ((pi->serial + 1) & 0xffffff); - __futex_wake(&pi->serial, INT32_MAX); -} - static int check_mac_perms(const char *name, char *sctx) { if (is_selinux_enabled() <= 0) @@ -328,8 +301,8 @@ static void write_persistent_property(const char *name, const char *value) int property_set(const char *name, const char *value) { - prop_area *pa; prop_info *pi; + int ret; size_t namelen = strlen(name); size_t valuelen = strlen(value); @@ -344,29 +317,13 @@ int property_set(const char *name, const char *value) /* ro.* properties may NEVER be modified once set */ if(!strncmp(name, "ro.", 3)) return -1; - pa = __system_property_area__; - update_prop_info(pi, value, valuelen); - pa->serial++; - __futex_wake(&pa->serial, INT32_MAX); + __system_property_update(pi, value, valuelen); } else { - pa = __system_property_area__; - if(pa->count == PA_COUNT_MAX) { - ERROR("Failed to set '%s'='%s', property pool is exhausted at %d entries", - name, value, PA_COUNT_MAX); - return -1; + ret = __system_property_add(name, namelen, value, valuelen); + if (ret < 0) { + ERROR("Failed to set '%s'='%s'", name, value); + return ret; } - - pi = pa_info_array + pa->count; - pi->serial = (valuelen << 24); - memcpy(pi->name, name, namelen + 1); - memcpy(pi->value, value, valuelen + 1); - - pa->toc[pa->count] = - (namelen << 24) | (((unsigned) pi) - ((unsigned) pa)); - - pa->count++; - pa->serial++; - __futex_wake(&pa->serial, INT32_MAX); } /* If name starts with "net." treat as a DNS property. */ if (strncmp("net.", name, strlen("net.")) == 0) { From 88ac54a4e8d2a63e4fd9c465e115795ace316776 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 29 Jan 2013 14:58:57 -0800 Subject: [PATCH 3/4] init: verify size of property buffers passed to property_get Verify that the buffer passed as the value parameter to property_get is always big enough. Change-Id: Ie5b6fcd94bb908215cfd55d0c9b07f717ddb70b1 --- init/property_service.c | 2 +- init/property_service.h | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/init/property_service.c b/init/property_service.c index 79ff6c088..846a0a312 100644 --- a/init/property_service.c +++ b/init/property_service.c @@ -272,7 +272,7 @@ static int check_perms(const char *name, unsigned int uid, unsigned int gid, cha return 0; } -int property_get(const char *name, char value[PROP_VALUE_MAX]) +int __property_get(const char *name, char *value) { return __system_property_get(name, value); } diff --git a/init/property_service.h b/init/property_service.h index b08c11854..46cbd8ff5 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -26,9 +26,25 @@ extern void property_load_boot_defaults(void); extern void load_persist_props(void); extern void start_property_service(void); void get_property_workspace(int *fd, int *sz); -extern int property_get(const char *name, char value[PROP_VALUE_MAX]); +extern int __property_get(const char *name, char *value); extern int property_set(const char *name, const char *value); extern int properties_inited(); int get_property_set_fd(void); +extern void __property_get_size_error() + __attribute__((__error__("property_get called with too small buffer"))); + +static inline +__attribute__ ((always_inline)) +__attribute__ ((gnu_inline)) +__attribute__ ((artificial)) +int property_get(const char *name, char *value) +{ + size_t value_len = __builtin_object_size(value, 0); + if (value_len != PROP_VALUE_MAX) + __property_get_size_error(); + + return __property_get(name, value); +} + #endif /* _INIT_PROPERTY_H */ From 91779634debc79bc75d3df4e0f59d964ad4f5f78 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 28 Jan 2013 17:14:04 -0800 Subject: [PATCH 4/4] toolbox: hide property implementation from watchprops Change-Id: Ia6609116d641d3354971ca40a16ffcab38484150 --- toolbox/watchprops.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/toolbox/watchprops.c b/toolbox/watchprops.c index d3119924b..3418d6293 100644 --- a/toolbox/watchprops.c +++ b/toolbox/watchprops.c @@ -9,9 +9,6 @@ #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ #include - -extern prop_area *__system_property_area__; - typedef struct pwatch pwatch; struct pwatch @@ -39,33 +36,35 @@ static void announce(const prop_info *pi) int watchprops_main(int argc, char *argv[]) { - prop_area *pa = __system_property_area__; - unsigned serial = pa->serial; - unsigned count = pa->count; + unsigned serial = 0; + unsigned count; unsigned n; - if(count >= 1024) exit(1); - - for(n = 0; n < count; n++) { + for(n = 0; n < 1024; n++) { watchlist[n].pi = __system_property_find_nth(n); - watchlist[n].serial = watchlist[n].pi->serial; + if (watchlist[n].pi == 0) + break; + watchlist[n].serial = __system_property_serial(watchlist[n].pi); } - for(;;) { - do { - __futex_wait(&pa->serial, serial, 0); - } while(pa->serial == serial); + count = n; + if (count == 1024) + exit(1); - while(count < pa->count){ + for(;;) { + serial = __system_property_wait_any(serial); + while(count < 1024){ watchlist[count].pi = __system_property_find_nth(count); - watchlist[count].serial = watchlist[n].pi->serial; + if (watchlist[count].pi == 0) + break; + watchlist[count].serial = __system_property_serial(watchlist[n].pi); announce(watchlist[count].pi); count++; if(count == 1024) exit(1); } for(n = 0; n < count; n++){ - unsigned tmp = watchlist[n].pi->serial; + unsigned tmp = __system_property_serial(watchlist[n].pi); if(watchlist[n].serial != tmp) { announce(watchlist[n].pi); watchlist[n].serial = tmp;