From f6941f9a44037da0b7a1407052bd207d988273d7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 23:23:56 +0200 Subject: [PATCH] event/SocketMonitor: don't cancel if OnSocketReady() returns false Expect OnSocketReady() to cancel events. If it returns false, the SocketMonitor may be destructed already. This fixes a use-after-free bug in the "httpd" output plugin. --- NEWS | 1 + src/db/plugins/ProxyDatabasePlugin.cxx | 6 ++++-- src/event/BufferedSocket.cxx | 8 +------- src/event/SocketMonitor.cxx | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index ae1e4a3b4..83362a28d 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.21.8 (not yet released) * output - httpd: add missing mutex lock + - httpd: fix use-after-free bug * fix Bonjour bug * fix build failure with GCC 9 diff --git a/src/db/plugins/ProxyDatabasePlugin.cxx b/src/db/plugins/ProxyDatabasePlugin.cxx index c4fdce0a1..f80ffd277 100644 --- a/src/db/plugins/ProxyDatabasePlugin.cxx +++ b/src/db/plugins/ProxyDatabasePlugin.cxx @@ -568,7 +568,8 @@ ProxyDatabase::OnSocketReady(gcc_unused unsigned flags) noexcept if (!is_idle) { // TODO: can this happen? IdleMonitor::Schedule(); - return false; + SocketMonitor::Cancel(); + return true; } unsigned idle = (unsigned)mpd_recv_idle(connection, false); @@ -586,7 +587,8 @@ ProxyDatabase::OnSocketReady(gcc_unused unsigned flags) noexcept idle_received |= idle; is_idle = false; IdleMonitor::Schedule(); - return false; + SocketMonitor::Cancel(); + return true; } void diff --git a/src/event/BufferedSocket.cxx b/src/event/BufferedSocket.cxx index c66eb9f42..ace399893 100644 --- a/src/event/BufferedSocket.cxx +++ b/src/event/BufferedSocket.cxx @@ -110,15 +110,9 @@ BufferedSocket::OnSocketReady(unsigned flags) noexcept if (flags & READ) { assert(!input.IsFull()); - if (!ReadToBuffer()) + if (!ReadToBuffer() || !ResumeInput()) return false; - if (!ResumeInput()) - /* we must return "true" here or - SocketMonitor::Dispatch() will call - Cancel() on a freed object */ - return true; - if (!input.IsFull()) ScheduleRead(); } diff --git a/src/event/SocketMonitor.cxx b/src/event/SocketMonitor.cxx index 0b2a36088..64cc42820 100644 --- a/src/event/SocketMonitor.cxx +++ b/src/event/SocketMonitor.cxx @@ -33,8 +33,8 @@ SocketMonitor::Dispatch(unsigned flags) noexcept { flags &= GetScheduledFlags(); - if (flags != 0 && !OnSocketReady(flags) && IsDefined()) - Cancel(); + if (flags != 0) + OnSocketReady(flags); } SocketMonitor::~SocketMonitor() noexcept