From 14f9500a35390e2e0252ccc1c03c9c850444e2b6 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 7 Jan 2019 19:16:21 -0800 Subject: [PATCH] base: add helpers for sending/receiving file descriptors. Almost all of the uses of cmsg(3) in our source tree are wrong in at least one of the following ways: - not aligning the cmsg buffer - leaking fds if more fds are received than expected - blindly dereferencing CMSG_DATA without checking the header - using CMSG_SPACE instead of CMSG_LEN for .cmsg_len - using CMSG_LEN instead of CMSG_SPACE for .msg_controllen - using a length specified in number of fds instead of bytes Implement wrapper functions that implement this dumpster fire of an API correctly. Bug: http://b/122047630 Test: libbase_test32 Change-Id: I6ac34d67bbbf1bfa9727ab598248fc178ea19df9 --- base/Android.bp | 5 + base/cmsg.cpp | 151 ++++++++++++++++++ base/cmsg_test.cpp | 202 ++++++++++++++++++++++++ base/include/android-base/cmsg.h | 106 +++++++++++++ base/include/android-base/collections.h | 60 +++++++ 5 files changed, 524 insertions(+) create mode 100644 base/cmsg.cpp create mode 100644 base/cmsg_test.cpp create mode 100644 base/include/android-base/cmsg.h create mode 100644 base/include/android-base/collections.h diff --git a/base/Android.bp b/base/Android.bp index b0181aa6a..38f301a1b 100644 --- a/base/Android.bp +++ b/base/Android.bp @@ -46,6 +46,7 @@ cc_defaults { defaults: ["libbase_cflags_defaults"], srcs: [ "chrono_utils.cpp", + "cmsg.cpp", "file.cpp", "logging.cpp", "mapped_file.cpp", @@ -85,6 +86,9 @@ cc_defaults { "errors_windows.cpp", "utf8.cpp", ], + exclude_srcs: [ + "cmsg.cpp", + ], enabled: true, }, }, @@ -121,6 +125,7 @@ cc_test { defaults: ["libbase_cflags_defaults"], host_supported: true, srcs: [ + "cmsg_test.cpp", "endian_test.cpp", "errors_test.cpp", "file_test.cpp", diff --git a/base/cmsg.cpp b/base/cmsg.cpp new file mode 100644 index 000000000..1879d4473 --- /dev/null +++ b/base/cmsg.cpp @@ -0,0 +1,151 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include + +#include + +namespace android { +namespace base { + +ssize_t SendFileDescriptorVector(int sockfd, const void* data, size_t len, + const std::vector& fds) { + size_t cmsg_space = CMSG_SPACE(sizeof(int) * fds.size()); + size_t cmsg_len = CMSG_LEN(sizeof(int) * fds.size()); + if (cmsg_space >= PAGE_SIZE) { + errno = ENOMEM; + return -1; + } + + alignas(struct cmsghdr) char cmsg_buf[cmsg_space]; + iovec iov = {.iov_base = const_cast(data), .iov_len = len}; + msghdr msg = { + .msg_name = nullptr, + .msg_namelen = 0, + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = cmsg_buf, + .msg_controllen = cmsg_space, + .msg_flags = 0, + }; + + struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = cmsg_len; + + int* cmsg_fds = reinterpret_cast(CMSG_DATA(cmsg)); + for (size_t i = 0; i < fds.size(); ++i) { + cmsg_fds[i] = fds[i]; + } + + return TEMP_FAILURE_RETRY(sendmsg(sockfd, &msg, MSG_NOSIGNAL)); +} + +ssize_t ReceiveFileDescriptorVector(int sockfd, void* data, size_t len, size_t max_fds, + std::vector* fds) { + fds->clear(); + + size_t cmsg_space = CMSG_SPACE(sizeof(int) * max_fds); + if (cmsg_space >= PAGE_SIZE) { + errno = ENOMEM; + return -1; + } + + alignas(struct cmsghdr) char cmsg_buf[cmsg_space]; + iovec iov = {.iov_base = const_cast(data), .iov_len = len}; + msghdr msg = { + .msg_name = nullptr, + .msg_namelen = 0, + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = cmsg_buf, + .msg_controllen = cmsg_space, + .msg_flags = 0, + }; + + ssize_t rc = TEMP_FAILURE_RETRY( + recvmsg(sockfd, &msg, MSG_TRUNC | MSG_CTRUNC | MSG_CMSG_CLOEXEC | MSG_NOSIGNAL)); + if (rc == -1) { + return -1; + } + + int error = 0; + if ((msg.msg_flags & MSG_TRUNC)) { + LOG(ERROR) << "message was truncated when receiving file descriptors"; + error = EMSGSIZE; + } else if ((msg.msg_flags & MSG_CTRUNC)) { + LOG(ERROR) << "control message was truncated when receiving file descriptors"; + error = EMSGSIZE; + } + + std::vector received_fds; + struct cmsghdr* cmsg; + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != nullptr; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { + LOG(ERROR) << "received unexpected cmsg: [" << cmsg->cmsg_level << ", " << cmsg->cmsg_type + << "]"; + error = EBADMSG; + continue; + } + + // There isn't a macro that does the inverse of CMSG_LEN, so hack around it ourselves, with + // some static asserts to ensure that CMSG_LEN behaves as we expect. + static_assert(CMSG_LEN(0) + 1 * sizeof(int) == CMSG_LEN(1 * sizeof(int))); + static_assert(CMSG_LEN(0) + 2 * sizeof(int) == CMSG_LEN(2 * sizeof(int))); + static_assert(CMSG_LEN(0) + 3 * sizeof(int) == CMSG_LEN(3 * sizeof(int))); + static_assert(CMSG_LEN(0) + 4 * sizeof(int) == CMSG_LEN(4 * sizeof(int))); + + if (cmsg->cmsg_len % sizeof(int) != 0) { + LOG(FATAL) << "cmsg_len(" << cmsg->cmsg_len << ") not aligned to sizeof(int)"; + } else if (cmsg->cmsg_len <= CMSG_LEN(0)) { + LOG(FATAL) << "cmsg_len(" << cmsg->cmsg_len << ") not long enough to hold any data"; + } + + int* cmsg_fds = reinterpret_cast(CMSG_DATA(cmsg)); + size_t cmsg_fdcount = static_cast(cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); + for (size_t i = 0; i < cmsg_fdcount; ++i) { + received_fds.emplace_back(cmsg_fds[i]); + } + } + + if (error != 0) { + errno = error; + return -1; + } + + if (received_fds.size() > max_fds) { + LOG(ERROR) << "received too many file descriptors, expected " << fds->size() << ", received " + << received_fds.size(); + errno = EMSGSIZE; + return -1; + } + + *fds = std::move(received_fds); + return rc; +} + +} // namespace base +} // namespace android diff --git a/base/cmsg_test.cpp b/base/cmsg_test.cpp new file mode 100644 index 000000000..9ee5c8253 --- /dev/null +++ b/base/cmsg_test.cpp @@ -0,0 +1,202 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include +#include + +#if !defined(_WIN32) + +using android::base::ReceiveFileDescriptors; +using android::base::SendFileDescriptors; +using android::base::unique_fd; + +static ino_t GetInode(int fd) { + struct stat st; + if (fstat(fd, &st) != 0) { + PLOG(FATAL) << "fstat failed"; + } + + return st.st_ino; +} + +struct CmsgTest : ::testing::TestWithParam { + bool Seqpacket() { return GetParam(); } + + void SetUp() override { + ASSERT_TRUE( + android::base::Socketpair(Seqpacket() ? SOCK_SEQPACKET : SOCK_STREAM, &send, &recv)); + int dup1 = dup(tmp1.fd); + ASSERT_NE(-1, dup1); + int dup2 = dup(tmp2.fd); + ASSERT_NE(-1, dup2); + + fd1.reset(dup1); + fd2.reset(dup2); + + ino1 = GetInode(dup1); + ino2 = GetInode(dup2); + } + + unique_fd send; + unique_fd recv; + + TemporaryFile tmp1; + TemporaryFile tmp2; + + unique_fd fd1; + unique_fd fd2; + + ino_t ino1; + ino_t ino2; +}; + +TEST_P(CmsgTest, smoke) { + ASSERT_EQ(1, SendFileDescriptors(send.get(), "x", 1, fd1.get())); + + char buf[2]; + unique_fd received; + ASSERT_EQ(1, ReceiveFileDescriptors(recv.get(), buf, 2, &received)); + ASSERT_EQ('x', buf[0]); + ASSERT_NE(-1, received.get()); + + ASSERT_EQ(ino1, GetInode(received.get())); +} + +TEST_P(CmsgTest, msg_trunc) { + ASSERT_EQ(2, SendFileDescriptors(send.get(), "ab", 2, fd1.get(), fd2.get())); + + char buf[2]; + unique_fd received1, received2; + + ssize_t rc = ReceiveFileDescriptors(recv.get(), buf, 1, &received1, &received2); + if (Seqpacket()) { + ASSERT_EQ(-1, rc); + ASSERT_EQ(EMSGSIZE, errno); + ASSERT_EQ(-1, received1.get()); + ASSERT_EQ(-1, received2.get()); + } else { + ASSERT_EQ(1, rc); + ASSERT_NE(-1, received1.get()); + ASSERT_NE(-1, received2.get()); + ASSERT_EQ(ino1, GetInode(received1.get())); + ASSERT_EQ(ino2, GetInode(received2.get())); + ASSERT_EQ(1, read(recv.get(), buf, 2)); + } +} + +TEST_P(CmsgTest, msg_ctrunc) { + ASSERT_EQ(1, SendFileDescriptors(send.get(), "a", 1, fd1.get(), fd2.get())); + + char buf[2]; + unique_fd received; + ASSERT_EQ(-1, ReceiveFileDescriptors(recv.get(), buf, 1, &received)); + ASSERT_EQ(EMSGSIZE, errno); + ASSERT_EQ(-1, received.get()); +} + +TEST_P(CmsgTest, peek) { + ASSERT_EQ(1, SendFileDescriptors(send.get(), "a", 1, fd1.get())); + + char buf[2]; + ASSERT_EQ(1, ::recv(recv.get(), buf, sizeof(buf), MSG_PEEK)); + ASSERT_EQ('a', buf[0]); + + unique_fd received; + ASSERT_EQ(1, ReceiveFileDescriptors(recv.get(), buf, 1, &received)); + ASSERT_EQ(ino1, GetInode(received.get())); +} + +TEST_P(CmsgTest, stream_fd_association) { + if (Seqpacket()) { + return; + } + + // fds are associated with the first byte of the write. + ASSERT_EQ(1, TEMP_FAILURE_RETRY(write(send.get(), "a", 1))); + ASSERT_EQ(2, SendFileDescriptors(send.get(), "bc", 2, fd1.get())); + ASSERT_EQ(1, SendFileDescriptors(send.get(), "d", 1, fd2.get())); + char buf[2]; + ASSERT_EQ(2, TEMP_FAILURE_RETRY(read(recv.get(), buf, 2))); + ASSERT_EQ(0, memcmp(buf, "ab", 2)); + + std::vector received1; + ssize_t rc = ReceiveFileDescriptorVector(recv.get(), buf, 1, 1, &received1); + ASSERT_EQ(1, rc); + ASSERT_EQ('c', buf[0]); + ASSERT_TRUE(received1.empty()); + + unique_fd received2; + rc = ReceiveFileDescriptors(recv.get(), buf, 1, &received2); + ASSERT_EQ(1, rc); + ASSERT_EQ('d', buf[0]); + ASSERT_EQ(ino2, GetInode(received2.get())); +} + +TEST_P(CmsgTest, multiple_fd_ordering) { + ASSERT_EQ(1, SendFileDescriptors(send.get(), "a", 1, fd1.get(), fd2.get())); + + char buf[2]; + unique_fd received1, received2; + ASSERT_EQ(1, ReceiveFileDescriptors(recv.get(), buf, 1, &received1, &received2)); + + ASSERT_NE(-1, received1.get()); + ASSERT_NE(-1, received2.get()); + + ASSERT_EQ(ino1, GetInode(received1.get())); + ASSERT_EQ(ino2, GetInode(received2.get())); +} + +TEST_P(CmsgTest, separate_fd_ordering) { + ASSERT_EQ(1, SendFileDescriptors(send.get(), "a", 1, fd1.get())); + ASSERT_EQ(1, SendFileDescriptors(send.get(), "b", 1, fd2.get())); + + char buf[2]; + unique_fd received1, received2; + ASSERT_EQ(1, ReceiveFileDescriptors(recv.get(), buf, 1, &received1)); + ASSERT_EQ(1, ReceiveFileDescriptors(recv.get(), buf, 1, &received2)); + + ASSERT_NE(-1, received1.get()); + ASSERT_NE(-1, received2.get()); + + ASSERT_EQ(ino1, GetInode(received1.get())); + ASSERT_EQ(ino2, GetInode(received2.get())); +} + +TEST_P(CmsgTest, separate_fds_no_coalescing) { + unique_fd sent1(dup(tmp1.fd)); + unique_fd sent2(dup(tmp2.fd)); + + ASSERT_EQ(1, SendFileDescriptors(send.get(), "", 1, fd1.get())); + ASSERT_EQ(1, SendFileDescriptors(send.get(), "", 1, fd2.get())); + + char buf[2]; + std::vector received; + ASSERT_EQ(1, ReceiveFileDescriptorVector(recv.get(), buf, 2, 2, &received)); + ASSERT_EQ(1U, received.size()); + ASSERT_EQ(ino1, GetInode(received[0].get())); + + ASSERT_EQ(1, ReceiveFileDescriptorVector(recv.get(), buf, 2, 2, &received)); + ASSERT_EQ(1U, received.size()); + ASSERT_EQ(ino2, GetInode(received[0].get())); +} + +INSTANTIATE_TEST_CASE_P(CmsgTest, CmsgTest, testing::Bool()); + +#endif diff --git a/base/include/android-base/cmsg.h b/base/include/android-base/cmsg.h new file mode 100644 index 000000000..7f93ddc3b --- /dev/null +++ b/base/include/android-base/cmsg.h @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include +#include + +#include +#include +#include + +namespace android { +namespace base { + +#if !defined(_WIN32) + +// Helpers for sending and receiving file descriptors across Unix domain sockets. +// +// The cmsg(3) API is very hard to get right, with multiple landmines that can +// lead to death. Almost all of the uses of cmsg in Android make at least one of +// the following mistakes: +// +// - not aligning the cmsg buffer +// - leaking fds if more fds are received than expected +// - blindly dereferencing CMSG_DATA without checking the header +// - using CMSG_SPACE instead of CMSG_LEN for .cmsg_len +// - using CMSG_LEN instead of CMSG_SPACE for .msg_controllen +// - using a length specified in number of fds instead of bytes +// +// These functions wrap the hard-to-use cmsg API with an easier to use abstraction. + +// Send file descriptors across a Unix domain socket. +// +// Note that the write can return short if the socket type is SOCK_STREAM. When +// this happens, file descriptors are still sent to the other end, but with +// truncated data. For this reason, using SOCK_SEQPACKET or SOCK_DGRAM is recommended. +ssize_t SendFileDescriptorVector(int sock, const void* data, size_t len, + const std::vector& fds); + +// Receive file descriptors from a Unix domain socket. +// +// If more FDs (or bytes, for datagram sockets) are received than expected, +// -1 is returned with errno set to EMSGSIZE, and all received FDs are thrown away. +ssize_t ReceiveFileDescriptorVector(int sock, void* data, size_t len, size_t max_fds, + std::vector* fds); + +// Helper for SendFileDescriptorVector that constructs a std::vector for you, e.g.: +// SendFileDescriptors(sock, "foo", 3, std::move(fd1), std::move(fd2)) +template +ssize_t SendFileDescriptors(int sock, const void* data, size_t len, Args&&... sent_fds) { + // Do not allow implicit conversion to int: people might try to do something along the lines of: + // SendFileDescriptors(..., std::move(a_unique_fd)) + // and be surprised when the unique_fd isn't closed afterwards. + AssertType(std::forward(sent_fds)...); + std::vector fds; + Append(fds, std::forward(sent_fds)...); + return SendFileDescriptorVector(sock, data, len, fds); +} + +// Helper for ReceiveFileDescriptorVector that receives an exact number of file descriptors. +// If more file descriptors are received than requested, -1 is returned with errno set to EMSGSIZE. +// If fewer file descriptors are received than requested, -1 is returned with errno set to ENOMSG. +// In both cases, all arguments are cleared and any received FDs are thrown away. +template +ssize_t ReceiveFileDescriptors(int sock, void* data, size_t len, Args&&... received_fds) { + std::vector fds; + Append(fds, std::forward(received_fds)...); + + std::vector result; + ssize_t rc = ReceiveFileDescriptorVector(sock, data, len, fds.size(), &result); + if (rc == -1 || result.size() != fds.size()) { + int err = rc == -1 ? errno : ENOMSG; + for (unique_fd* fd : fds) { + fd->reset(); + } + errno = err; + return -1; + } + + for (size_t i = 0; i < fds.size(); ++i) { + *fds[i] = std::move(result[i]); + } + return rc; +} + +#endif + +} // namespace base +} // namespace android diff --git a/base/include/android-base/collections.h b/base/include/android-base/collections.h new file mode 100644 index 000000000..be0683ab9 --- /dev/null +++ b/base/include/android-base/collections.h @@ -0,0 +1,60 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace android { +namespace base { + +// Helpers for converting a variadic template parameter pack to a homogeneous collection. +// Parameters must be implictly convertible to the contained type (including via move/copy ctors). +// +// Use as follows: +// +// template +// std::vector CreateVector(Args&&... args) { +// std::vector result; +// Append(result, std::forward(args)...); +// return result; +// } +template +void Append(CollectionType& collection, T&& arg) { + collection.push_back(std::forward(arg)); +} + +template +void Append(CollectionType& collection, T&& arg, Args&&... args) { + collection.push_back(std::forward(arg)); + return Append(collection, std::forward(args)...); +} + +// Assert that all of the arguments in a variadic template parameter pack are of a given type +// after std::decay. +template +void AssertType(Arg&&) { + static_assert(std::is_same::type>::value); +} + +template +void AssertType(Arg&&, Args&&... args) { + static_assert(std::is_same::type>::value); + AssertType(std::forward(args)...); +} + +} // namespace base +} // namespace android