Attachment 243956 Details for Bug 140068 – Patch

[patch] Patch

0005-Linux-SeccompBrokerClient-should-cache-arbitrary-fil.patch (text/plain), 5.73 KB, created by Michael Catanzaro on 2015-01-04 19:54:23 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Catanzaro

Created: 2015-01-04 19:54:23 PST

Size: 5.73 KB

patch

obsolete

>From 46b8386e10c1b6f0ff579911cb37f5446b4bcd6d Mon Sep 17 00:00:00 2001
>From: Michael Catanzaro <mcatanzaro@igalia.com>
>Date: Mon, 29 Dec 2014 19:50:20 -0600
>Subject: [PATCH 5/5] [Linux] SeccompBrokerClient should cache arbitrary file
> descriptors
>
>If malloc() attempts to open /proc/sys/vm/overcommit_memory in a SIGSYS
>signal handler, the SeccompBroker will attempt to recursively broker the
>open() syscall. Generalize the existing code that already handles the
>similar case where malloc() opens /sys/devices/system/cpu/online to
>handle this situation as well.
>---
> Source/WebKit2/ChangeLog                           | 21 ++++++++++++
> .../Shared/linux/SeccompFilters/SeccompBroker.cpp  | 40 +++++++++++++++-------
> 2 files changed, 49 insertions(+), 12 deletions(-)
>
>diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
>index ed2aeb7..9ac65d8 100644
>--- a/Source/WebKit2/ChangeLog
>+++ b/Source/WebKit2/ChangeLog
>@@ -1,5 +1,26 @@
> 2015-01-04  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>+        [Linux] SeccompBrokerClient should cache arbitrary file descriptors
>+        https://bugs.webkit.org/show_bug.cgi?id=140068
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        If malloc() attempts to open /proc/sys/vm/overcommit_memory in a SIGSYS
>+        signal handler, the SeccompBroker will attempt to recursively broker the
>+        open() syscall. Generalize the existing code that already handles the
>+        similar case where malloc() opens /sys/devices/system/cpu/online to
>+        handle this situation as well.
>+
>+        * Shared/linux/SeccompFilters/SeccompBroker.cpp:
>+        (WebKit::SIGSYSHandler):
>+        (WebKit::SeccompBrokerClient::SeccompBrokerClient):
>+        (WebKit::SeccompBrokerClient::~SeccompBrokerClient):
>+        (WebKit::SeccompBrokerClient::handleIfOpeningCachedFile):
>+        (WebKit::SeccompBrokerClient::cacheFile):
>+        (WebKit::SeccompBrokerClient::handleIfOpeningOnlineCPUCount): Deleted.
>+
>+2015-01-04  Michael Catanzaro  <mcatanzaro@igalia.com>
>+
>         [Linux] SeccompFilters: whitelist all NPAPI plugin directories
>         https://bugs.webkit.org/show_bug.cgi?id=140067
> 
>diff --git a/Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp b/Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp
>index 58e73f9..3dee16d 100644
>--- a/Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp
>+++ b/Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp
>@@ -46,7 +46,6 @@
> #endif
> 
> static const size_t messageMaxSize = 4096;
>-static const char onlineCPUCountPath[] = "/sys/devices/system/cpu/online";
> 
> namespace WebKit {
> 
>@@ -57,15 +56,19 @@ public:
> 
>     void dispatch(Syscall*) const;
> 
>-    bool handleIfOpeningOnlineCPUCount(mcontext_t*) const;
>+    bool handleIfOpeningCachedFile(mcontext_t*) const;
> 
> private:
>     SeccompBrokerClient(int socket);
> 
>+    void cacheFile(const char* pathname);
>+
>     int m_socket;
>-    int m_onlineCPUCountFd;
> 
>     mutable Mutex m_socketLock;
>+
>+    // Maps files that may be read by malloc() to open file descriptors.
>+    HashMap<String, int> m_fileDescriptorCache;
> };
> 
> static ssize_t sendMessage(int socket, void* data, size_t size, int fd = -1)
>@@ -142,7 +145,7 @@ static void SIGSYSHandler(int signal, siginfo_t* info, void* data)
> 
>     SeccompBrokerClient* client = &SeccompBrokerClient::shared();
> 
>-    if (client->handleIfOpeningOnlineCPUCount(&ucontext->uc_mcontext))
>+    if (client->handleIfOpeningCachedFile(&ucontext->uc_mcontext))
>         return;
> 
>     // createFromContext might return a nullptr if it is able to resolve the
>@@ -183,15 +186,18 @@ SeccompBrokerClient& SeccompBrokerClient::shared(int socket)
> 
> SeccompBrokerClient::SeccompBrokerClient(int socket)
>     : m_socket(socket)
>-    , m_onlineCPUCountFd(open(onlineCPUCountPath, O_RDONLY))
> {
>-    ASSERT(m_socket >= 0 && m_onlineCPUCountFd >= 0);
>+    ASSERT(m_socket >= 0);
>+
>+    cacheFile("/proc/sys/vm/overcommit_memory");
>+    cacheFile("/sys/devices/system/cpu/online");
> }
> 
> SeccompBrokerClient::~SeccompBrokerClient()
> {
>+    for (auto pair : m_fileDescriptorCache)
>+        close(pair.value);
>     close(m_socket);
>-    close(m_onlineCPUCountFd);
> }
> 
> void SeccompBrokerClient::dispatch(Syscall* syscall) const
>@@ -227,25 +233,35 @@ void SeccompBrokerClient::dispatch(Syscall* syscall) const
>     syscall->setResult(result.get());
> }
> 
>-bool SeccompBrokerClient::handleIfOpeningOnlineCPUCount(mcontext_t* context) const
>+bool SeccompBrokerClient::handleIfOpeningCachedFile(mcontext_t* context) const
> {
>     if (context->gregs[REG_SYSCALL] != __NR_open)
>         return false;
> 
>     const char *path = reinterpret_cast<char*>(context->gregs[REG_ARG0]);
>-    if (strcmp(onlineCPUCountPath, path))
>+
>+    auto iter = m_fileDescriptorCache.find(String(path));
>+    if (iter == m_fileDescriptorCache.end())
>         return false;
> 
>     // Malloc will eventually check the number of online CPUs (i.e being
>     // scheduled) present on the system by opening a special file. If it does
>     // that in the middle of the SIGSYS signal handler, it might trigger a
>-    // recursive attempt of proxying the open() syscall to the broker.
>-    // Because of that, we cache this resource.
>-    context->gregs[REG_SYSCALL] = dup(m_onlineCPUCountFd);
>+    // recursive attempt of proxying the open() syscall to the broker. The same
>+    // problem occurs if malloc() checks the memory overcommit policy. Because
>+    // of that, we cache these resources.
>+    context->gregs[REG_SYSCALL] = dup(iter->value);
> 
>     return true;
> }
> 
>+void SeccompBrokerClient::cacheFile(const char* pathname)
>+{
>+    int fd = open(pathname, O_RDONLY);
>+    ASSERT(fd >= 0);
>+    m_fileDescriptorCache.set(pathname, fd);
>+}
>+
> void SeccompBroker::launchProcess(SeccompFilters* filters, const SyscallPolicy& policy)
> {
>     static bool initialized = false;
>-- 
>2.1.0
>

Actions: View | Formatted Diff | Diff

Attachments on bug 140068: 243956 | 249294 | 256839 | 256840