From 511826763a77f21669405fe2d48c88f0e4ba5a9f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 12:27:18 +0200 Subject: [PATCH 1/8] increment version number to 0.21.8 --- NEWS | 2 ++ android/AndroidManifest.xml | 4 ++-- doc/conf.py | 2 +- meson.build | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 9cfeb1115..90eb2286e 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,5 @@ +ver 0.21.8 (not yet released) + ver 0.21.7 (2019/04/03) * input - qobuz/tidal: scan tags when loading a playlist diff --git a/android/AndroidManifest.xml b/android/AndroidManifest.xml index e2672f984..ba343453d 100644 --- a/android/AndroidManifest.xml +++ b/android/AndroidManifest.xml @@ -2,8 +2,8 @@ + android:versionCode="30" + android:versionName="0.21.8"> diff --git a/doc/conf.py b/doc/conf.py index 426eb731b..bf7b60eb0 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -38,7 +38,7 @@ author = 'Max Kellermann' # built documents. # # The short X.Y version. -version = '0.21.7' +version = '0.21.8' # The full version, including alpha/beta/rc tags. release = version diff --git a/meson.build b/meson.build index 203191c80..466d1149d 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ project( 'mpd', ['c', 'cpp'], - version: '0.21.7', + version: '0.21.8', meson_version: '>= 0.49.0', default_options: [ 'c_std=c99', From 37b54179d882fef38ca6735b53e322027414b62e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 16:59:53 +0200 Subject: [PATCH 2/8] net/IPv[46]Address: add cast to void* to fix GCC9 build failure Fixes: src/net/IPv4Address.hxx: In member function 'constexpr IPv4Address::operator SocketAddress() const': src/net/IPv4Address.hxx:171:24: error: a reinterpret_cast is not a constant expression 171 | return SocketAddress((const struct sockaddr *)&address, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/net/IPv6Address.hxx: In member function 'constexpr IPv6Address::operator SocketAddress() const': src/net/IPv6Address.hxx:138:24: error: a reinterpret_cast is not a constant expression 138 | return SocketAddress((const struct sockaddr *)&address, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Closes https://github.com/MusicPlayerDaemon/MPD/issues/522 --- NEWS | 1 + src/net/IPv4Address.hxx | 2 +- src/net/IPv6Address.hxx | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 90eb2286e..388f8dd7b 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ ver 0.21.8 (not yet released) +* fix build failure with GCC 9 ver 0.21.7 (2019/04/03) * input diff --git a/src/net/IPv4Address.hxx b/src/net/IPv4Address.hxx index 7a39ed016..3af0210c3 100644 --- a/src/net/IPv4Address.hxx +++ b/src/net/IPv4Address.hxx @@ -168,7 +168,7 @@ public: } constexpr operator SocketAddress() const noexcept { - return SocketAddress((const struct sockaddr *)&address, + return SocketAddress((const struct sockaddr *)(const void *)&address, sizeof(address)); } diff --git a/src/net/IPv6Address.hxx b/src/net/IPv6Address.hxx index e9531b3f7..eae9b60c1 100644 --- a/src/net/IPv6Address.hxx +++ b/src/net/IPv6Address.hxx @@ -135,7 +135,7 @@ public: } constexpr operator SocketAddress() const noexcept { - return SocketAddress((const struct sockaddr *)&address, + return SocketAddress((const struct sockaddr *)(const void *)&address, sizeof(address)); } From 9111bc2c2164feeb976e489b4a50111afb11c3fa Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 22:49:25 +0200 Subject: [PATCH 3/8] output/httpd: add more API documentation about locking --- src/output/plugins/httpd/HttpdClient.hxx | 2 ++ src/output/plugins/httpd/HttpdInternal.hxx | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/output/plugins/httpd/HttpdClient.hxx b/src/output/plugins/httpd/HttpdClient.hxx index 296be22da..751d3f2c3 100644 --- a/src/output/plugins/httpd/HttpdClient.hxx +++ b/src/output/plugins/httpd/HttpdClient.hxx @@ -142,6 +142,8 @@ public: /** * Frees the client and removes it from the server's client list. + * + * Caller must lock the mutex. */ void Close() noexcept; diff --git a/src/output/plugins/httpd/HttpdInternal.hxx b/src/output/plugins/httpd/HttpdInternal.hxx index 0bc36f779..d2ac65d02 100644 --- a/src/output/plugins/httpd/HttpdInternal.hxx +++ b/src/output/plugins/httpd/HttpdInternal.hxx @@ -208,10 +208,15 @@ public: return HasClients(); } + /** + * Caller must lock the mutex. + */ void AddClient(UniqueSocketDescriptor fd) noexcept; /** * Removes a client from the httpd_output.clients linked list. + * + * Caller must lock the mutex. */ void RemoveClient(HttpdClient &client) noexcept; @@ -239,10 +244,14 @@ public: /** * Broadcasts data from the encoder to all clients. + * + * Mutext must not be locked. */ void BroadcastFromEncoder(); /** + * Mutext must not be locked. + * * Throws #std::runtime_error on error. */ void EncodeAndPlay(const void *chunk, size_t size); @@ -251,6 +260,9 @@ public: size_t Play(const void *chunk, size_t size) override; + /** + * Mutext must not be locked. + */ void CancelAllClients() noexcept; void Cancel() noexcept override; From 380656d8c97d37c0d97025d64c01cba887245d46 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 22:53:03 +0200 Subject: [PATCH 4/8] output/httpd: add missing mutex lock --- NEWS | 2 ++ src/output/plugins/httpd/HttpdClient.cxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 388f8dd7b..6712fdf37 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.21.8 (not yet released) +* output + - httpd: add missing mutex lock * fix build failure with GCC 9 ver 0.21.7 (2019/04/03) diff --git a/src/output/plugins/httpd/HttpdClient.cxx b/src/output/plugins/httpd/HttpdClient.cxx index df4e6ded1..205689462 100644 --- a/src/output/plugins/httpd/HttpdClient.cxx +++ b/src/output/plugins/httpd/HttpdClient.cxx @@ -154,7 +154,7 @@ HttpdClient::SendResponse() noexcept FormatWarning(httpd_output_domain, "failed to write to client: %s", (const char *)msg); - Close(); + LockClose(); return false; } From 325c7b8e8bd88348fcba7339006368ba78369b83 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 23:24:44 +0200 Subject: [PATCH 5/8] output/httpd: close client connection on error This missing piece probably never really hurt, because HttpdClient::OnSocketClosed() would be called right after a socket error, but it's better to be explicit about closing on error. --- src/output/plugins/httpd/HttpdClient.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/output/plugins/httpd/HttpdClient.cxx b/src/output/plugins/httpd/HttpdClient.cxx index 205689462..db602b955 100644 --- a/src/output/plugins/httpd/HttpdClient.cxx +++ b/src/output/plugins/httpd/HttpdClient.cxx @@ -428,6 +428,7 @@ void HttpdClient::OnSocketError(std::exception_ptr ep) noexcept { LogError(ep); + LockClose(); } void From df33a898d7c1ba3228135bc27a29249923635a36 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 4 Apr 2019 10:24:10 +0200 Subject: [PATCH 6/8] zeroconf/Bonjour: fix OnSocketReady() return value Keep the SocketMonitor registered. This wrong return value was added 6 years ago in commit 72cf8dd8a0451a4d9dae14a68483dc31adc61b09, andd apparently, nobody ever noticed. --- NEWS | 1 + src/zeroconf/ZeroconfBonjour.cxx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 6712fdf37..ae1e4a3b4 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.21.8 (not yet released) * output - httpd: add missing mutex lock +* fix Bonjour bug * fix build failure with GCC 9 ver 0.21.7 (2019/04/03) diff --git a/src/zeroconf/ZeroconfBonjour.cxx b/src/zeroconf/ZeroconfBonjour.cxx index 104116ebc..6c3b98240 100644 --- a/src/zeroconf/ZeroconfBonjour.cxx +++ b/src/zeroconf/ZeroconfBonjour.cxx @@ -50,7 +50,7 @@ protected: /* virtual methods from class SocketMonitor */ bool OnSocketReady(gcc_unused unsigned flags) noexcept override { DNSServiceProcessResult(service_ref); - return false; + return true; } }; From d2eb4df8fc61c4c66e29804f334e2d0e935c7ba9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 4 Apr 2019 10:14:38 +0200 Subject: [PATCH 7/8] event/{Fully,}BufferedSocket: add more API documentation --- src/event/BufferedSocket.hxx | 5 +++++ src/event/FullyBufferedSocket.hxx | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/event/BufferedSocket.hxx b/src/event/BufferedSocket.hxx index 112ae829e..09bc1a099 100644 --- a/src/event/BufferedSocket.hxx +++ b/src/event/BufferedSocket.hxx @@ -46,6 +46,11 @@ public: using SocketMonitor::Close; private: + /** + * @return the number of bytes read from the socket, 0 if the + * socket isn't ready for reading, -1 on error (the socket has + * been closed and probably destructed) + */ ssize_t DirectRead(void *data, size_t length) noexcept; /** diff --git a/src/event/FullyBufferedSocket.hxx b/src/event/FullyBufferedSocket.hxx index 2351ebf14..8cda1f968 100644 --- a/src/event/FullyBufferedSocket.hxx +++ b/src/event/FullyBufferedSocket.hxx @@ -45,6 +45,11 @@ public: } private: + /** + * @return the number of bytes written to the socket, 0 if the + * socket isn't ready for writing, -1 on error (the socket has + * been closed and probably destructed) + */ ssize_t DirectWrite(const void *data, size_t length) noexcept; protected: From f6941f9a44037da0b7a1407052bd207d988273d7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Apr 2019 23:23:56 +0200 Subject: [PATCH 8/8] 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