Commit 5e25b795 authored by Jun Cai's avatar Jun Cai Committed by Commit Bot

Mojo: Update ChannelPosix::WriteNoLock() to send limited number of handles at a time

This CL changes ChannelPosix::WriteNoLock() to send limited number
of handles at a time so that it won't overflow the |cmsg_buf| buffer
that is used at mojo::SendmsgWithHandles().

Bug: 935357
Change-Id: I9df239763d5976022564881b8da81acfa6d61f1c
Reviewed-on: https://chromium-review.googlesource.com/c/1489104
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#636287}
parent 9492e003
...@@ -85,10 +85,17 @@ class MessageView { ...@@ -85,10 +85,17 @@ class MessageView {
handles_ = std::move(handles); handles_ = std::move(handles);
} }
size_t num_handles_sent() { return num_handles_sent_; }
void set_num_handles_sent(size_t num_handles_sent) {
num_handles_sent_ = num_handles_sent;
}
private: private:
Channel::MessagePtr message_; Channel::MessagePtr message_;
size_t offset_; size_t offset_;
std::vector<PlatformHandleInTransit> handles_; std::vector<PlatformHandleInTransit> handles_;
size_t num_handles_sent_ = 0;
DISALLOW_COPY_AND_ASSIGN(MessageView); DISALLOW_COPY_AND_ASSIGN(MessageView);
}; };
...@@ -514,17 +521,21 @@ class ChannelPosix : public Channel, ...@@ -514,17 +521,21 @@ class ChannelPosix : public Channel,
return true; return true;
} }
size_t bytes_written = 0; size_t bytes_written = 0;
std::vector<PlatformHandleInTransit> handles = message_view.TakeHandles();
size_t num_handles = handles.size();
size_t handles_written = message_view.num_handles_sent();
do { do {
message_view.advance_data_offset(bytes_written); message_view.advance_data_offset(bytes_written);
ssize_t result; ssize_t result;
std::vector<PlatformHandleInTransit> handles = message_view.TakeHandles(); if (handles_written < num_handles) {
if (!handles.empty()) {
iovec iov = {const_cast<void*>(message_view.data()), iovec iov = {const_cast<void*>(message_view.data()),
message_view.data_num_bytes()}; message_view.data_num_bytes()};
std::vector<base::ScopedFD> fds(handles.size()); size_t num_handles_to_send =
for (size_t i = 0; i < handles.size(); ++i) std::min(num_handles - handles_written, kMaxSendmsgHandles);
fds[i] = handles[i].TakeHandle().TakeFD(); std::vector<base::ScopedFD> fds(num_handles_to_send);
for (size_t i = 0; i < num_handles_to_send; ++i)
fds[i] = handles[i + handles_written].TakeHandle().TakeFD();
// TODO: Handle lots of handles. // TODO: Handle lots of handles.
result = SendmsgWithHandles(socket_.get(), &iov, 1, fds); result = SendmsgWithHandles(socket_.get(), &iov, 1, fds);
if (result >= 0) { if (result >= 0) {
...@@ -549,11 +560,14 @@ class ChannelPosix : public Channel, ...@@ -549,11 +560,14 @@ class ChannelPosix : public Channel,
fds_to_close_.emplace_back(std::move(fd)); fds_to_close_.emplace_back(std::move(fd));
} }
#endif // defined(OS_MACOSX) #endif // defined(OS_MACOSX)
handles_written += num_handles_to_send;
DCHECK_LE(handles_written, num_handles);
message_view.set_num_handles_sent(handles_written);
} else { } else {
// Message transmission failed, so pull the FDs back into |handles| // Message transmission failed, so pull the FDs back into |handles|
// so they can be held by the Message again. // so they can be held by the Message again.
for (size_t i = 0; i < fds.size(); ++i) { for (size_t i = 0; i < fds.size(); ++i) {
handles[i] = handles[i + handles_written] =
PlatformHandleInTransit(PlatformHandle(std::move(fds[i]))); PlatformHandleInTransit(PlatformHandle(std::move(fds[i])));
} }
} }
...@@ -591,7 +605,8 @@ class ChannelPosix : public Channel, ...@@ -591,7 +605,8 @@ class ChannelPosix : public Channel,
} }
bytes_written = static_cast<size_t>(result); bytes_written = static_cast<size_t>(result);
} while (bytes_written < message_view.data_num_bytes()); } while (handles_written < num_handles ||
bytes_written < message_view.data_num_bytes());
return FlushOutgoingMessagesNoLock(); return FlushOutgoingMessagesNoLock();
} }
......
...@@ -73,8 +73,6 @@ constexpr int kSendmsgFlags = 0; ...@@ -73,8 +73,6 @@ constexpr int kSendmsgFlags = 0;
constexpr int kSendmsgFlags = MSG_NOSIGNAL; constexpr int kSendmsgFlags = MSG_NOSIGNAL;
#endif #endif
constexpr size_t kMaxSendmsgHandles = 128;
} // namespace } // namespace
ssize_t SocketWrite(base::PlatformFile socket, ssize_t SocketWrite(base::PlatformFile socket,
...@@ -107,7 +105,7 @@ ssize_t SendmsgWithHandles(base::PlatformFile socket, ...@@ -107,7 +105,7 @@ ssize_t SendmsgWithHandles(base::PlatformFile socket,
DCHECK(iov); DCHECK(iov);
DCHECK_GT(num_iov, 0u); DCHECK_GT(num_iov, 0u);
DCHECK(!descriptors.empty()); DCHECK(!descriptors.empty());
DCHECK_LE(descriptors.size(), kMaxSendmsgHandles); CHECK_LE(descriptors.size(), kMaxSendmsgHandles);
char cmsg_buf[CMSG_SPACE(kMaxSendmsgHandles * sizeof(int))]; char cmsg_buf[CMSG_SPACE(kMaxSendmsgHandles * sizeof(int))];
struct msghdr msg = {}; struct msghdr msg = {};
......
...@@ -18,6 +18,11 @@ struct iovec; // Declared in <sys/uio.h> ...@@ -18,6 +18,11 @@ struct iovec; // Declared in <sys/uio.h>
namespace mojo { namespace mojo {
// There is an upper bound of number of handles on what is supported across
// various OS implementations of sendmsg(). This value was chosen because it
// should be safe across all supported platforms.
constexpr size_t kMaxSendmsgHandles = 128;
// NOTE: Functions declared here really don't belong in Mojo, but they exist to // NOTE: Functions declared here really don't belong in Mojo, but they exist to
// support code which used to rely on internal parts of the Mojo implementation // support code which used to rely on internal parts of the Mojo implementation
// and there wasn't a much better home for them. Consider moving them to // and there wasn't a much better home for them. Consider moving them to
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment