From 1010584f0c07b26816d70d8162f1c7717630bf44 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 11 May 2023 21:20:54 -0400 Subject: [PATCH] Fix alignment problem when casting struct sockaddr pointers to SockAddr. (#26488) * Fix alignment problem when casting struct sockaddr pointers to SockAddr. SockAddr, because it includes sockaddr_storage, has a more stringent alignment requirement than struct sockaddr does. As a result, the casting we do of "sockaddr &" to "SockAddr &" is not really OK. The fix is to have a version of SockAddr that lets us do the type-punning we want to do without including sockaddr_storage. We don't need storage in this case because we are working with an existing sockaddr that lives somewhere, not storing one of our own. This also lets us enable UndefinedBehaviorSanitizer for libCHIP in the Darwin framework tests. * Fix typo in comment. --- .github/workflows/darwin.yaml | 4 +++- src/inet/IPAddress.cpp | 2 +- src/inet/IPAddress.h | 25 +++++++++++++++++++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/.github/workflows/darwin.yaml b/.github/workflows/darwin.yaml index 169606644a9e19..00c669eb81a4ce 100644 --- a/.github/workflows/darwin.yaml +++ b/.github/workflows/darwin.yaml @@ -145,7 +145,9 @@ jobs: # And a different port from the test harness too; the test harness uses port 5541. ../../../out/debug/chip-ota-requestor-app --interface-id -1 --secured-device-port 5542 --discriminator 1111 --KVS /tmp/chip-ota-requestor-kvs1 --otaDownloadPath /tmp/chip-ota-requestor-downloaded-image1 --autoApplyImage > >(tee /tmp/darwin/framework-tests/ota-requestor-app-1.log) 2> >(tee /tmp/darwin/framework-tests/ota-requestor-app-err-1.log >&2) & ../../../out/debug/chip-ota-requestor-app --interface-id -1 --secured-device-port 5543 --discriminator 1112 --KVS /tmp/chip-ota-requestor-kvs2 --otaDownloadPath /tmp/chip-ota-requestor-downloaded-image2 --autoApplyImage > >(tee /tmp/darwin/framework-tests/ota-requestor-app-2.log) 2> >(tee /tmp/darwin/framework-tests/ota-requestor-app-err-2.log >&2) & - xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-incomplete-umbrella -Wno-unguarded-availability-new' > >(tee /tmp/darwin/framework-tests/darwin-tests.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-err.log >&2) + # UndefinedBehaviorSanitizer is enabled in the Xcode scheme, so the code in Matter.framework gets instrumented, + # but to instrument the code in the underlying libCHIP we need to pass CHIP_IS_UBSAN=YES + xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-incomplete-umbrella -Wno-unguarded-availability-new' CHIP_IS_UBSAN=YES > >(tee /tmp/darwin/framework-tests/darwin-tests.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-err.log >&2) working-directory: src/darwin/Framework - name: Uploading log files uses: actions/upload-artifact@v3 diff --git a/src/inet/IPAddress.cpp b/src/inet/IPAddress.cpp index 3b4c25eb926185..19da2acaee94f7 100644 --- a/src/inet/IPAddress.cpp +++ b/src/inet/IPAddress.cpp @@ -237,7 +237,7 @@ struct in6_addr IPAddress::ToIPv6() const return ipAddr; } -CHIP_ERROR IPAddress::GetIPAddressFromSockAddr(const SockAddr & sockaddr, IPAddress & outIPAddress) +CHIP_ERROR IPAddress::GetIPAddressFromSockAddr(const SockAddrWithoutStorage & sockaddr, IPAddress & outIPAddress) { #if INET_CONFIG_ENABLE_IPV4 if (sockaddr.any.sa_family == AF_INET) diff --git a/src/inet/IPAddress.h b/src/inet/IPAddress.h index 0e89afa77ca347..661d0f84d65a91 100644 --- a/src/inet/IPAddress.h +++ b/src/inet/IPAddress.h @@ -107,6 +107,14 @@ enum class IPv6MulticastFlag : uint8_t using IPv6MulticastFlags = BitFlags; #if CHIP_SYSTEM_CONFIG_USE_SOCKETS +/** + * SockAddr should be used when calling any API that returns (by copying into + * it) a sockaddr, because that will need enough storage that it can hold data + * for any socket type. + * + * It can also be used when calling an API that accepts a sockaddr, to simplify + * the type-punning needed. + */ union SockAddr { sockaddr any; @@ -114,6 +122,19 @@ union SockAddr sockaddr_in6 in6; sockaddr_storage storage; }; + +/** + * SockAddrWithoutStorage can be used any time we want to do the sockaddr + * type-punning but will not store the data ourselves (e.g. we're working with + * an existing sockaddr pointer, and reintepret it as a + * pointer-to-SockAddrWithoutStorage). + */ +union SockAddrWithoutStorage +{ + sockaddr any; + sockaddr_in in; + sockaddr_in6 in6; +}; #endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS /** @@ -550,10 +571,10 @@ class DLL_EXPORT IPAddress /** * Get the IP address from a SockAddr. */ - static CHIP_ERROR GetIPAddressFromSockAddr(const SockAddr & sockaddr, IPAddress & outIPAddress); + static CHIP_ERROR GetIPAddressFromSockAddr(const SockAddrWithoutStorage & sockaddr, IPAddress & outIPAddress); static CHIP_ERROR GetIPAddressFromSockAddr(const sockaddr & sockaddr, IPAddress & outIPAddress) { - return GetIPAddressFromSockAddr(reinterpret_cast(sockaddr), outIPAddress); + return GetIPAddressFromSockAddr(reinterpret_cast(sockaddr), outIPAddress); } static IPAddress FromSockAddr(const sockaddr_in6 & sockaddr) { return IPAddress(sockaddr.sin6_addr); } #if INET_CONFIG_ENABLE_IPV4