From 1a6f4c3bf29a4d474c4582e70fb279d0d28fb7f1 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 28 Jan 2013 17:13:35 -0800 Subject: [PATCH 1/5] init: switch property_get to use __system_property_get (cherry picked from commit 2deedfe0b1ac86ebd62d19cf7da9e7dcb508ab09) Change-Id: If3fba2cc1dd5c167b0924ddfe42dbe2e6387208a --- init/init.c | 22 ++++++++++++---------- init/init_parser.c | 16 ++++++++-------- init/keychords.c | 11 +++++------ init/property_service.c | 21 +++++++-------------- init/property_service.h | 3 ++- 5 files changed, 34 insertions(+), 39 deletions(-) diff --git a/init/init.c b/init/init.c index 39df0ffae..16470784a 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 686640ed4..cce109386 100644 --- a/init/init_parser.c +++ b/init/init_parser.c @@ -206,8 +206,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) { @@ -265,14 +266,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; @@ -543,7 +544,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); @@ -552,9 +553,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 aab08195e..d18a6e41c 100644 --- a/init/keychords.c +++ b/init/keychords.c @@ -95,24 +95,23 @@ void keychord_init() void handle_keychord() { struct service *svc; - const char* debuggable; - const char* adb_enabled; + char debuggable[PROP_VALUE_MAX]; + char adb_enabled[PROP_VALUE_MAX]; int ret; __u16 id; // only handle keychords if ro.debuggable is set or adb is enabled. // the logic here is that bugreports should be enabled in userdebug or eng builds // and on user builds for users that are developers. - debuggable = property_get("ro.debuggable"); - adb_enabled = property_get("init.svc.adbd"); + property_get("ro.debuggable", debuggable); + 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 ((debuggable && !strcmp(debuggable, "1")) || - (adb_enabled && !strcmp(adb_enabled, "running"))) { + if (!strcmp(debuggable, "1") || !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 48488be9c..dc547dbac 100755 --- a/init/property_service.c +++ b/init/property_service.c @@ -292,19 +292,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) @@ -591,8 +581,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 993b6ceeb05677229dee60f1a68c2102c205a7bd Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 23 Jan 2013 23:09:48 -0800 Subject: [PATCH 2/5] 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. (cherry picked from commit 9f5af635010a7ba92edf1fca543f7271cc9d75c8) Change-Id: Idf6100d1d0170751acd5163a22597912bff480f0 --- 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 dc547dbac..cd4d3c7cb 100755 --- 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) @@ -321,8 +294,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); @@ -337,29 +310,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 a5a860ef20cc5e65d4fa9b57cd72231f63f6d316 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 29 Jan 2013 14:58:57 -0800 Subject: [PATCH 3/5] 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. (cherry picked from commit 88ac54a4e8d2a63e4fd9c465e115795ace316776) Change-Id: Iacc2b42bfe4069e0bfcbb1c48474f30126a93139 --- 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 cd4d3c7cb..86e35f132 100755 --- a/init/property_service.c +++ b/init/property_service.c @@ -265,7 +265,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 81963164113f98f5917c7d7990ae2b76f2d4044e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 28 Jan 2013 17:14:04 -0800 Subject: [PATCH 4/5] toolbox: hide property implementation from watchprops (cherry picked from commit 91779634debc79bc75d3df4e0f59d964ad4f5f78) Change-Id: I7a2d8aa507ac61cedc5f67c563531a7d4ec8e4c2 --- 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; From 5e484e9c439f8c065270209f32bdbfb731729250 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 17 Jun 2013 16:20:08 -0700 Subject: [PATCH 5/5] init: fix copying boot properties The previous patch "init: verify size of property buffers passed to property_get" incorrectly modified one of the callers, resulting in ro.serialno, ro.bootmode, ro.baseband, and ro.bootloader always being set to their default values. Bug: 9469860 (cherry picked from commit 67e3663fc93c65b69b5d121db05b0833b98d97f1) Change-Id: Ia7b337e1fab6e334729f47ee1269e6c736615177 --- init/init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/init/init.c b/init/init.c index 16470784a..afa4d44ef 100755 --- a/init/init.c +++ b/init/init.c @@ -640,7 +640,9 @@ static void export_kernel_boot_props(void) for (i = 0; i < ARRAY_SIZE(prop_map); i++) { ret = property_get(prop_map[i].src_prop, tmp); - if (ret == 0) + if (ret > 0) + property_set(prop_map[i].dest_prop, tmp); + else property_set(prop_map[i].dest_prop, prop_map[i].def_val); }