From 761ba27d62a67c098a3323fb37175a7274ee5f19 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Mon, 16 Jul 2012 08:24:32 -0700 Subject: [PATCH 1/3] Merge "FORTIFY_SOURCE: revert memcpy changes." --- libc/include/string.h | 10 +--------- libc/string/__memcpy_chk.c | 32 +++----------------------------- 2 files changed, 4 insertions(+), 38 deletions(-) diff --git a/libc/include/string.h b/libc/include/string.h index 8730ea3f4..8e472e7c0 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -93,10 +93,6 @@ extern void __memcpy_src_size_error() __attribute__((__error__("memcpy called with size bigger than source"))); extern void __memcpy_overlap_error() __attribute__((__error__("memcpy called with overlapping regions"))); -extern void *__memcpy_real(void *, const void *, size_t) - __asm__(__USER_LABEL_PREFIX__ "memcpy"); -extern void *__memcpy_chk2(void *, const void *, size_t, size_t, size_t); - __BIONIC_FORTIFY_INLINE void *memcpy (void *dest, const void *src, size_t copy_amount) { @@ -118,11 +114,7 @@ void *memcpy (void *dest, const void *src, size_t copy_amount) { __memcpy_overlap_error(); } - if (__builtin_constant_p(copy_amount) && __builtin_constant_p(d - s)) { - return __memcpy_real(dest, src, copy_amount); - } - - return __memcpy_chk2(dest, src, copy_amount, d_len, s_len); + return __builtin___memcpy_chk(dest, src, copy_amount, d_len); } __BIONIC_FORTIFY_INLINE diff --git a/libc/string/__memcpy_chk.c b/libc/string/__memcpy_chk.c index 934ed673f..10334bab4 100644 --- a/libc/string/__memcpy_chk.c +++ b/libc/string/__memcpy_chk.c @@ -32,7 +32,7 @@ #include /* - * Runtime implementation of __memcpy_chk2. + * Runtime implementation of __memcpy_chk. * * See * http://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html @@ -42,12 +42,9 @@ * This memcpy check is called if _FORTIFY_SOURCE is defined and * greater than 0. */ -void *__memcpy_chk2(void *dest, const void *src, - size_t copy_amount, size_t dest_len, size_t src_len) +void *__memcpy_chk(void *dest, const void *src, + size_t copy_amount, size_t dest_len) { - char *d = (char *) dest; - const char *s = (const char *) src; - if (__builtin_expect(copy_amount > dest_len, 0)) { __libc_android_log_print(ANDROID_LOG_FATAL, "libc", "*** memcpy buffer overflow detected ***\n"); @@ -55,28 +52,5 @@ void *__memcpy_chk2(void *dest, const void *src, abort(); } - if (__builtin_expect(copy_amount > src_len, 0)) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** memcpy read overflow detected ***\n"); - abort(); - } - - if (__builtin_expect(((d <= s) && ((size_t)(s - d) < copy_amount)) - || ((d >= s) && ((size_t)(d - s) < copy_amount)), 0)) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** memcpy memory overlap detected ***\n"); - abort(); - } - return memcpy(dest, src, copy_amount); } - -/* - * GCC can create references to __memcpy_chk when using - * __builtin__memmove_chk(). - */ -void *__memcpy_chk(void *dest, const void *src, - size_t copy_amount, size_t dest_len) -{ - return __memcpy_chk2(dest, src, copy_amount, dest_len, (size_t) -1); -} From f27874740f02d8acba79f2d2924a62e9162da02b Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Tue, 22 Jan 2013 12:44:11 -0800 Subject: [PATCH 2/3] Revert "libc: make system properties more secure." This reverts commit f10c5a2215b3da2e226e8bd148c86e2c146d8e90. Bug: 8045561 --- libc/bionic/system_properties.c | 29 +++++++++++++-------------- libc/include/sys/_system_properties.h | 1 - 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/libc/bionic/system_properties.c b/libc/bionic/system_properties.c index 8e53accfd..caa5ca61b 100644 --- a/libc/bionic/system_properties.c +++ b/libc/bionic/system_properties.c @@ -31,7 +31,6 @@ #include #include #include -#include #include @@ -39,7 +38,6 @@ #include #include #include -#include #include #include @@ -57,34 +55,33 @@ prop_area *__system_property_area__ = (void*) &dummy_props; int __system_properties_init(void) { prop_area *pa; - int fd; - struct stat fd_stat; + int s, fd; + unsigned sz; + char *env; if(__system_property_area__ != ((void*) &dummy_props)) { return 0; } - fd = open(PROP_FILENAME, O_RDONLY | O_NOFOLLOW); - - if (fd < 0) { + env = getenv("ANDROID_PROPERTY_WORKSPACE"); + if (!env) { return -1; } - - if (fstat(fd, &fd_stat) < 0) { - close(fd); + fd = atoi(env); + env = strchr(env, ','); + if (!env) { return -1; } - - pa = mmap(0, fd_stat.st_size, PROT_READ, MAP_SHARED, fd, 0); - - close(fd); + sz = atoi(env + 1); + + pa = mmap(0, sz, PROT_READ, MAP_SHARED, fd, 0); if(pa == MAP_FAILED) { return -1; } if((pa->magic != PROP_AREA_MAGIC) || (pa->version != PROP_AREA_VERSION)) { - munmap(pa, fd_stat.st_size); + munmap(pa, sz); return -1; } @@ -221,6 +218,8 @@ static int send_prop_msg(prop_msg *msg) int __system_property_set(const char *key, const char *value) { int err; + int tries = 0; + int update_seen = 0; prop_msg msg; if(key == 0) return -1; diff --git a/libc/include/sys/_system_properties.h b/libc/include/sys/_system_properties.h index 10c0fae16..42a7f6c0f 100644 --- a/libc/include/sys/_system_properties.h +++ b/libc/include/sys/_system_properties.h @@ -41,7 +41,6 @@ typedef struct prop_msg prop_msg; #define PROP_AREA_VERSION 0x45434f76 #define PROP_SERVICE_NAME "property_service" -#define PROP_FILENAME "/dev/__properties__" /* #define PROP_MAX_ENTRIES 247 */ /* 247 -> 32620 bytes (<32768) */ From 472fdc95a67a31125674718b48c92b2fc54d17bc Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Wed, 23 Jan 2013 11:12:07 -0800 Subject: [PATCH 3/3] am b8726037: am e7d937b5: am b3351f12: Merge "libc: use more secure system properties if available" * commit 'b8726037ee1100e2704e90d0a54ea2313bf96b00': libc: use more secure system properties if available --- libc/bionic/system_properties.c | 61 +++++++++++++++++++-------- libc/include/sys/_system_properties.h | 1 + 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/libc/bionic/system_properties.c b/libc/bionic/system_properties.c index caa5ca61b..a1312af11 100644 --- a/libc/bionic/system_properties.c +++ b/libc/bionic/system_properties.c @@ -31,12 +31,15 @@ #include #include #include +#include +#include #include #include #include #include +#include #include #include #include @@ -52,36 +55,62 @@ static unsigned dummy_props = 0; prop_area *__system_property_area__ = (void*) &dummy_props; +static int get_fd_from_env(void) +{ + char *env = getenv("ANDROID_PROPERTY_WORKSPACE"); + + if (!env) { + return -1; + } + + return atoi(env); +} + int __system_properties_init(void) { - prop_area *pa; - int s, fd; - unsigned sz; - char *env; + bool fromFile = true; if(__system_property_area__ != ((void*) &dummy_props)) { return 0; } - env = getenv("ANDROID_PROPERTY_WORKSPACE"); - if (!env) { + int fd = open(PROP_FILENAME, O_RDONLY | O_NOFOLLOW); + + if ((fd < 0) && (errno == ENOENT)) { + /* + * For backwards compatibility, if the file doesn't + * exist, we use the environment to get the file descriptor. + * For security reasons, we only use this backup if the kernel + * returns ENOENT. We don't want to use the backup if the kernel + * returns other errors such as ENOMEM or ENFILE, since it + * might be possible for an external program to trigger this + * condition. + */ + fd = get_fd_from_env(); + fromFile = false; + } + + if (fd < 0) { return -1; } - fd = atoi(env); - env = strchr(env, ','); - if (!env) { + + struct stat fd_stat; + if (fstat(fd, &fd_stat) < 0) { return -1; } - sz = atoi(env + 1); - - pa = mmap(0, sz, PROT_READ, MAP_SHARED, fd, 0); - - if(pa == MAP_FAILED) { + + prop_area *pa = mmap(0, fd_stat.st_size, PROT_READ, MAP_SHARED, fd, 0); + + if (fromFile) { + close(fd); + } + + if (pa == MAP_FAILED) { return -1; } if((pa->magic != PROP_AREA_MAGIC) || (pa->version != PROP_AREA_VERSION)) { - munmap(pa, sz); + munmap(pa, fd_stat.st_size); return -1; } @@ -218,8 +247,6 @@ static int send_prop_msg(prop_msg *msg) int __system_property_set(const char *key, const char *value) { int err; - int tries = 0; - int update_seen = 0; prop_msg msg; if(key == 0) return -1; diff --git a/libc/include/sys/_system_properties.h b/libc/include/sys/_system_properties.h index 42a7f6c0f..10c0fae16 100644 --- a/libc/include/sys/_system_properties.h +++ b/libc/include/sys/_system_properties.h @@ -41,6 +41,7 @@ typedef struct prop_msg prop_msg; #define PROP_AREA_VERSION 0x45434f76 #define PROP_SERVICE_NAME "property_service" +#define PROP_FILENAME "/dev/__properties__" /* #define PROP_MAX_ENTRIES 247 */ /* 247 -> 32620 bytes (<32768) */