From cc785eb6f1955cfb12114d8b8fd47b73cdf92d40 Mon Sep 17 00:00:00 2001 From: johnx Date: Mon, 1 Jul 2024 12:24:38 -0700 Subject: [PATCH] [AOSP]Fix musl vs SDK addrinfo definition mismatch (#3637) musl and Android SDK have different header definitions for addrinfo struct and its flag values. This was fine on Linux EG platforms because the headers are very similar and it was fine on Kimono because Kimono directly uses headers from the SDK. b/348023522 Change-Id: I7c27c08f7c7d503abc608388b5b3c7f29016eba6 --- starboard/build/config/BUILD.gn | 3 + starboard/elf_loader/exported_symbols.cc | 2 + .../posix_socket_resolve_test.cc | 87 +++++++- .../cobalt_layer_posix_socket_abi_wrappers.cc | 5 + ...arboard_layer_posix_socket_abi_wrappers.cc | 209 +++++++++++++++++- ...tarboard_layer_posix_socket_abi_wrappers.h | 40 +++- starboard/shared/win32/test_filters.py | 2 + 7 files changed, 330 insertions(+), 18 deletions(-) diff --git a/starboard/build/config/BUILD.gn b/starboard/build/config/BUILD.gn index 3484a91e338e..d31feb192e63 100644 --- a/starboard/build/config/BUILD.gn +++ b/starboard/build/config/BUILD.gn @@ -52,6 +52,9 @@ if (cobalt_pending_clean_up) { "-Wno-unknown-attributes", "-Wno-unused-command-line-argument", "-Wno-unused-function", + + # musl getaddrinfo implementation null-check static array on Linux + "-Wno-tautological-pointer-compare", ] if (!using_old_compiler) { diff --git a/starboard/elf_loader/exported_symbols.cc b/starboard/elf_loader/exported_symbols.cc index 252bd7dbe1d9..0110fea7491a 100644 --- a/starboard/elf_loader/exported_symbols.cc +++ b/starboard/elf_loader/exported_symbols.cc @@ -604,6 +604,8 @@ ExportedSymbols::ExportedSymbols() { map_["bind"] = reinterpret_cast(&__abi_wrap_bind); map_["connect"] = reinterpret_cast(&__abi_wrap_connect); map_["getaddrinfo"] = reinterpret_cast(&__abi_wrap_getaddrinfo); + map_["freeaddrinfo"] = + reinterpret_cast(&__abi_wrap_freeaddrinfo); map_["getifaddrs"] = reinterpret_cast(&__abi_wrap_getifaddrs); map_["setsockopt"] = reinterpret_cast(&__abi_wrap_setsockopt); diff --git a/starboard/nplb/posix_compliance/posix_socket_resolve_test.cc b/starboard/nplb/posix_compliance/posix_socket_resolve_test.cc index 43ad22446b5c..3f7a749af8e5 100644 --- a/starboard/nplb/posix_compliance/posix_socket_resolve_test.cc +++ b/starboard/nplb/posix_compliance/posix_socket_resolve_test.cc @@ -12,6 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include +#include +#include + #include "starboard/nplb/posix_compliance/posix_socket_helpers.h" namespace starboard { @@ -27,7 +31,7 @@ TEST(PosixSocketResolveTest, SunnyDay) { struct addrinfo* ai = nullptr; int result = getaddrinfo(kTestHostName, 0, &hints, &ai); - EXPECT_TRUE(result == 0); + EXPECT_EQ(result, 0); ASSERT_NE(nullptr, ai); int address_count = 0; @@ -49,12 +53,91 @@ TEST(PosixSocketResolveTest, SunnyDay) { freeaddrinfo(ai); } +#if SB_API_VERSION >= 16 +TEST(PosixSocketResolveTest, SunnyDaySocketType) { + struct addrinfo hints = {0}; + hints.ai_socktype = SOCK_DGRAM; + struct addrinfo* ai = nullptr; + + int result = getaddrinfo(kTestHostName, 0, &hints, &ai); + EXPECT_EQ(result, 0); + ASSERT_NE(nullptr, ai); + + struct sockaddr_in* ai_addr = nullptr; + + for (const struct addrinfo* i = ai; i != nullptr; i = i->ai_next) { + EXPECT_EQ(i->ai_socktype, SOCK_DGRAM); + } + + freeaddrinfo(ai); +} + +TEST(PosixSocketResolveTest, SunnyDayFamily) { + struct addrinfo hints = {0}; + hints.ai_family = AF_INET6; + struct addrinfo* ai = nullptr; + + int result = getaddrinfo(kTestHostName, 0, &hints, &ai); + EXPECT_EQ(result, 0); + ASSERT_NE(nullptr, ai); + + for (const struct addrinfo* i = ai; i != nullptr; i = i->ai_next) { + EXPECT_EQ(i->ai_addr->sa_family, AF_INET6); + } + + freeaddrinfo(ai); +} + +TEST(PosixSocketResolveTest, SunnyDayFlags) { + struct addrinfo hints = {0}; + int flags_to_test[] = { + AI_PASSIVE, AI_ADDRCONFIG, AI_PASSIVE, AI_CANONNAME, AI_V4MAPPED, AI_ALL, + }; + for (auto flag : flags_to_test) { + hints.ai_flags |= flag; + struct addrinfo* ai = nullptr; + + int result = getaddrinfo(kTestHostName, 0, &hints, &ai); + EXPECT_EQ(result, 0); + ASSERT_NE(nullptr, ai); + + for (const struct addrinfo* i = ai; i != nullptr; i = i->ai_next) { + EXPECT_EQ(i->ai_flags, hints.ai_flags); + } + + freeaddrinfo(ai); + } +} + +TEST(PosixSocketResolveTest, SunnyDayProtocol) { + struct addrinfo hints = {0}; + int protocol_to_test[] = { + IPPROTO_TCP, + IPPROTO_UDP, + }; + for (auto protocol : protocol_to_test) { + hints.ai_protocol = protocol; + struct addrinfo* ai = nullptr; + + int result = getaddrinfo(kTestHostName, 0, &hints, &ai); + EXPECT_EQ(result, 0); + ASSERT_NE(nullptr, ai); + + for (const struct addrinfo* i = ai; i != nullptr; i = i->ai_next) { + EXPECT_EQ(i->ai_protocol, hints.ai_protocol); + } + + freeaddrinfo(ai); + } +} +#endif // SB_API_VERSION >= 16 + TEST(PosixSocketResolveTest, Localhost) { struct addrinfo hints = {0}; struct addrinfo* ai = nullptr; int result = getaddrinfo(kLocalhost, 0, &hints, &ai); - EXPECT_TRUE(result == 0); + EXPECT_EQ(result, 0); ASSERT_NE(nullptr, ai); int address_count = 0; diff --git a/starboard/shared/modular/cobalt_layer_posix_socket_abi_wrappers.cc b/starboard/shared/modular/cobalt_layer_posix_socket_abi_wrappers.cc index f2701a9ae793..fd8d3e1c8ebd 100644 --- a/starboard/shared/modular/cobalt_layer_posix_socket_abi_wrappers.cc +++ b/starboard/shared/modular/cobalt_layer_posix_socket_abi_wrappers.cc @@ -50,6 +50,11 @@ int getaddrinfo(const char* node, return __abi_wrap_getaddrinfo(node, service, hints, res); } +void __abi_wrap_freeaddrinfo(struct addrinfo* ai); +void freeaddrinfo(struct addrinfo* ai) { + return __abi_wrap_freeaddrinfo(ai); +} + int __abi_wrap_getifaddrs(struct ifaddrs** ifap); int getifaddrs(struct ifaddrs** ifap) { return __abi_wrap_getifaddrs(ifap); diff --git a/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.cc b/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.cc index a12f22213039..1c4b21224fa6 100644 --- a/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.cc +++ b/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.cc @@ -13,11 +13,110 @@ // limitations under the License. #include "starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.h" +#include #include #if SB_HAS_QUIRK(SOCKADDR_WITH_LENGTH) #include #endif +#include "starboard/log.h" + +namespace { +// Corresponding arrays to for musl<->platform translation. +int MUSL_AI_ORDERED[] = { + MUSL_AI_PASSIVE, MUSL_AI_CANONNAME, MUSL_AI_NUMERICHOST, MUSL_AI_V4MAPPED, + MUSL_AI_ALL, MUSL_AI_ADDRCONFIG, MUSL_AI_NUMERICSERV, +}; +int PLATFORM_AI_ORDERED[] = { + AI_PASSIVE, AI_CANONNAME, AI_NUMERICHOST, AI_V4MAPPED, + AI_ALL, AI_ADDRCONFIG, AI_NUMERICSERV, +}; +// Corresponding arrays to for musl<->platform translation. +int MUSL_SOCK_ORDERED[] = { + MUSL_SOCK_STREAM, + MUSL_SOCK_DGRAM, + MUSL_SOCK_RAW, +}; +int PLATFORM_SOCK_ORDERED[] = { + SOCK_STREAM, + SOCK_DGRAM, + SOCK_RAW, +}; +// Corresponding arrays to for musl<->platform translation. +int MUSL_IPPROTO_ORDERED[] = { + MUSL_IPPROTO_TCP, + MUSL_IPPROTO_UDP, +}; +int PLATFORM_IPPROTO_ORDERED[] = { + IPPROTO_TCP, + IPPROTO_UDP, +}; +// Corresponding arrays to for musl<->platform translation. +int MUSL_AF_ORDERED[] = { + MUSL_AF_INET, + MUSL_AF_INET6, +}; +int PLATFORM_AF_ORDERED[] = { + AF_INET, + AF_INET6, +}; +int musl_hints_to_platform_hints(const struct musl_addrinfo* hints, + struct addrinfo* platform_hints) { + int ai_left = hints->ai_flags; + for (int i = 0; i < sizeof(MUSL_AI_ORDERED) / sizeof(int); i++) { + if (hints->ai_flags & MUSL_AI_ORDERED[i]) { + platform_hints->ai_flags |= PLATFORM_AI_ORDERED[i]; + ai_left &= ~MUSL_AI_ORDERED[i]; + } + } + for (int i = 0; i < sizeof(MUSL_SOCK_ORDERED) / sizeof(int); i++) { + if (hints->ai_socktype == MUSL_SOCK_ORDERED[i]) + platform_hints->ai_socktype = PLATFORM_SOCK_ORDERED[i]; + } + for (int i = 0; i < sizeof(MUSL_IPPROTO_ORDERED) / sizeof(int); i++) { + if (hints->ai_protocol == MUSL_IPPROTO_ORDERED[i]) + platform_hints->ai_protocol = PLATFORM_IPPROTO_ORDERED[i]; + } + for (int i = 0; i < sizeof(MUSL_AF_ORDERED) / sizeof(int); i++) { + if (hints->ai_family == MUSL_AF_ORDERED[i]) + platform_hints->ai_family = PLATFORM_AF_ORDERED[i]; + } + if (ai_left != 0 || + (hints->ai_socktype != 0 && platform_hints->ai_socktype == 0) || + (hints->ai_protocol != 0 && platform_hints->ai_protocol == 0) || + (hints->ai_family != 0 && platform_hints->ai_family == 0)) + return -1; + return 0; +} +int platform_hints_to_musl_hints(const struct addrinfo* hints, + struct musl_addrinfo* musl_hints) { + int ai_left = hints->ai_flags; + for (int i = 0; i < sizeof(PLATFORM_AI_ORDERED) / sizeof(int); i++) { + if (hints->ai_flags & PLATFORM_AI_ORDERED[i]) { + musl_hints->ai_flags |= MUSL_AI_ORDERED[i]; + ai_left &= ~PLATFORM_AI_ORDERED[i]; + } + } + for (int i = 0; i < sizeof(PLATFORM_SOCK_ORDERED) / sizeof(int); i++) { + if (hints->ai_socktype == PLATFORM_SOCK_ORDERED[i]) + musl_hints->ai_socktype = MUSL_SOCK_ORDERED[i]; + } + for (int i = 0; i < sizeof(PLATFORM_IPPROTO_ORDERED) / sizeof(int); i++) { + if (hints->ai_protocol == PLATFORM_IPPROTO_ORDERED[i]) + musl_hints->ai_protocol = MUSL_IPPROTO_ORDERED[i]; + } + for (int i = 0; i < sizeof(PLATFORM_AF_ORDERED) / sizeof(int); i++) { + if (hints->ai_family == PLATFORM_AF_ORDERED[i]) + musl_hints->ai_family = MUSL_AF_ORDERED[i]; + } + if (ai_left != 0 || + (hints->ai_socktype != 0 && musl_hints->ai_socktype == 0) || + (hints->ai_protocol != 0 && musl_hints->ai_protocol == 0) || + (hints->ai_family != 0 && musl_hints->ai_family == 0)) + return -1; + return 0; +} +} // namespace SB_EXPORT int __abi_wrap_accept(int sockfd, musl_sockaddr* addr, @@ -77,27 +176,113 @@ SB_EXPORT int __abi_wrap_connect(int sockfd, SB_EXPORT int __abi_wrap_getaddrinfo(const char* node, const char* service, - const struct addrinfo* hints, - struct addrinfo** res) { - int result = getaddrinfo(node, service, hints, res); + const struct musl_addrinfo* hints, + struct musl_addrinfo** res) { + if (res == nullptr) + return -1; + // musl addrinfo definition might differ from platform definition. + // So we need to do a manual conversion to avoid header mismatches. + struct addrinfo new_hints = {0}; -#if SB_HAS_QUIRK(SOCKADDR_WITH_LENGTH) - struct addrinfo* ai = *res; + if (hints != nullptr) { + SbLogRawFormatF("hints ai_family: %d\n", hints->ai_family); + SbLogRawFormatF("hints ai_socktype: %d\n", hints->ai_socktype); + SbLogRawFormatF("hints ai_flags: %d\n", hints->ai_flags); + SbLogRawFormatF("hints ai_protocol: %d\n", hints->ai_protocol); + if (musl_hints_to_platform_hints(hints, &new_hints) == -1) { + SbLogRawFormatF("musl_hints_to_platform_hints failed"); + return -1; + } + SbLogRawFormatF("new_hints ai_family: %d\n", new_hints.ai_family); + SbLogRawFormatF("new_hints ai_socktype: %d\n", new_hints.ai_socktype); + SbLogRawFormatF("new_hints ai_flags: %d\n", new_hints.ai_flags); + SbLogRawFormatF("new_hints ai_protocol: %d\n", new_hints.ai_protocol); + } + + addrinfo* native_res = nullptr; + int result = getaddrinfo( + node, service, + (hints == nullptr) ? nullptr + : reinterpret_cast(&new_hints), + &native_res); + + struct addrinfo* ai = native_res; + struct musl_addrinfo* last_ai = nullptr; + *res = nullptr; while (ai != nullptr) { if (ai->ai_addr != nullptr) { - musl_sockaddr* musl_addr_ptr = - reinterpret_cast(ai->ai_addr); - struct sockaddr* addr_ptr = - reinterpret_cast(ai->ai_addr); - uint8_t sa_family = addr_ptr->sa_family; - musl_addr_ptr->sa_family = sa_family; + addrinfo ai_copy = *ai; + struct musl_addrinfo* musl_ai = + (struct musl_addrinfo*)calloc(1, sizeof(struct musl_addrinfo)); + SbLogRawFormatF("returned ai_flags: %d\n", ai_copy.ai_flags); + SbLogRawFormatF("returned ai_socktype: %d\n", ai_copy.ai_socktype); + SbLogRawFormatF("returned ai_protocol: %d\n", ai_copy.ai_protocol); + SbLogRawFormatF("returned ai_family: %d\n", ai_copy.ai_family); + if (platform_hints_to_musl_hints(&ai_copy, musl_ai) == -1) { + SbLogRawFormatF("platform_hints_to_musl_hints failed"); + free(musl_ai); + return -1; + } + SbLogRawFormatF("translated ai_flags: %d\n", musl_ai->ai_flags); + SbLogRawFormatF("translated ai_socktype: %d\n", musl_ai->ai_socktype); + SbLogRawFormatF("translated ai_protocol: %d\n", musl_ai->ai_protocol); + SbLogRawFormatF("translated ai_family: %d\n", musl_ai->ai_family); + musl_ai->ai_addrlen = ai_copy.ai_addrlen; + musl_ai->ai_addr = + (struct musl_sockaddr*)calloc(1, sizeof(struct musl_sockaddr)); + SbLogRawFormatF("ai_addrlen: %d\n", ai_copy.ai_addrlen); + for (int i = 0; i < sizeof(PLATFORM_AF_ORDERED) / sizeof(int); i++) { + if (ai_copy.ai_addr->sa_family == PLATFORM_AF_ORDERED[i]) + musl_ai->ai_addr->sa_family = MUSL_AF_ORDERED[i]; + } + SbLogRawFormatF("ai_copy.ai_addr->sa_family: %d\n", + ai_copy.ai_addr->sa_family); + SbLogRawFormatF("musl_ai->ai_addr->sa_family: %d\n", + musl_ai->ai_addr->sa_family); + if (ai_copy.ai_addr->sa_data != nullptr) { + memcpy(musl_ai->ai_addr->sa_data, ai_copy.ai_addr->sa_data, + sizeof(ai_copy.ai_addr->sa_data)); + } + SbLogRawFormatF("musl_ai->ai_addr->sa_data: %s\n", + musl_ai->ai_addr->sa_data); + + SbLogRaw(musl_ai->ai_addr->sa_data); + if (ai_copy.ai_canonname) { + size_t canonname_len = strlen(ai_copy.ai_canonname); + musl_ai->ai_canonname = + reinterpret_cast(calloc(canonname_len + 1, sizeof(char))); + memcpy(musl_ai->ai_canonname, ai_copy.ai_canonname, canonname_len); + } + if (*res == nullptr) { + *res = musl_ai; + last_ai = musl_ai; + } else { + last_ai->ai_next = musl_ai; + last_ai = musl_ai; + } } ai = ai->ai_next; } -#endif + freeaddrinfo(native_res); + return result; } +SB_EXPORT void __abi_wrap_freeaddrinfo(struct musl_addrinfo* ai) { + struct musl_addrinfo* ptr = ai; + while (ai != nullptr) { + if (ai->ai_addr != nullptr) { + free(ai->ai_addr); + } + if (ai->ai_canonname != nullptr) { + free(ai->ai_canonname); + } + ai = ai->ai_next; + free(ptr); + ptr = ai; + } +} + SB_EXPORT int __abi_wrap_getifaddrs(struct ifaddrs** ifap) { int result = getifaddrs(ifap); #if SB_HAS_QUIRK(SOCKADDR_WITH_LENGTH) diff --git a/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.h b/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.h index 58208a48d4a5..0b21ff46716b 100644 --- a/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.h +++ b/starboard/shared/modular/starboard_layer_posix_socket_abi_wrappers.h @@ -29,14 +29,44 @@ extern "C" { #endif -// sizeof(sockaddr_in6) = 28, 28 - 2 = 26 +// The value from musl headers +#define MUSL_AI_PASSIVE 0x01 +#define MUSL_AI_CANONNAME 0x02 +#define MUSL_AI_NUMERICHOST 0x04 +#define MUSL_AI_V4MAPPED 0x08 +#define MUSL_AI_ALL 0x10 +#define MUSL_AI_ADDRCONFIG 0x20 +#define MUSL_AI_NUMERICSERV 0x400 + +#define MUSL_SOCK_STREAM 1 +#define MUSL_SOCK_DGRAM 2 +#define MUSL_SOCK_RAW 3 + +#define MUSL_IPPROTO_TCP 6 +#define MUSL_IPPROTO_UDP 17 + +#define MUSL_AF_INET 2 +#define MUSL_AF_INET6 10 + +// sizeof(sockaddr_in6) = 28 // This size enables musl_sockaddr to work for both IPv4 and IPv6 -#define MUSL_SOCKADDR_SA_DATA_SIZE 26 +#define MUSL_SOCKADDR_SA_DATA_SIZE 28 typedef struct musl_sockaddr { uint16_t sa_family; char sa_data[MUSL_SOCKADDR_SA_DATA_SIZE]; } musl_sockaddr; +typedef struct musl_addrinfo { + int32_t /* int */ ai_flags; + int32_t /* int */ ai_family; + int32_t /* int */ ai_socktype; + int32_t /* int */ ai_protocol; + uint32_t /* socklen_t */ ai_addrlen; + struct musl_sockaddr* ai_addr; + char* ai_canonname; + struct musl_addrinfo* ai_next; +} musl_addrinfo; + SB_EXPORT int __abi_wrap_accept(int sockfd, musl_sockaddr* addr, socklen_t* addrlen_ptr); @@ -51,8 +81,10 @@ SB_EXPORT int __abi_wrap_connect(int sockfd, SB_EXPORT int __abi_wrap_getaddrinfo(const char* node, const char* service, - const struct addrinfo* hints, - struct addrinfo** res); + const struct musl_addrinfo* hints, + struct musl_addrinfo** res); + +SB_EXPORT void __abi_wrap_freeaddrinfo(struct musl_addrinfo* ai); SB_EXPORT int __abi_wrap_getifaddrs(struct ifaddrs** ifap); diff --git a/starboard/shared/win32/test_filters.py b/starboard/shared/win32/test_filters.py index 8236713eac4e..81ef96333048 100644 --- a/starboard/shared/win32/test_filters.py +++ b/starboard/shared/win32/test_filters.py @@ -22,6 +22,8 @@ # TODO(b/304335954): Re-enable the test for UWP after fixing DST # implementation. 'SbTimeZoneGetNameTest.IsIANAFormat', + 'PosixSocketResolveTest.SunnyDayFamily', + 'PosixSocketResolveTest.SunnyDayFlags', ], 'base_test': [ # The `ProcessMetricsHelper` depends on the virtual files in /proc.