Skip to content

GitLab

T
tangled
Commit 41f4f965 authored by Vlad Tsyrklevich's avatar Vlad Tsyrklevich Committed by Commit Bot
Browse files
Options

sandbox: Deprecate use of base::ProtectedMemory 

base::ProtectedMemory is being deprecated because it's not widely used
enough to make a security impact and justify its maintenance burden.
Replace use of base::ProtectedMemory with raw function pointers and add
an attribute to disable CFI-icall checking.

Bug: 1018834
Change-Id: Ia29d4fca693a2f718f1a09ae8de5c50624103c02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884459Reviewed-by: default avatarChris Palmer <palmer@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710155}
parent 67942a1c
Hide whitespace changes
Inline Side-by-side
Showing with 30 additions and 39 deletions
sandbox/linux/services/libc_interceptor.cc
View file @ 41f4f965
......@@ -20,9 +20,8 @@
#include <set>
#include <string>
#include "base/compiler_specific.h"
#include "base/lazy_instance.h"
#include "base/memory/protected_memory.h"
#include "base/memory/protected_memory_cfi.h"
#include "base/pickle.h"
#include "base/posix/eintr_wrapper.h"
#include "base/posix/global_descriptors.h"
......@@ -174,32 +173,24 @@ bool HandleLocalTime(int fd,
typedef struct tm* (*LocaltimeFunction)(const time_t* timep);
typedef struct tm* (*LocaltimeRFunction)(const time_t* timep,
struct tm* result);
struct LibcFunctions {
LocaltimeFunction localtime;
LocaltimeFunction localtime64;
LocaltimeRFunction localtime_r;
LocaltimeRFunction localtime64_r;
};
static pthread_once_t g_libc_funcs_guard = PTHREAD_ONCE_INIT;
// The libc function pointers are stored in read-only memory after being
// dynamically resolved as a security mitigation to prevent the pointer from
// being tampered with. See https://crbug.com/771365 for details.
static PROTECTED_MEMORY_SECTION base::ProtectedMemory<LibcFunctions>
g_libc_funcs;
static pthread_once_t g_libc_localtime_funcs_guard = PTHREAD_ONCE_INIT;
static LocaltimeFunction g_libc_localtime;
static LocaltimeFunction g_libc_localtime64;
static LocaltimeRFunction g_libc_localtime_r;
static LocaltimeRFunction g_libc_localtime64_r;
static void InitLibcLocaltimeFunctionsImpl() {
auto writer = base::AutoWritableMemory::Create(g_libc_funcs);
g_libc_funcs->localtime =
g_libc_localtime =
reinterpret_cast<LocaltimeFunction>(dlsym(RTLD_NEXT, "localtime"));
g_libc_funcs->localtime64 =
g_libc_localtime64 =
reinterpret_cast<LocaltimeFunction>(dlsym(RTLD_NEXT, "localtime64"));
g_libc_funcs->localtime_r =
g_libc_localtime_r =
reinterpret_cast<LocaltimeRFunction>(dlsym(RTLD_NEXT, "localtime_r"));
g_libc_funcs->localtime64_r =
g_libc_localtime64_r =
reinterpret_cast<LocaltimeRFunction>(dlsym(RTLD_NEXT, "localtime64_r"));
if (!g_libc_funcs->localtime || !g_libc_funcs->localtime_r) {
if (!g_libc_localtime || !g_libc_localtime_r) {
// https://bugs.chromium.org/p/chromium/issues/detail?id=16800
//
// Nvidia's libGL.so overrides dlsym for an unknown reason and replaces
......@@ -211,14 +202,14 @@ static void InitLibcLocaltimeFunctionsImpl() {
"https://bugs.chromium.org/p/chromium/issues/detail?id=16800";
}
if (!g_libc_funcs->localtime)
g_libc_funcs->localtime = gmtime;
if (!g_libc_funcs->localtime64)
g_libc_funcs->localtime64 = g_libc_funcs->localtime;
if (!g_libc_funcs->localtime_r)
g_libc_funcs->localtime_r = gmtime_r;
if (!g_libc_funcs->localtime64_r)
g_libc_funcs->localtime64_r = g_libc_funcs->localtime_r;
if (!g_libc_localtime)
g_libc_localtime = gmtime;
if (!g_libc_localtime64)
g_libc_localtime64 = g_libc_localtime;
if (!g_libc_localtime_r)
g_libc_localtime_r = gmtime_r;
if (!g_libc_localtime64_r)
g_libc_localtime64_r = g_libc_localtime_r;
}
// Define localtime_override() function with asm name "localtime", so that all
......@@ -228,6 +219,7 @@ static void InitLibcLocaltimeFunctionsImpl() {
__attribute__((__visibility__("default"))) struct tm* localtime_override(
const time_t* timep) __asm__("localtime");
NO_SANITIZE("cfi-icall")
__attribute__((__visibility__("default"))) struct tm* localtime_override(
const time_t* timep) {
if (g_am_zygote_or_renderer && g_use_localtime_override) {
......@@ -239,8 +231,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime_override(
}
InitLibcLocaltimeFunctions();
struct tm* res =
base::UnsanitizedCfiCall(g_libc_funcs, &LibcFunctions::localtime)(timep);
struct tm* res = g_libc_localtime(timep);
#if defined(MEMORY_SANITIZER)
if (res)
__msan_unpoison(res, sizeof(*res));
......@@ -254,6 +245,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime_override(
__attribute__((__visibility__("default"))) struct tm* localtime64_override(
const time_t* timep) __asm__("localtime64");
NO_SANITIZE("cfi-icall")
__attribute__((__visibility__("default"))) struct tm* localtime64_override(
const time_t* timep) {
if (g_am_zygote_or_renderer && g_use_localtime_override) {
......@@ -265,8 +257,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime64_override(
}
InitLibcLocaltimeFunctions();
struct tm* res = base::UnsanitizedCfiCall(g_libc_funcs,
&LibcFunctions::localtime64)(timep);
struct tm* res = g_libc_localtime64(timep);
#if defined(MEMORY_SANITIZER)
if (res)
__msan_unpoison(res, sizeof(*res));
......@@ -280,6 +271,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime_r_override(
const time_t* timep,
struct tm* result) __asm__("localtime_r");
NO_SANITIZE("cfi-icall")
__attribute__((__visibility__("default"))) struct tm* localtime_r_override(
const time_t* timep,
struct tm* result) {
......@@ -289,8 +281,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime_r_override(
}
InitLibcLocaltimeFunctions();
struct tm* res = base::UnsanitizedCfiCall(
g_libc_funcs, &LibcFunctions::localtime_r)(timep, result);
struct tm* res = g_libc_localtime_r(timep, result);
#if defined(MEMORY_SANITIZER)
if (res)
__msan_unpoison(res, sizeof(*res));
......@@ -304,6 +295,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime64_r_override(
const time_t* timep,
struct tm* result) __asm__("localtime64_r");
NO_SANITIZE("cfi-icall")
__attribute__((__visibility__("default"))) struct tm* localtime64_r_override(
const time_t* timep,
struct tm* result) {
......@@ -313,8 +305,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime64_r_override(
}
InitLibcLocaltimeFunctions();
struct tm* res = base::UnsanitizedCfiCall(
g_libc_funcs, &LibcFunctions::localtime64_r)(timep, result);
struct tm* res = g_libc_localtime64_r(timep, result);
#if defined(MEMORY_SANITIZER)
if (res)
__msan_unpoison(res, sizeof(*res));
......@@ -344,8 +335,8 @@ bool HandleInterceptedCall(int kind,
}
void InitLibcLocaltimeFunctions() {
CHECK_EQ(0,
pthread_once(&g_libc_funcs_guard, InitLibcLocaltimeFunctionsImpl));
CHECK_EQ(0, pthread_once(&g_libc_localtime_funcs_guard,
InitLibcLocaltimeFunctionsImpl));
}
} // namespace sandbox
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