Skip to content

Commit

Permalink
Fix alignment problem when casting struct sockaddr pointers to SockAddr.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple committed May 10, 2023
1 parent 5471bca commit 92f0839
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/darwin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/inet/IPAddress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 23 additions & 2 deletions src/inet/IPAddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,34 @@ enum class IPv6MulticastFlag : uint8_t
using IPv6MulticastFlags = BitFlags<IPv6MulticastFlag>;

#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;
sockaddr_in in;
sockaddr_in6 in6;
sockaddr_storage storage;
};

/**
* SockAddrWithoutStorage cane 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

/**
Expand Down Expand Up @@ -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<const SockAddr &>(sockaddr), outIPAddress);
return GetIPAddressFromSockAddr(reinterpret_cast<const SockAddrWithoutStorage &>(sockaddr), outIPAddress);
}
static IPAddress FromSockAddr(const sockaddr_in6 & sockaddr) { return IPAddress(sockaddr.sin6_addr); }
#if INET_CONFIG_ENABLE_IPV4
Expand Down

0 comments on commit 92f0839

Please sign in to comment.