Bug 1734735 - Part 3: Generalize IPC message footer writing code, r=handyman

Differential Revision: https://phabricator.services.mozilla.com/D128212
This commit is contained in:
Nika Layzell 2021-11-23 16:15:21 +00:00
parent 972727feaa
commit 16985051ce
4 changed files with 62 additions and 83 deletions

View File

@ -651,65 +651,46 @@ bool Channel::ChannelImpl::AcceptHandles(Message& msg) {
return false;
}
// Seek to the start of our handle payload.
mozilla::CheckedInt<uint32_t> handles_payload_size(num_handles);
handles_payload_size *= sizeof(uint32_t);
if (!handles_payload_size.isValid() ||
handles_payload_size.value() > msg.header()->payload_size) {
CHROMIUM_LOG(ERROR) << "invalid handle count " << num_handles
<< " or payload size: " << msg.header()->payload_size;
return false;
}
uint32_t handles_offset =
msg.header()->payload_size - handles_payload_size.value();
PickleIterator handles_start{msg};
if (!msg.IgnoreBytes(&handles_start, handles_offset)) {
CHROMIUM_LOG(ERROR) << "IgnoreBytes failed";
// Read in the payload from the footer, truncating the message.
nsTArray<uint32_t> payload;
payload.AppendElements(num_handles);
if (!msg.ReadFooter(payload.Elements(), num_handles * sizeof(uint32_t),
/* truncate */ true)) {
CHROMIUM_LOG(ERROR) << "failed to read handle payload from message";
return false;
}
msg.header()->num_handles = 0;
// Read in the handles themselves, transferring ownership as required.
nsTArray<mozilla::UniqueFileHandle> handles;
{
PickleIterator iter{handles_start};
for (uint32_t i = 0; i < num_handles; ++i) {
uint32_t handleValue;
if (!msg.ReadUInt32(&iter, &handleValue)) {
CHROMIUM_LOG(ERROR) << "failed to read handle value";
nsTArray<mozilla::UniqueFileHandle> handles(num_handles);
for (uint32_t handleValue : payload) {
HANDLE handle = Uint32ToHandle(handleValue);
// If we're the privileged process, the remote process will have leaked
// the sent handles in its local address space, and be relying on us to
// duplicate them, otherwise the remote privileged side will have
// transferred the handles to us already.
if (privileged_) {
if (other_process_ == INVALID_HANDLE_VALUE) {
CHROMIUM_LOG(ERROR) << "other_process_ is invalid in AcceptHandles";
return false;
}
HANDLE handle = Uint32ToHandle(handleValue);
// If we're the privileged process, the remote process will have leaked
// the sent handles in its local address space, and be relying on us to
// duplicate them, otherwise the remote privileged side will have
// transferred the handles to us already.
if (privileged_) {
if (other_process_ == INVALID_HANDLE_VALUE) {
CHROMIUM_LOG(ERROR) << "other_process_ is invalid in AcceptHandles";
return false;
}
if (!::DuplicateHandle(
other_process_, handle, GetCurrentProcess(), &handle, 0, FALSE,
DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) {
CHROMIUM_LOG(ERROR) << "DuplicateHandle failed for handle " << handle
<< " in AcceptHandles";
return false;
}
if (!::DuplicateHandle(other_process_, handle, GetCurrentProcess(),
&handle, 0, FALSE,
DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) {
CHROMIUM_LOG(ERROR) << "DuplicateHandle failed for handle " << handle
<< " in AcceptHandles";
return false;
}
// The handle is directly owned by this process now, and can be added to
// our `handles` array.
handles.AppendElement(mozilla::UniqueFileHandle(handle));
}
// The handle is directly owned by this process now, and can be added to
// our `handles` array.
handles.AppendElement(mozilla::UniqueFileHandle(handle));
}
// We're done with the handle footer, truncate the message at that point.
msg.Truncate(&handles_start);
msg.SetAttachedFileHandles(std::move(handles));
msg.header()->num_handles = 0;
MOZ_ASSERT(msg.header()->payload_size == handles_offset);
MOZ_ASSERT(msg.num_handles() == num_handles);
return true;
}
@ -732,7 +713,7 @@ bool Channel::ChannelImpl::TransferHandles(Message& msg) {
uint32_t handles_offset = msg.header()->payload_size;
#endif
// Write handles from `attached_handles_` into the message payload.
nsTArray<uint32_t> payload(num_handles);
for (uint32_t i = 0; i < num_handles; ++i) {
// Release ownership of the handle. It'll be cloned when the parent process
// transfers it with DuplicateHandle either in this process or the remote
@ -756,14 +737,13 @@ bool Channel::ChannelImpl::TransferHandles(Message& msg) {
}
}
if (!msg.WriteUInt32(HandleToUint32(handle))) {
CHROMIUM_LOG(ERROR) << "failed to write handle value " << handle;
return false;
}
payload.AppendElement(HandleToUint32(handle));
}
msg.attached_handles_.Clear();
msg.WriteFooter(payload.Elements(), payload.Length() * sizeof(uint32_t));
msg.header()->num_handles = num_handles;
MOZ_ASSERT(msg.header()->payload_size ==
handles_offset + (sizeof(uint32_t) * num_handles),
"Unexpected number of bytes written for handles footer?");

View File

@ -44,7 +44,7 @@ Message::Message(int32_t routing_id, msgid_t type, uint32_t segment_capacity,
#if defined(OS_MACOSX)
header()->cookie = 0;
#endif
header()->footer_offset = -1;
header()->event_footer_size = 0;
if (recordWriteLatency) {
create_time_ = mozilla::TimeStamp::Now();
}
@ -95,54 +95,41 @@ Message& Message::operator=(Message&& other) {
}
void Message::WriteFooter(const void* data, uint32_t data_len) {
MOZ_ASSERT(header()->footer_offset < 0, "Already wrote a footer!");
if (data_len == 0) {
return;
}
// Record the start of the footer.
header()->footer_offset = header()->payload_size;
WriteBytes(data, data_len);
}
bool Message::ReadFooter(void* buffer, uint32_t buffer_len, bool truncate) {
MOZ_ASSERT(buffer_len == FooterSize());
MOZ_ASSERT(header()->footer_offset <= int64_t(header()->payload_size));
if (buffer_len == 0) {
return true;
}
// FIXME: This is a really inefficient way to seek to the end of the message
// for sufficiently large messages.
PickleIterator footer_iter(*this);
if (NS_WARN_IF(!IgnoreBytes(&footer_iter, header()->footer_offset))) {
if (NS_WARN_IF(AlignInt(header()->payload_size) != header()->payload_size) ||
NS_WARN_IF(AlignInt(buffer_len) > header()->payload_size)) {
return false;
}
// Seek to the start of the footer, and read it in. We read in with a
// duplicate of the iterator so we can use it to truncate later.
uint32_t offset = header()->payload_size - AlignInt(buffer_len);
PickleIterator footer_iter(*this);
if (NS_WARN_IF(!IgnoreBytes(&footer_iter, offset))) {
return false;
}
// Use a copy of the footer iterator for reading bytes so that we can use the
// previous iterator to truncate the message if requested.
PickleIterator read_iter(footer_iter);
bool ok = ReadBytesInto(&read_iter, buffer, buffer_len);
// If requested, truncate the buffer to the start of the footer, and clear our
// footer offset back to `-1`.
// If requested, truncate the buffer to the start of the footer.
if (truncate) {
header()->footer_offset = -1;
Truncate(&footer_iter);
}
return ok;
}
uint32_t Message::FooterSize() const {
if (header()->footer_offset >= 0 &&
uint32_t(header()->footer_offset) < header()->payload_size) {
return header()->payload_size - header()->footer_offset;
}
return 0;
}
bool Message::WriteFileHandle(mozilla::UniqueFileHandle handle) {
uint32_t handle_index = attached_handles_.Length();
WriteUInt32(handle_index);

View File

@ -292,10 +292,20 @@ class Message : public mojo::core::ports::UserMessage, public Pickle {
size_t GetSizeIfSerialized() const override { return size(); }
bool WillBeRoutedExternally(mojo::core::ports::UserMessageEvent&) override;
// Write the given footer bytes to the end of the current message. The
// footer's `data_len` will be padded to a multiple of 4 bytes.
void WriteFooter(const void* data, uint32_t data_len);
// Read a footer written with `WriteFooter` from the end of the message, given
// a buffer and the length of the footer. If `truncate` is true, the message
// will be truncated, removing the footer.
[[nodiscard]] bool ReadFooter(void* buffer, uint32_t buffer_len,
bool truncate);
uint32_t FooterSize() const;
uint32_t event_footer_size() const { return header()->event_footer_size; }
void set_event_footer_size(uint32_t size) {
header()->event_footer_size = size;
}
// Used for async messages with no parameters.
static void Log(const Message* msg, std::wstring* l) {}
@ -372,8 +382,8 @@ class Message : public mojo::core::ports::UserMessage, public Pickle {
uint32_t interrupt_local_stack_depth;
// Sequence number
int32_t seqno;
// Offset of the message's footer in the payload, or -1 if invalid.
int32_t footer_offset;
// Size of the message's event footer
uint32_t event_footer_size;
};
Header* header() { return headerT<Header>(); }

View File

@ -197,12 +197,13 @@ auto NodeController::SerializeEventMessage(UniquePtr<Event> aEvent,
}
message->WriteFooter(buffer.begin(), buffer.length());
message->set_event_footer_size(buffer.length());
#ifdef DEBUG
// Debug-assert that we can read the same data back out of the buffer.
MOZ_ASSERT(message->FooterSize() == length);
MOZ_ASSERT(message->event_footer_size() == length);
Vector<char, 256, InfallibleAllocPolicy> buffer2;
(void)buffer2.initLengthUninitialized(message->FooterSize());
(void)buffer2.initLengthUninitialized(message->event_footer_size());
MOZ_ASSERT(message->ReadFooter(buffer2.begin(), buffer2.length(),
/* truncate */ false));
MOZ_ASSERT(!memcmp(buffer2.begin(), buffer.begin(), buffer.length()));
@ -220,7 +221,7 @@ auto NodeController::DeserializeEventMessage(UniquePtr<IPC::Message> aMessage,
}
Vector<char, 256, InfallibleAllocPolicy> buffer;
(void)buffer.initLengthUninitialized(aMessage->FooterSize());
(void)buffer.initLengthUninitialized(aMessage->event_footer_size());
// Truncate the message when reading the footer, so that the extra footer data
// is no longer present in the message. This allows future code to eventually
// send the same `IPC::Message` to another process.
@ -230,6 +231,7 @@ auto NodeController::DeserializeEventMessage(UniquePtr<IPC::Message> aMessage,
aMessage->name());
return nullptr;
}
aMessage->set_event_footer_size(0);
UniquePtr<Event> event;
if (aRelayTarget) {