diff options
author | Robin Burchell <robin+git@viroteck.net> | 2019-07-19 17:46:21 +0200 |
---|---|---|
committer | Andreas Kling <awesomekling@gmail.com> | 2019-07-19 19:06:28 +0200 |
commit | 2d4d465206d0ca2bc6f1cc70edba89b848768f98 (patch) | |
tree | 2fbb25ab6087aa272dc03cd1bd0df84529364ef1 /Kernel/SharedBuffer.cpp | |
parent | f8beb0f665af7d21bfe36f48647b8cd88f331c52 (diff) | |
download | serenity-2d4d465206d0ca2bc6f1cc70edba89b848768f98.zip |
SharedBuffer: Fix a denial of service
It's a very bad idea to increment the refcount on behalf of another
process. That process may (for either benign or evil reasons) not
reference the SharedBuffer, and then we'll be stuck with loads of
SharedBuffers until we OOM.
Instead, increment the refcount when the buffer is mapped. That way, a
buffer is only kept if *someone* has explicitly requested it via
get_shared_buffer.
Fixes #341
Diffstat (limited to 'Kernel/SharedBuffer.cpp')
-rw-r--r-- | Kernel/SharedBuffer.cpp | 10 |
1 files changed, 6 insertions, 4 deletions
diff --git a/Kernel/SharedBuffer.cpp b/Kernel/SharedBuffer.cpp index c5574f12df..a082555bae 100644 --- a/Kernel/SharedBuffer.cpp +++ b/Kernel/SharedBuffer.cpp @@ -21,12 +21,14 @@ bool SharedBuffer::is_shared_with(pid_t peer_pid) return false; } -void* SharedBuffer::get_address(Process& process) +void* SharedBuffer::ref_for_process_and_get_address(Process& process) { LOCKER(shared_buffers().lock()); ASSERT(is_shared_with(process.pid())); for (auto& ref : m_refs) { if (ref.pid == process.pid()) { + ref.count++; + m_total_refs++; if (ref.region == nullptr) { ref.region = process.allocate_region_with_vmo(VirtualAddress(), size(), m_vmo, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0)); ref.region->set_shared(true); @@ -42,7 +44,7 @@ void SharedBuffer::share_with(pid_t peer_pid) LOCKER(shared_buffers().lock()); for (auto& ref : m_refs) { if (ref.pid == peer_pid) { - ref.count++; + // don't increment the reference count yet; let them get_shared_buffer it first. return; } } @@ -50,7 +52,7 @@ void SharedBuffer::share_with(pid_t peer_pid) m_refs.append(Reference(peer_pid)); } -void SharedBuffer::release(Process& process) +void SharedBuffer::deref_for_process(Process& process) { LOCKER(shared_buffers().lock()); for (int i = 0; i < m_refs.size(); ++i) { @@ -94,7 +96,7 @@ void SharedBuffer::disown(pid_t pid) void SharedBuffer::destroy_if_unused() { LOCKER(shared_buffers().lock()); - if (m_refs.size() == 0) { + if (m_total_refs == 0) { #ifdef SHARED_BUFFER_DEBUG kprintf("Destroying unused SharedBuffer{%p} id: %d\n", this, m_shared_buffer_id); #endif |