You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/09/19 20:44:22 UTC
[1/4] mesos git commit: Fixed potential FD double close in the
libevent socket.
Repository: mesos
Updated Branches:
refs/heads/1.0.x c76a3d355 -> d6132c58c
Fixed potential FD double close in the libevent socket.
`evbuffer_add_file` will take ownership of the file descriptor passed
into it. Normally, this file descriptor is owned by the `FileEncoder`
in the libprocess's `SocketManager`. Since there are two owners, one
of the owners may close the file descriptor when it has been re-used.
In this case, when multiple threads access the master's web UI at once
with SSL enabled, the master may CHECK-fail due to a bad (closed)
file descriptor.
Review: https://reviews.apache.org/r/51509/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4c5bdd68
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4c5bdd68
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4c5bdd68
Branch: refs/heads/1.0.x
Commit: 4c5bdd683ef35880f9de2884657f2b7aead8abb6
Parents: c76a3d3
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Mon Sep 12 12:40:09 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Sep 15 09:58:17 2016 -0700
----------------------------------------------------------------------
3rdparty/libprocess/src/libevent_ssl_socket.cpp | 38 ++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4c5bdd68/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index 57a63f6..93a33e5 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -741,6 +741,36 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
std::swap(request, send_request);
}
+ // Duplicate the file descriptor because Libevent will take ownership
+ // and control the lifecycle separately.
+ //
+ // TODO(josephw): We can avoid duplicating the file descriptor in
+ // future versions of Libevent. In Libevent versions 2.1.2 and later,
+ // we may use `evbuffer_file_segment_new` and `evbuffer_add_file_segment`
+ // instead of `evbuffer_add_file`.
+ int owned_fd = dup(fd);
+ if (fd == -1) {
+ return Failure(ErrnoError("Failed to duplicate file descriptor"));
+ }
+
+ // Set the close-on-exec flag.
+ Try<Nothing> cloexec = os::cloexec(owned_fd);
+ if (cloexec.isError()) {
+ os::close(owned_fd);
+ return Failure(
+ "Failed to set close-on-exec on duplicated file descriptor: " +
+ cloexec.error());
+ }
+
+ // Make the file descriptor non-blocking.
+ Try<Nothing> nonblock = os::nonblock(owned_fd);
+ if (nonblock.isError()) {
+ os::close(owned_fd);
+ return Failure(
+ "Failed to make duplicated file descriptor non-blocking: " +
+ nonblock.error());
+ }
+
// Extend the life-time of 'this' through the execution of the
// lambda in the event loop. Note: The 'self' needs to be explicitly
// captured because we're not using it in the body of the lambda. We
@@ -749,7 +779,7 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
auto self = shared(this);
run_in_event_loop(
- [self, fd, offset, size]() {
+ [self, owned_fd, offset, size]() {
CHECK(__in_event_loop__);
CHECK(self);
@@ -765,12 +795,16 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
}
if (write) {
+ // NOTE: `evbuffer_add_file` will take ownership of the file
+ // descriptor and close it after it has finished reading it.
int result = evbuffer_add_file(
bufferevent_get_output(self->bev),
- fd,
+ owned_fd,
offset,
size);
CHECK_EQ(0, result);
+ } else {
+ os::close(owned_fd);
}
},
DISALLOW_SHORT_CIRCUIT);
[2/4] mesos git commit: Added check in FileEncoder's destructor.
Posted by jo...@apache.org.
Added check in FileEncoder's destructor.
This check will force libprocess to fail fast if a file descriptor
is closed underneath it.
Review: https://reviews.apache.org/r/51511/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f67964d8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f67964d8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f67964d8
Branch: refs/heads/1.0.x
Commit: f67964d8591ae4f8176745a4a3951a1edbfd63ce
Parents: 4c5bdd6
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Tue Sep 13 14:11:50 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Sep 15 09:58:18 2016 -0700
----------------------------------------------------------------------
3rdparty/libprocess/src/encoder.hpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f67964d8/3rdparty/libprocess/src/encoder.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/encoder.hpp b/3rdparty/libprocess/src/encoder.hpp
index 9cd0d3f..af083d1 100644
--- a/3rdparty/libprocess/src/encoder.hpp
+++ b/3rdparty/libprocess/src/encoder.hpp
@@ -256,7 +256,7 @@ public:
virtual ~FileEncoder()
{
- os::close(fd);
+ CHECK_SOME(os::close(fd)) << "Failed to close file descriptor";
}
virtual Kind kind() const
[3/4] mesos git commit: Checked correct file descriptor after call to
`dup`.
Posted by jo...@apache.org.
Checked correct file descriptor after call to `dup`.
Review: https://reviews.apache.org/r/51839/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/54695e5b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/54695e5b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/54695e5b
Branch: refs/heads/1.0.x
Commit: 54695e5bcfa9c238d29710a12f53e0bff39e0002
Parents: f67964d
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Tue Sep 13 11:43:19 2016 +0200
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Sep 15 09:58:18 2016 -0700
----------------------------------------------------------------------
3rdparty/libprocess/src/libevent_ssl_socket.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/54695e5b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index 93a33e5..26eafc1 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -749,7 +749,7 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
// we may use `evbuffer_file_segment_new` and `evbuffer_add_file_segment`
// instead of `evbuffer_add_file`.
int owned_fd = dup(fd);
- if (fd == -1) {
+ if (owned_fd < 0) {
return Failure(ErrnoError("Failed to duplicate file descriptor"));
}
[4/4] mesos git commit: Backported MESOS-6104 and MESOS-6152 to 1.0.x.
Posted by jo...@apache.org.
Backported MESOS-6104 and MESOS-6152 to 1.0.x.
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d6132c58
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d6132c58
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d6132c58
Branch: refs/heads/1.0.x
Commit: d6132c58c121737a67a02e0a563436510b61d5e7
Parents: 54695e5
Author: Joseph Wu <jo...@apache.org>
Authored: Thu Sep 15 10:01:06 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Sep 15 10:01:06 2016 -0700
----------------------------------------------------------------------
CHANGELOG | 2 ++
1 file changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/d6132c58/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 7da6db4..97d0b6a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -7,6 +7,8 @@ All Issues:
* [MESOS-6013] - Use readdir instead of readdir_r.
* [MESOS-6074] - Master check failure if the metrics endpoint is polled soon after it starts.
* [MESOS-6122] - Mesos slave throws systemd errors even when passed a flag to disable systemd.
+ * [MESOS-6104] - Potential FD double close in libevent's implementation of `sendfile`.
+ * [MESOS-6152] - Resource leak in libevent_ssl_socket.cpp.
Release Notes - Mesos - Version 1.0.1