Fix the alignement in optval of setsockopt when compiled with libc++. (…

…envoyproxy#7958)

Description:
libc++ std::string may inline the data which results the memory is not
aligned to `void*`. Use vector instead to store the optval.

Detected by UBSAN with libc++ config. Preparation for envoyproxy#4251

Risk Level: Low
Testing: unittest locally
Docs Changes: N/A
Release Notes: N/A
Fixes envoyproxy#7968

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>

source/common/network/socket_option_impl.cc

@@ -14,7 +14,7 @@ bool SocketOptionImpl::setOption(Socket& socket,
                                  envoy::api::v2::core::SocketOption::SocketState state) const {
   if (in_state_ == state) {
     const Api::SysCallIntResult result =
-        SocketOptionImpl::setSocketOption(socket, optname_, value_);
+        SocketOptionImpl::setSocketOption(socket, optname_, value_.data(), value_.size());
     if (result.rc_ != 0) {
       ENVOY_LOG(warn, "Setting option on socket failed: {}", strerror(result.errno_));
       return false;
@@ -32,22 +32,22 @@ SocketOptionImpl::getOptionDetails(const Socket&,
 
   Socket::Option::Details info;
   info.name_ = optname_;
-  info.value_ = value_;
-  return absl::optional<Option::Details>(std::move(info));
+  info.value_ = {value_.begin(), value_.end()};
+  return absl::make_optional(std::move(info));
 }
 
 bool SocketOptionImpl::isSupported() const { return optname_.has_value(); }
 
 Api::SysCallIntResult SocketOptionImpl::setSocketOption(Socket& socket,
                                                         Network::SocketOptionName optname,
-                                                        const absl::string_view value) {
+                                                        const void* value, size_t size) {
 
   if (!optname.has_value()) {
     return {-1, ENOTSUP};
   }
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   return os_syscalls.setsockopt(socket.ioHandle().fd(), optname.value().first,
-                                optname.value().second, value.data(), value.size());
+                                optname.value().second, value, size);
 }
 
 } // namespace Network

source/common/network/socket_option_impl.h

@@ -7,6 +7,7 @@
 #include "envoy/api/os_sys_calls.h"
 #include "envoy/network/listen_socket.h"
 
+#include "common/common/assert.h"
 #include "common/common/logger.h"
 
 namespace Envoy {
@@ -91,7 +92,9 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
 
   SocketOptionImpl(envoy::api::v2::core::SocketOption::SocketState in_state,
                    Network::SocketOptionName optname, absl::string_view value)
-      : in_state_(in_state), optname_(optname), value_(value) {}
+      : in_state_(in_state), optname_(optname), value_(value.begin(), value.end()) {
+    ASSERT(reinterpret_cast<uintptr_t>(value_.data()) % alignof(void*) == 0);
+  }
 
   // Socket::Option
   bool setOption(Socket& socket,
@@ -111,16 +114,19 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
    * @param socket the socket on which to apply the option.
    * @param optname the option name.
    * @param value the option value.
+   * @param size the option value size.
    * @return a Api::SysCallIntResult with rc_ = 0 for success and rc = -1 for failure. If the call
    * is successful, errno_ shouldn't be used.
    */
   static Api::SysCallIntResult setSocketOption(Socket& socket, Network::SocketOptionName optname,
-                                               absl::string_view value);
+                                               const void* value, size_t size);
 
 private:
   const envoy::api::v2::core::SocketOption::SocketState in_state_;
   const Network::SocketOptionName optname_;
-  const std::string value_;
+  // This has to be a std::vector<uint8_t> but not std::string because std::string might inline
+  // the buffer so its data() is not aligned in to alignof(void*).
+  const std::vector<uint8_t> value_;
 };
 
 } // namespace Network

test/common/network/socket_option_factory_test.cc

@@ -154,20 +154,21 @@ TEST_F(SocketOptionFactoryTest, TestBuildLiteralOptions) {
   EXPECT_TRUE(option_details.has_value());
   EXPECT_EQ(SOL_SOCKET, option_details->name_->first);
   EXPECT_EQ(SO_LINGER, option_details->name_->second);
-  EXPECT_EQ(sizeof(struct linger), option_details->value_.size());
-  const struct linger* linger_ptr =
-      reinterpret_cast<const struct linger*>(option_details->value_.data());
-  EXPECT_EQ(1, linger_ptr->l_onoff);
-  EXPECT_EQ(3456, linger_ptr->l_linger);
+  struct linger expected_linger;
+  expected_linger.l_onoff = 1;
+  expected_linger.l_linger = 3456;
+  absl::string_view linger_bstr{reinterpret_cast<const char*>(&expected_linger),
+                                sizeof(struct linger)};
+  EXPECT_EQ(linger_bstr, option_details->value_);
 
   option_details = socket_options->at(1)->getOptionDetails(
       socket_mock_, envoy::api::v2::core::SocketOption::STATE_PREBIND);
   EXPECT_TRUE(option_details.has_value());
   EXPECT_EQ(SOL_SOCKET, option_details->name_->first);
   EXPECT_EQ(SO_KEEPALIVE, option_details->name_->second);
-  EXPECT_EQ(sizeof(int), option_details->value_.size());
-  const int* flag_ptr = reinterpret_cast<const int*>(option_details->value_.data());
-  EXPECT_EQ(1, *flag_ptr);
+  int value = 1;
+  absl::string_view value_bstr{reinterpret_cast<const char*>(&value), sizeof(int)};
+  EXPECT_EQ(value_bstr, option_details->value_);
 }
 
 } // namespace

test/common/network/socket_option_impl_test.cc

@@ -8,7 +8,8 @@ class SocketOptionImplTest : public SocketOptionTest {};
 
 TEST_F(SocketOptionImplTest, BadFd) {
   absl::string_view zero("\0\0\0\0", 4);
-  Api::SysCallIntResult result = SocketOptionImpl::setSocketOption(socket_, {}, zero);
+  Api::SysCallIntResult result =
+      SocketOptionImpl::setSocketOption(socket_, {}, zero.data(), zero.size());
   EXPECT_EQ(-1, result.rc_);
   EXPECT_EQ(ENOTSUP, result.errno_);
 }