From d1188b67c4a9096eb7027e9196f3032873833d13 Mon Sep 17 00:00:00 2001
From: Lizan Zhou <lizan@tetrate.io>
Date: Tue, 20 Aug 2019 11:14:38 -0700
Subject: [PATCH] Fix the alignement in optval of setsockopt when compiled with
 libc++. (#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 #4251

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

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
---
 source/common/network/socket_option_impl.cc    | 10 +++++-----
 source/common/network/socket_option_impl.h     | 12 +++++++++---
 test/common/network/socket_option_impl_test.cc |  3 ++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/source/common/network/socket_option_impl.cc b/source/common/network/socket_option_impl.cc
index 2065309ed7a5..a4313490fdaf 100644
--- a/source/common/network/socket_option_impl.cc
+++ b/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
diff --git a/source/common/network/socket_option_impl.h b/source/common/network/socket_option_impl.h
index 4ff917bb70ec..94f11b081b52 100644
--- a/source/common/network/socket_option_impl.h
+++ b/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
diff --git a/test/common/network/socket_option_impl_test.cc b/test/common/network/socket_option_impl_test.cc
index aa066522bde6..8d39beba1a59 100644
--- a/test/common/network/socket_option_impl_test.cc
+++ b/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_);
 }