From ff615aa95005294f00fce7ba7485a99a76dadbef Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Tue, 20 Aug 2019 00:01:21 +0000 Subject: [PATCH] network: fix misaligned access in setsockopt(). Fixes #7968. Signed-off-by: Piotr Sikora --- include/envoy/network/listen_socket.h | 2 +- source/common/network/socket_option_impl.cc | 4 ++-- source/common/network/socket_option_impl.h | 6 +++--- test/common/network/socket_option_impl_test.cc | 2 +- test/common/network/socket_option_test.h | 2 +- .../filters/http/original_src/original_src_test.cc | 3 ++- .../filters/listener/original_src/original_src_test.cc | 3 ++- 7 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/envoy/network/listen_socket.h b/include/envoy/network/listen_socket.h index 4b39a71eda69..a53ed5c9c398 100644 --- a/include/envoy/network/listen_socket.h +++ b/include/envoy/network/listen_socket.h @@ -111,7 +111,7 @@ class Socket { */ struct Details { SocketOptionName name_; - std::string value_; ///< Binary string representation of an option's value. + std::vector value_; ///< Binary string representation of an option's value. bool operator==(const Details& other) const { return name_ == other.name_ && value_ == other.value_; diff --git a/source/common/network/socket_option_impl.cc b/source/common/network/socket_option_impl.cc index 65b364810f21..89014c649d54 100644 --- a/source/common/network/socket_option_impl.cc +++ b/source/common/network/socket_option_impl.cc @@ -47,14 +47,14 @@ bool SocketOptionImpl::isSupported() const { return optname_.has_value(); } Api::SysCallIntResult SocketOptionImpl::setSocketOption(Socket& socket, const Network::SocketOptionName& optname, - const absl::string_view value) { + const std::vector& value) { if (!optname.has_value()) { return {-1, ENOTSUP}; } auto& os_syscalls = Api::OsSysCallsSingleton::get(); return os_syscalls.setsockopt(socket.ioHandle().fd(), optname.level(), optname.option(), - value.data(), value.size()); + static_cast(value.data()), value.size()); } } // namespace Network diff --git a/source/common/network/socket_option_impl.h b/source/common/network/socket_option_impl.h index 59931506323c..9eedaf6c6e92 100644 --- a/source/common/network/socket_option_impl.h +++ b/source/common/network/socket_option_impl.h @@ -103,7 +103,7 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable& value); private: const envoy::api::v2::core::SocketOption::SocketState in_state_; const Network::SocketOptionName optname_; - const std::string value_; + const std::vector value_; }; } // namespace Network diff --git a/test/common/network/socket_option_impl_test.cc b/test/common/network/socket_option_impl_test.cc index 8d9e55ae2d19..01231a64b986 100644 --- a/test/common/network/socket_option_impl_test.cc +++ b/test/common/network/socket_option_impl_test.cc @@ -7,7 +7,7 @@ namespace { class SocketOptionImplTest : public SocketOptionTest {}; TEST_F(SocketOptionImplTest, BadFd) { - absl::string_view zero("\0\0\0\0", 4); + std::vector zero{'\0', '\0', '\0', '\0'}; Api::SysCallIntResult result = SocketOptionImpl::setSocketOption(socket_, {}, zero); EXPECT_EQ(-1, result.rc_); EXPECT_EQ(ENOTSUP, result.errno_); diff --git a/test/common/network/socket_option_test.h b/test/common/network/socket_option_test.h index 2fe7a59d2c7e..078e5575fae8 100644 --- a/test/common/network/socket_option_test.h +++ b/test/common/network/socket_option_test.h @@ -73,7 +73,7 @@ class SocketOptionTest : public testing::Test { Socket::Option::Details expected_info; expected_info.name_ = name; - expected_info.value_ = std::string(value_as_bstr); + expected_info.value_ = std::vector(value_as_bstr.begin(), value_as_bstr.end()); return expected_info; } diff --git a/test/extensions/filters/http/original_src/original_src_test.cc b/test/extensions/filters/http/original_src/original_src_test.cc index c166824bfab3..a5ac87f9366c 100644 --- a/test/extensions/filters/http/original_src/original_src_test.cc +++ b/test/extensions/filters/http/original_src/original_src_test.cc @@ -170,7 +170,8 @@ TEST_F(OriginalSrcHttpTest, FilterAddsMarkOption) { ASSERT_TRUE(mark_option.has_value()); uint32_t value = 1234; absl::string_view value_as_bstr(reinterpret_cast(&value), sizeof(value)); - EXPECT_EQ(value_as_bstr, mark_option->value_); + std::vector value_as_vector(value_as_bstr.begin(), value_as_bstr.end()); + EXPECT_EQ(value_as_vector, mark_option->value_); } TEST_F(OriginalSrcHttpTest, Mark0NotAdded) { diff --git a/test/extensions/filters/listener/original_src/original_src_test.cc b/test/extensions/filters/listener/original_src/original_src_test.cc index ffed37e4855c..ed6725739cdf 100644 --- a/test/extensions/filters/listener/original_src/original_src_test.cc +++ b/test/extensions/filters/listener/original_src/original_src_test.cc @@ -156,7 +156,8 @@ TEST_F(OriginalSrcTest, filterAddsMarkOption) { ASSERT_TRUE(mark_option.has_value()); uint32_t value = 1234; absl::string_view value_as_bstr(reinterpret_cast(&value), sizeof(value)); - EXPECT_EQ(value_as_bstr, mark_option->value_); + std::vector value_as_vector(value_as_bstr.begin(), value_as_bstr.end()); + EXPECT_EQ(value_as_vector, mark_option->value_); } TEST_F(OriginalSrcTest, Mark0NotAdded) {