summaryrefslogtreecommitdiff
path: root/Kernel/SharedBuffer.cpp
diff options
context:
space:
mode:
authorRobin Burchell <robin+git@viroteck.net>2019-07-19 17:46:21 +0200
committerAndreas Kling <awesomekling@gmail.com>2019-07-19 19:06:28 +0200
commit2d4d465206d0ca2bc6f1cc70edba89b848768f98 (patch)
tree2fbb25ab6087aa272dc03cd1bd0df84529364ef1 /Kernel/SharedBuffer.cpp
parentf8beb0f665af7d21bfe36f48647b8cd88f331c52 (diff)
downloadserenity-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.cpp10
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