From 4f8010293506d4e08d184e66bf4af44ef3483611 Mon Sep 17 00:00:00 2001 From: Junjie Hu Date: Wed, 11 Nov 2015 12:52:25 +0800 Subject: Fix potential race condition on CTS TC pthread_gettid_np Root cause: If start_routine thread exits before pthread_gettid_np is invokded, the "tid" field will be cleared so that pthread_gettid_np will get "0" (which is cleared by kernel, due to the flag "CLONE_CHILD_CLEARTID" is set while calling clone system call inside pthread_create). Proposed patch: Use a mutex to guarantee pthread_gettid_np will be invoked and returned before the start_routine exits Signed-off-by: Junjie Hu Change-Id: I22411f1b0f7446d76a0373cef4ccec858fac7018 --- tests/pthread_test.cpp | 7 +++++++ 1 file changed, 7 insertions(+) mode change 100644 => 100755 tests/pthread_test.cpp diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp old mode 100644 new mode 100755 index 8ae28d8..f15cdab --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -1244,8 +1244,11 @@ TEST(pthread, pthread_attr_getstack_18908062) { } #if defined(__BIONIC__) +static pthread_mutex_t gettid_mutex; static void* pthread_gettid_np_helper(void* arg) { + pthread_mutex_lock(&gettid_mutex); *reinterpret_cast(arg) = gettid(); + pthread_mutex_unlock(&gettid_mutex); return NULL; } #endif @@ -1256,11 +1259,15 @@ TEST(pthread, pthread_gettid_np) { pid_t t_gettid_result; pthread_t t; + pthread_mutex_init(&gettid_mutex, NULL); + pthread_mutex_lock(&gettid_mutex); pthread_create(&t, NULL, pthread_gettid_np_helper, &t_gettid_result); pid_t t_pthread_gettid_np_result = pthread_gettid_np(t); + pthread_mutex_unlock(&gettid_mutex); pthread_join(t, NULL); + pthread_mutex_destroy(&gettid_mutex); ASSERT_EQ(t_gettid_result, t_pthread_gettid_np_result); #else -- cgit v1.1 From 02b1d48d51ffed14442390c80e773f3ac89396ff Mon Sep 17 00:00:00 2001 From: "Christopher R. Palmer" Date: Sun, 14 Feb 2016 11:38:44 -0500 Subject: bionic: linker: Load shim libs *before* the self-linked libs By loading them earlier, this allows us to override a symbol in a library that is being directly linked. I believe this explains why some people have had problems shimming one lib but when the changet he shim to be against a different lib it magically works. It also makes it possible to override some symbols that were nearly impossible to override before this change. For example, it is pretty much impossible to override a symbol in libutils without this change because it's loaded almost everywhere so no matter where you try to place the shimming, it will be too late and the other symbol will have priority. In particularly, this is necessary to be able to correctly shim the VectorImpl symbols for dlx. Change-Id: I461ca416bc288e28035352da00fde5f34f8d9ffa --- linker/linker.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/linker/linker.cpp b/linker/linker.cpp index c81f5d3..ed2536a 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -980,16 +980,16 @@ static bool walk_dependencies_tree(soinfo* root_soinfos[], size_t root_soinfos_s visited.push_back(si); - si->get_children().for_each([&](soinfo* child) { - visit_list.push_back(child); - }); - if (do_shims) { shim_libs_for_each(si->get_realpath(), [&](soinfo* child) { si->add_child(child); visit_list.push_back(child); }); } + + si->get_children().for_each([&](soinfo* child) { + visit_list.push_back(child); + }); } return true; -- cgit v1.1 From 4954aab7dfaab4709cdf973c71c4e39419489729 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 28 Oct 2015 17:14:48 -0700 Subject: Add prlimit to LP32. Bug: http://b/24918750 Change-Id: I0151cd66ccf79a6169610de35bb9c288c0fa4917 --- libc/bionic/legacy_32_bit_support.cpp | 19 +++++++++++++++++++ libc/include/sys/resource.h | 3 --- tests/sys_resource_test.cpp | 25 ++++++++++++++++++------- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/libc/bionic/legacy_32_bit_support.cpp b/libc/bionic/legacy_32_bit_support.cpp index a107664..1336b43 100644 --- a/libc/bionic/legacy_32_bit_support.cpp +++ b/libc/bionic/legacy_32_bit_support.cpp @@ -91,3 +91,22 @@ int getrlimit64(int resource, rlimit64* limits64) { int setrlimit64(int resource, const rlimit64* limits64) { return prlimit64(0, resource, limits64, NULL); } + +// There is no prlimit system call, so we need to use prlimit64. +int prlimit(pid_t pid, int resource, const rlimit* n32, rlimit* o32) { + rlimit64 n64; + if (n32 != nullptr) { + n64.rlim_cur = (n32->rlim_cur == RLIM_INFINITY) ? RLIM64_INFINITY : n32->rlim_cur; + n64.rlim_max = (n32->rlim_max == RLIM_INFINITY) ? RLIM64_INFINITY : n32->rlim_max; + } + + rlimit64 o64; + int result = prlimit64(pid, resource, + (n32 != nullptr) ? &n64 : nullptr, + (o32 != nullptr) ? &o64 : nullptr); + if (result != -1 && o32 != nullptr) { + o32->rlim_cur = (o64.rlim_cur == RLIM64_INFINITY) ? RLIM_INFINITY : o64.rlim_cur; + o32->rlim_max = (o64.rlim_max == RLIM64_INFINITY) ? RLIM_INFINITY : o64.rlim_max; + } + return result; +} diff --git a/libc/include/sys/resource.h b/libc/include/sys/resource.h index 3f8dd45..8209dfb 100644 --- a/libc/include/sys/resource.h +++ b/libc/include/sys/resource.h @@ -53,10 +53,7 @@ extern int setpriority(int, int, int); extern int getrusage(int, struct rusage*); -#if __LP64__ -/* Implementing prlimit for 32-bit isn't worth the effort. */ extern int prlimit(pid_t, int, const struct rlimit*, struct rlimit*); -#endif extern int prlimit64(pid_t, int, const struct rlimit64*, struct rlimit64*); __END_DECLS diff --git a/tests/sys_resource_test.cpp b/tests/sys_resource_test.cpp index 8cefc65..0b6b6ef 100644 --- a/tests/sys_resource_test.cpp +++ b/tests/sys_resource_test.cpp @@ -33,7 +33,8 @@ class SysResourceTest : public ::testing::Test { virtual void SetUp() { ASSERT_EQ(0, getrlimit(RLIMIT_CORE, &l32_)); ASSERT_EQ(0, getrlimit64(RLIMIT_CORE, &l64_)); - ASSERT_EQ(0, prlimit64(0, RLIMIT_CORE, NULL, &pr_l64_)); + ASSERT_EQ(0, prlimit(0, RLIMIT_CORE, nullptr, &pr_l32_)); + ASSERT_EQ(0, prlimit64(0, RLIMIT_CORE, nullptr, &pr_l64_)); } void CheckResourceLimits(); @@ -41,21 +42,28 @@ class SysResourceTest : public ::testing::Test { protected: rlimit l32_; rlimit64 l64_; + rlimit pr_l32_; rlimit64 pr_l64_; }; void SysResourceTest::CheckResourceLimits() { ASSERT_EQ(0, getrlimit(RLIMIT_CORE, &l32_)); ASSERT_EQ(0, getrlimit64(RLIMIT_CORE, &l64_)); - ASSERT_EQ(0, prlimit64(0, RLIMIT_CORE, NULL, &pr_l64_)); + ASSERT_EQ(0, prlimit(0, RLIMIT_CORE, nullptr, &pr_l32_)); + ASSERT_EQ(0, prlimit64(0, RLIMIT_CORE, nullptr, &pr_l64_)); + + ASSERT_EQ(l32_.rlim_cur, pr_l32_.rlim_cur); ASSERT_EQ(l64_.rlim_cur, pr_l64_.rlim_cur); + if (l64_.rlim_cur == RLIM64_INFINITY) { ASSERT_EQ(RLIM_INFINITY, l32_.rlim_cur); } else { ASSERT_EQ(l64_.rlim_cur, l32_.rlim_cur); } + ASSERT_EQ(l32_.rlim_max, pr_l32_.rlim_max); ASSERT_EQ(l64_.rlim_max, pr_l64_.rlim_max); + if (l64_.rlim_max == RLIM64_INFINITY) { ASSERT_EQ(RLIM_INFINITY, l32_.rlim_max); } else { @@ -88,13 +96,16 @@ TEST_F(SysResourceTest, setrlimit64) { ASSERT_EQ(456U, l64_.rlim_cur); } +TEST_F(SysResourceTest, prlimit) { + pr_l32_.rlim_cur = pr_l32_.rlim_max; + ASSERT_EQ(0, prlimit(0, RLIMIT_CORE, &pr_l32_, nullptr)); + CheckResourceLimits(); + ASSERT_EQ(pr_l32_.rlim_max, pr_l32_.rlim_cur); +} + TEST_F(SysResourceTest, prlimit64) { pr_l64_.rlim_cur = pr_l64_.rlim_max; - ASSERT_EQ(0, prlimit64(0, RLIMIT_CORE, &pr_l64_, NULL)); + ASSERT_EQ(0, prlimit64(0, RLIMIT_CORE, &pr_l64_, nullptr)); CheckResourceLimits(); ASSERT_EQ(pr_l64_.rlim_max, pr_l64_.rlim_cur); } - -TEST_F(SysResourceTest, prlimit) { - // prlimit is prlimit64 on LP64 and unimplemented on 32-bit. So we only test prlimit64. -} -- cgit v1.1 From 1bb333cd6711ebe80a1bc3a63298eb882297dcca Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 3 Nov 2015 18:46:02 -0800 Subject: Fix GNU/POSIX basename headers. Including glibc's will result in the user getting the POSIX version of basename always, regardless of when it is included relative to . Prior to this patch, our implementation would result in the one that's included first winning. Bug: http://b/25459151 Change-Id: Id4aaf1670dad317d6bbc05763a84ee87596e8e59 --- libc/include/libgen.h | 17 ++++---- libc/include/string.h | 4 +- tests/Android.mk | 1 + tests/libgen_basename_test.cpp | 89 ++++++++++++++++++++++++++++++++++++++++++ tests/libgen_test.cpp | 24 ------------ 5 files changed, 101 insertions(+), 34 deletions(-) create mode 100644 tests/libgen_basename_test.cpp diff --git a/libc/include/libgen.h b/libc/include/libgen.h index e89328e..4d22d15 100644 --- a/libc/include/libgen.h +++ b/libc/include/libgen.h @@ -32,18 +32,19 @@ #include #include + __BEGIN_DECLS -#if !defined(__bionic_using_gnu_basename) /* - * gets you the GNU basename. - * the POSIX one. - * Note that our "POSIX" one has the wrong argument cv-qualifiers, but doesn't - * modify its input and uses thread-local storage for the result if necessary. + * Including will get you the GNU basename, unless is + * included, either before or after including . + * + * Note that this has the wrong argument cv-qualifiers, but doesn't modify its + * input and uses thread-local storage for the result if necessary. */ -extern char* basename(const char*); -#define __bionic_using_posix_basename -#endif +extern char* __posix_basename(const char*) __RENAME(basename); + +#define basename __posix_basename /* This has the wrong argument cv-qualifiers, but doesn't modify its input and uses thread-local storage for the result if necessary. */ extern char* dirname(const char*); diff --git a/libc/include/string.h b/libc/include/string.h index d32c164..8ceccd5 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -107,18 +107,18 @@ extern size_t strxfrm(char* __restrict, const char* __restrict, size_t); extern int strcoll_l(const char *, const char *, locale_t) __purefunc; extern size_t strxfrm_l(char* __restrict, const char* __restrict, size_t, locale_t); -#if defined(__USE_GNU) && !defined(__bionic_using_posix_basename) +#if defined(__USE_GNU) && !defined(basename) /* * glibc has a basename in that's different to the POSIX one in . * It doesn't modify its argument, and in C++ it's const-correct. */ + #if defined(__cplusplus) extern "C++" char* basename(char*) __RENAME(__gnu_basename) __nonnull((1)); extern "C++" const char* basename(const char*) __RENAME(__gnu_basename) __nonnull((1)); #else extern char* basename(const char*) __RENAME(__gnu_basename) __nonnull((1)); #endif -#define __bionic_using_gnu_basename #endif extern void* __memchr_chk(const void*, int, size_t, size_t); diff --git a/tests/Android.mk b/tests/Android.mk index dc2e410..b994cc3 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -64,6 +64,7 @@ libBionicStandardTests_src_files := \ getcwd_test.cpp \ inttypes_test.cpp \ libc_logging_test.cpp \ + libgen_basename_test.cpp \ libgen_test.cpp \ locale_test.cpp \ malloc_test.cpp \ diff --git a/tests/libgen_basename_test.cpp b/tests/libgen_basename_test.cpp new file mode 100644 index 0000000..55939d1 --- /dev/null +++ b/tests/libgen_basename_test.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _GNU_SOURCE + #define _GNU_SOURCE 1 +#endif + +#include + +#if defined(basename) + #error basename should not be defined at this point +#endif + +static const char* gnu_basename(const char* in) { + return basename(in); +} + +#include + +#if !defined(basename) + #error basename should be defined at this point +#endif + +static char* posix_basename(char* in) { + return basename(in); +} + +#include +#include + +static void __TestGnuBasename(const char* in, const char* expected_out, int line) { + const char* out = gnu_basename(in); + ASSERT_STREQ(expected_out, out) << "(" << line << "): " << in << std::endl; + ASSERT_EQ(0, errno) << "(" << line << "): " << in << std::endl; +} + +static void __TestPosixBasename(const char* in, const char* expected_out, int line) { + char* writable_in = (in != NULL) ? strdup(in) : NULL; + errno = 0; + const char* out = posix_basename(&writable_in[0]); + ASSERT_STREQ(expected_out, out) << "(" << line << "): " << in << std::endl; + ASSERT_EQ(0, errno) << "(" << line << "): " << in << std::endl; + free(writable_in); +} + +#define TestGnuBasename(in, expected) __TestGnuBasename(in, expected, __LINE__) +#define TestPosixBasename(in, expected) __TestPosixBasename(in, expected, __LINE__) + +TEST(libgen_basename, gnu_basename) { + // GNU's basename doesn't accept NULL + // TestGnuBasename(NULL, "."); + TestGnuBasename("", ""); + TestGnuBasename("/usr/lib", "lib"); + TestGnuBasename("/system/bin/sh/", ""); + TestGnuBasename("/usr/", ""); + TestGnuBasename("usr", "usr"); + TestGnuBasename("/", ""); + TestGnuBasename(".", "."); + TestGnuBasename("..", ".."); + TestGnuBasename("///", ""); + TestGnuBasename("//usr//lib//", ""); +} + +TEST(libgen_basename, posix_basename) { + TestPosixBasename(NULL, "."); + TestPosixBasename("", "."); + TestPosixBasename("/usr/lib", "lib"); + TestPosixBasename("/system/bin/sh/", "sh"); + TestPosixBasename("/usr/", "usr"); + TestPosixBasename("usr", "usr"); + TestPosixBasename("/", "/"); + TestPosixBasename(".", "."); + TestPosixBasename("..", ".."); + TestPosixBasename("///", "/"); + TestPosixBasename("//usr//lib//", "lib"); +} diff --git a/tests/libgen_test.cpp b/tests/libgen_test.cpp index e9a5d5c..8a37a3f 100644 --- a/tests/libgen_test.cpp +++ b/tests/libgen_test.cpp @@ -19,15 +19,6 @@ #include #include -static void TestBasename(const char* in, const char* expected_out) { - char* writable_in = (in != NULL) ? strdup(in) : NULL; - errno = 0; - const char* out = basename(&writable_in[0]); - ASSERT_STREQ(expected_out, out) << in; - ASSERT_EQ(0, errno) << in; - free(writable_in); -} - static void TestDirname(const char* in, const char* expected_out) { char* writable_in = (in != NULL) ? strdup(in) : NULL; errno = 0; @@ -37,21 +28,6 @@ static void TestDirname(const char* in, const char* expected_out) { free(writable_in); } -// Do not use basename as the test name, it's defined to another value in glibc -// so leads to a differently named test on host versus target architectures. -TEST(libgen, posix_basename) { - TestBasename(NULL, "."); - TestBasename("", "."); - TestBasename("/usr/lib", "lib"); - TestBasename("/usr/", "usr"); - TestBasename("usr", "usr"); - TestBasename("/", "/"); - TestBasename(".", "."); - TestBasename("..", ".."); - TestBasename("///", "/"); - TestBasename("//usr//lib//", "lib"); -} - TEST(libgen, dirname) { TestDirname(NULL, "."); TestDirname("", "."); -- cgit v1.1 From e31f2d8c7de0d3f38329695afc12a7a74711dd26 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Fri, 4 Mar 2016 17:16:55 +0900 Subject: Don't leak sockets if setsockopt() or fchown() fail. Change-Id: Idcf8c08ff50d21c3a04b7ef80c4044f3f9762f2b --- libc/dns/net/getaddrinfo.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libc/dns/net/getaddrinfo.c b/libc/dns/net/getaddrinfo.c index 829b679..cc8b8b4 100644 --- a/libc/dns/net/getaddrinfo.c +++ b/libc/dns/net/getaddrinfo.c @@ -1791,10 +1791,14 @@ _find_src_addr(const struct sockaddr *addr, struct sockaddr *src_addr, unsigned return -1; } } - if (mark != MARK_UNSET && setsockopt(sock, SOL_SOCKET, SO_MARK, &mark, sizeof(mark)) < 0) + if (mark != MARK_UNSET && setsockopt(sock, SOL_SOCKET, SO_MARK, &mark, sizeof(mark)) < 0) { + close(sock); return 0; - if (uid > 0 && uid != NET_CONTEXT_INVALID_UID && fchown(sock, uid, (gid_t)-1) < 0) + } + if (uid > 0 && uid != NET_CONTEXT_INVALID_UID && fchown(sock, uid, (gid_t)-1) < 0) { + close(sock); return 0; + } do { ret = __connect(sock, addr, len); } while (ret == -1 && errno == EINTR); -- cgit v1.1 From f6f044521265ce0161276c42ea8f87d1ab6570cf Mon Sep 17 00:00:00 2001 From: "Christopher R. Palmer" Date: Sun, 7 Feb 2016 06:46:05 -0500 Subject: linker: Allow text-relocs for x86 (only) This effectively reverts https://android.googlesource.com/platform/bionic/+/e4ad91f86a47b39612e030a162f4793cb3421d31%5E%21/#F0 for x86 platforms. Unfortunately, this seems like it is required if we are going to support ffmpeg. The ffmpeg team decreed that they require text relocations for x86 (only) and that they would not fix the fact that android 6.0 makes ffmpeg unusable on x86: https://trac.ffmpeg.org/ticket/4928 Change-Id: I68397f4d62f4f6acd8e0d41b7ecdc115969b890a --- linker/linker.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/linker/linker.cpp b/linker/linker.cpp index ed2536a..bc40cf1 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -3013,12 +3013,14 @@ bool soinfo::link_image(const soinfo_list_t& global_group, const soinfo_list_t& if (has_text_relocations) { // Fail if app is targeting sdk version > 22 // TODO (dimitry): remove != __ANDROID_API__ check once http://b/20020312 is fixed +#if !defined(__i386__) // ffmpeg says that they require text relocations on x86 if (get_application_target_sdk_version() != __ANDROID_API__ && get_application_target_sdk_version() > 22) { PRINT("%s: has text relocations", get_realpath()); DL_ERR("%s: has text relocations", get_realpath()); return false; } +#endif // Make segments writable to allow text relocations to work properly. We will later call // phdr_table_protect_segments() after all of them are applied and all constructors are run. DL_WARN("%s has text relocations. This is wasting memory and prevents " -- cgit v1.1