Commit 9aea3a28 authored by Alex Ilin's avatar Alex Ilin Committed by Commit Bot

third_party/ashmem: Fix race in funcs initialization

ashmem_get_funcs() returns a pointer to a static local variable
s_ashmem_funcs that is initialized in a non thread-safe way.

It is possible that one thread starts the initialization of
s_ashmem_funcs and sets s_ashmem_funcs->create. s_ashmem_funcs->setProt
is not initialized yet. Then another thread calls ashmem_get_funcs() and
since s_ashmem_funcs->create != NULL, ashmem_get_funcs() returns a
pointer to a partially initialized struct. An attempt to call
s_ashmem_funcs->setProt fails on the null pointer.

This CL fixes the data race by using pthread_once(). pthread_once()
guarantees that the initialization function will be called exactly once
ans that on return from pthread_once() the initialization has completed.

Bug: 985870
Change-Id: I182c3edd43eb2cb58ec9e70d8e03f82742004138
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1769452Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690873}
parent 5ece0a24
...@@ -11,3 +11,6 @@ Patches: ...@@ -11,3 +11,6 @@ Patches:
0002-Use-AShareMemory-functions-when-possible.patch: 0002-Use-AShareMemory-functions-when-possible.patch:
Use ASharedMemory_xxx() functions from libandroid.so when possible Use ASharedMemory_xxx() functions from libandroid.so when possible
in order to prepare for future devices without ashmem support. in order to prepare for future devices without ashmem support.
0003-Pthread-once-for-funcs-init.patch:
Fix the data race that was introduced in the previous patch by using
pthread_once() for function pointers initialization.
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include <dlfcn.h> #include <dlfcn.h>
#include <errno.h> #include <errno.h>
#include <pthread.h>
#include <unistd.h> #include <unistd.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
...@@ -190,26 +191,30 @@ typedef struct { ...@@ -190,26 +191,30 @@ typedef struct {
ASharedMemory_setProtFunc setProt; ASharedMemory_setProtFunc setProt;
} ASharedMemoryFuncs; } ASharedMemoryFuncs;
const ASharedMemoryFuncs* ashmem_get_funcs() { static ASharedMemoryFuncs s_ashmem_funcs = {};
static ASharedMemoryFuncs s_ashmem_funcs = {}; static pthread_once_t s_ashmem_funcs_once = PTHREAD_ONCE_INIT;
static void ashmem_init_funcs() {
ASharedMemoryFuncs* funcs = &s_ashmem_funcs; ASharedMemoryFuncs* funcs = &s_ashmem_funcs;
if (funcs->create == NULL) { if (device_api_level() >= __ANDROID_API_O__) {
if (device_api_level() >= __ANDROID_API_O__) { /* Leaked intentionally! */
/* Leaked intentionally! */ void* lib = dlopen("libandroid.so", RTLD_NOW);
void* lib = dlopen("libandroid.so", RTLD_NOW); funcs->create =
funcs->create = (ASharedMemory_createFunc) (ASharedMemory_createFunc)dlsym(lib, "ASharedMemory_create");
dlsym(lib, "ASharedMemory_create"); funcs->getSize =
funcs->getSize = (ASharedMemory_getSizeFunc) (ASharedMemory_getSizeFunc)dlsym(lib, "ASharedMemory_getSize");
dlsym(lib, "ASharedMemory_getSize"); funcs->setProt =
funcs->setProt = (ASharedMemory_setProtFunc) (ASharedMemory_setProtFunc)dlsym(lib, "ASharedMemory_setProt");
dlsym(lib, "ASharedMemory_setProt"); } else {
} else { funcs->create = &ashmem_dev_create_region;
funcs->create = &ashmem_dev_create_region; funcs->getSize = &ashmem_dev_get_size_region;
funcs->getSize = &ashmem_dev_get_size_region; funcs->setProt = &ashmem_dev_set_prot_region;
funcs->setProt = &ashmem_dev_set_prot_region;
}
} }
return funcs; }
static const ASharedMemoryFuncs* ashmem_get_funcs() {
pthread_once(&s_ashmem_funcs_once, ashmem_init_funcs);
return &s_ashmem_funcs;
} }
int ashmem_create_region(const char* name, size_t size) { int ashmem_create_region(const char* name, size_t size) {
......
diff --git a/third_party/ashmem/ashmem-dev.c b/third_party/ashmem/ashmem-dev.c
index 25a33cdcd0c8..399ea36ce382 100644
--- a/third_party/ashmem/ashmem-dev.c
+++ b/third_party/ashmem/ashmem-dev.c
@@ -18,6 +18,7 @@
#include <dlfcn.h>
#include <errno.h>
+#include <pthread.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
@@ -190,26 +191,30 @@ typedef struct {
ASharedMemory_setProtFunc setProt;
} ASharedMemoryFuncs;
-const ASharedMemoryFuncs* ashmem_get_funcs() {
- static ASharedMemoryFuncs s_ashmem_funcs = {};
+static ASharedMemoryFuncs s_ashmem_funcs = {};
+static pthread_once_t s_ashmem_funcs_once = PTHREAD_ONCE_INIT;
+
+static void ashmem_init_funcs() {
ASharedMemoryFuncs* funcs = &s_ashmem_funcs;
- if (funcs->create == NULL) {
- if (device_api_level() >= __ANDROID_API_O__) {
- /* Leaked intentionally! */
- void* lib = dlopen("libandroid.so", RTLD_NOW);
- funcs->create = (ASharedMemory_createFunc)
- dlsym(lib, "ASharedMemory_create");
- funcs->getSize = (ASharedMemory_getSizeFunc)
- dlsym(lib, "ASharedMemory_getSize");
- funcs->setProt = (ASharedMemory_setProtFunc)
- dlsym(lib, "ASharedMemory_setProt");
- } else {
- funcs->create = &ashmem_dev_create_region;
- funcs->getSize = &ashmem_dev_get_size_region;
- funcs->setProt = &ashmem_dev_set_prot_region;
- }
+ if (device_api_level() >= __ANDROID_API_O__) {
+ /* Leaked intentionally! */
+ void* lib = dlopen("libandroid.so", RTLD_NOW);
+ funcs->create =
+ (ASharedMemory_createFunc)dlsym(lib, "ASharedMemory_create");
+ funcs->getSize =
+ (ASharedMemory_getSizeFunc)dlsym(lib, "ASharedMemory_getSize");
+ funcs->setProt =
+ (ASharedMemory_setProtFunc)dlsym(lib, "ASharedMemory_setProt");
+ } else {
+ funcs->create = &ashmem_dev_create_region;
+ funcs->getSize = &ashmem_dev_get_size_region;
+ funcs->setProt = &ashmem_dev_set_prot_region;
}
- return funcs;
+}
+
+static const ASharedMemoryFuncs* ashmem_get_funcs() {
+ pthread_once(&s_ashmem_funcs_once, ashmem_init_funcs);
+ return &s_ashmem_funcs;
}
int ashmem_create_region(const char* name, size_t size) {
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment