diff --git a/src/reflector/default_address_monitor.cpp b/src/reflector/default_address_monitor.cpp index d2bad38..b29dd23 100644 --- a/src/reflector/default_address_monitor.cpp +++ b/src/reflector/default_address_monitor.cpp @@ -219,7 +219,7 @@ void DefaultAddressMonitor::OnReadable(int /*fd*/) noexcept { // parsing — the collected list would only be discarded. if (!overflowed) { CollectChangedInterfaces( - std::span{buffer.data(), static_cast(received)}, changed); + std::span{buffer.data(), static_cast(received)}, changed); } } @@ -236,25 +236,27 @@ void DefaultAddressMonitor::OnReadable(int /*fd*/) noexcept { #if defined(__linux__) // The kernel netlink macros (NLMSG_OK, NLMSG_NEXT, NLMSG_DATA) use C-style casts and byte-wise -// pointer arithmetic that the project's strict warning set rejects; the access is sound (OnReadable -// aligns the buffer for nlmsghdr and messages are NLMSG_ALIGN padded). Scope the suppression to the -// parser. +// pointer arithmetic that the project's strict warning set rejects; the alignment is sound +// (OnReadable aligns the buffer for nlmsghdr and messages are NLMSG_ALIGN padded). Scope the +// suppression to the parser. Every struct the walk reads is start_lifetime_as'd first: the macros +// only compute addresses and read fields of the already-blessed current header, so blessing each +// derived pointer before its first dereference keeps all accesses on live objects. #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wold-style-cast" #pragma GCC diagnostic ignored "-Wcast-align" #pragma GCC diagnostic ignored "-Wsign-conversion" #pragma GCC diagnostic ignored "-Wconversion" -void DefaultAddressMonitor::CollectChangedInterfaces(std::span messages, +void DefaultAddressMonitor::CollectChangedInterfaces(std::span messages, std::vector& changed) const noexcept { - const auto* header = reinterpret_cast(messages.data()); + auto* header = start_lifetime_as(messages.data()); for (int length = static_cast(messages.size()); NLMSG_OK(header, length); - header = NLMSG_NEXT(header, length)) { + header = start_lifetime_as(NLMSG_NEXT(header, length))) { if (header->nlmsg_type == RTM_NEWADDR || header->nlmsg_type == RTM_DELADDR) { - const auto* address = static_cast(NLMSG_DATA(header)); + const auto* address = start_lifetime_as(NLMSG_DATA(header)); AddUnique(changed, address->ifa_index); } else if (header->nlmsg_type == RTM_NEWLINK || header->nlmsg_type == RTM_DELLINK) { - const auto* link = static_cast(NLMSG_DATA(header)); + const auto* link = start_lifetime_as(NLMSG_DATA(header)); AddUnique(changed, static_cast(link->ifi_index)); } } @@ -264,7 +266,7 @@ void DefaultAddressMonitor::CollectChangedInterfaces(std::span #else -void DefaultAddressMonitor::CollectChangedInterfaces(std::span messages, +void DefaultAddressMonitor::CollectChangedInterfaces(std::span messages, std::vector& changed) const noexcept { // PF_ROUTE messages pack back-to-back; each begins with rt_msghdr's prefix (u_short msglen; // u_char version; u_char type). Read the fields with memcpy — the buffer carries no diff --git a/src/reflector/default_address_monitor.h b/src/reflector/default_address_monitor.h index 63188ee..cc6e7fe 100644 --- a/src/reflector/default_address_monitor.h +++ b/src/reflector/default_address_monitor.h @@ -68,8 +68,9 @@ class DefaultAddressMonitor : public AddressMonitor, NoMove { // Parses a buffer of kernel notification messages and appends each changed interface index to // `changed`, skipping any already present. Split out from OnReadable so tests can drive it with - // synthesized messages. - void CollectChangedInterfaces(std::span messages, + // synthesized messages. The span is mutable because the Linux walk start_lifetime_as's the + // netlink structs over the received bytes, which reuses that storage. + void CollectChangedInterfaces(std::span messages, std::vector& changed) const noexcept; Dispatcher* dispatcher_; diff --git a/src/reflector/interface_address.cpp b/src/reflector/interface_address.cpp index 6045522..16bdad5 100644 --- a/src/reflector/interface_address.cpp +++ b/src/reflector/interface_address.cpp @@ -3,6 +3,7 @@ #include "error.h" #include "logger.h" #include "platform.h" +#include "util/start_lifetime_as.h" #include #include @@ -87,10 +88,12 @@ bool IsUsable(uint32_t ifa_flags) noexcept { } // The kernel netlink macros (NLMSG_*, RTA_*, IFLA_RTA, IFA_RTA) use C-style casts and -// byte-wise pointer arithmetic that the project's strict warning set rejects. The accesses are -// sound — the receive buffer is netlink-aligned (alignas below) and every message/attribute is -// NLMSG_ALIGN/RTA_ALIGN padded — so scope the suppression to the netlink code rather than -// hand-reimplementing the macros. +// byte-wise pointer arithmetic that the project's strict warning set rejects; the alignment is +// sound (the receive buffer is max-aligned and every message/attribute is NLMSG_ALIGN/RTA_ALIGN +// padded), so scope the suppression to the netlink code rather than hand-reimplementing the +// macros. Every struct the walks read is start_lifetime_as'd first: the macros only compute +// addresses and read fields of the already-blessed current struct, so blessing each derived +// pointer before its first dereference keeps all accesses on live objects. #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wold-style-cast" #pragma GCC diagnostic ignored "-Wcast-align" @@ -154,9 +157,9 @@ bool NetlinkDump(int fd, uint16_t request_type, uint32_t seq, Handler&& handle) continue; } - auto* header = reinterpret_cast(buffer.data()); + auto* header = start_lifetime_as(buffer.data()); for (int length = static_cast(received); NLMSG_OK(header, length); - header = NLMSG_NEXT(header, length)) { + header = start_lifetime_as(NLMSG_NEXT(header, length))) { if (header->nlmsg_seq != seq) { continue; } @@ -165,7 +168,7 @@ bool NetlinkDump(int fd, uint16_t request_type, uint32_t seq, Handler&& handle) } if (header->nlmsg_type == NLMSG_ERROR) { // nlmsgerr.error is a negative errno (0 is an ACK, not a failure). - const auto* error = static_cast(NLMSG_DATA(header)); + const auto* error = start_lifetime_as(NLMSG_DATA(header)); if (error->error != 0) { GetLogger().Error("Netlink dump returned an error: {}", Error::FromErrno(-error->error)); return false; @@ -184,13 +187,14 @@ void ResolveViaNetlink(unsigned index, InterfaceAddresses& result) noexcept { return; } - NetlinkDump(fd, RTM_GETLINK, 1, [&](const nlmsghdr* header) { - const auto* link = static_cast(NLMSG_DATA(header)); + NetlinkDump(fd, RTM_GETLINK, 1, [&](nlmsghdr* header) { + const auto* link = start_lifetime_as(NLMSG_DATA(header)); if (static_cast(link->ifi_index) != index) { return; } int length = IFLA_PAYLOAD(header); - for (const rtattr* attr = IFLA_RTA(link); RTA_OK(attr, length); attr = RTA_NEXT(attr, length)) { + for (auto* attr = start_lifetime_as(IFLA_RTA(link)); RTA_OK(attr, length); + attr = start_lifetime_as(RTA_NEXT(attr, length))) { if (attr->rta_type == IFLA_ADDRESS && RTA_PAYLOAD(attr) == MAC_SIZE) { const auto* mac = static_cast(RTA_DATA(attr)); result.mac = MacAddress::FromBytes(std::span{mac, MAC_SIZE}); @@ -200,8 +204,8 @@ void ResolveViaNetlink(unsigned index, InterfaceAddresses& result) noexcept { }); std::vector candidates; - NetlinkDump(fd, RTM_GETADDR, 2, [&](const nlmsghdr* header) { - const auto* addr = static_cast(NLMSG_DATA(header)); + NetlinkDump(fd, RTM_GETADDR, 2, [&](nlmsghdr* header) { + const auto* addr = start_lifetime_as(NLMSG_DATA(header)); if (addr->ifa_index != index) { return; } @@ -213,7 +217,8 @@ void ResolveViaNetlink(unsigned index, InterfaceAddresses& result) noexcept { const std::byte* address_attr = nullptr; const std::byte* local_attr = nullptr; int length = IFA_PAYLOAD(header); - for (const rtattr* attr = IFA_RTA(addr); RTA_OK(attr, length); attr = RTA_NEXT(attr, length)) { + for (auto* attr = start_lifetime_as(IFA_RTA(addr)); RTA_OK(attr, length); + attr = start_lifetime_as(RTA_NEXT(attr, length))) { switch (attr->rta_type) { case IFA_ADDRESS: address_attr = static_cast(RTA_DATA(attr)); @@ -222,7 +227,7 @@ void ResolveViaNetlink(unsigned index, InterfaceAddresses& result) noexcept { local_attr = static_cast(RTA_DATA(attr)); break; case IFA_FLAGS: - flags = *static_cast(RTA_DATA(attr)); + flags = *start_lifetime_as(RTA_DATA(attr)); break; default: break; @@ -329,7 +334,11 @@ void ResolveViaGetifaddrs(std::string_view interface, InterfaceAddresses& result } break; case AF_INET6: { - const auto& sin6 = *reinterpret_cast(ifa->ifa_addr); + // Copy rather than cast-and-read: getifaddrs owns the storage and FromSockaddr below + // still reads it as a sockaddr, so beginning a sockaddr_in6's lifetime over it would + // just move the aliasing problem there. The copy is defined either way. + sockaddr_in6 sin6{}; + std::memcpy(&sin6, ifa->ifa_addr, sizeof(sin6)); if (!IsUsableIpv6(inet6_fd, ifa->ifa_name, sin6)) { break; } diff --git a/src/reflector/ip_address.cpp b/src/reflector/ip_address.cpp index c2d0d95..183c46d 100644 --- a/src/reflector/ip_address.cpp +++ b/src/reflector/ip_address.cpp @@ -4,6 +4,7 @@ #include "reflector/logger.h" #include "reflector/platform.h" #include "reflector/util/narrow_cast.h" +#include "reflector/util/start_lifetime_as.h" #include #include @@ -185,10 +186,13 @@ std::optional IpAddress::FromSockaddr(const sockaddr* address) noexce socklen_t IpAddress::ToSockaddr(sockaddr_storage& storage, uint16_t port, unsigned scope_id) const noexcept { std::memset(&storage, 0, sizeof(storage)); + // start_lifetime_as, not a reinterpret_cast: it begins the family struct's lifetime over the + // zeroed bytes, so the member writes below go to a real object instead of aliasing one that + // was never created there. switch (family_) { using enum Family; case V4: { - auto* v4 = reinterpret_cast(&storage); + auto* v4 = start_lifetime_as(&storage); v4->sin_family = AF_INET; v4->sin_port = htons(port); std::memcpy(&v4->sin_addr, bytes_.data(), sizeof(v4->sin_addr)); @@ -201,7 +205,7 @@ socklen_t IpAddress::ToSockaddr(sockaddr_storage& storage, uint16_t port, unsign return sizeof(sockaddr_in); } case V6: { - auto* v6 = reinterpret_cast(&storage); + auto* v6 = start_lifetime_as(&storage); v6->sin6_family = AF_INET6; v6->sin6_port = htons(port); v6->sin6_scope_id = NeedsScopeId(*this) ? scope_id : 0; diff --git a/src/reflector/port_reservation.cpp b/src/reflector/port_reservation.cpp index 055dec3..bfa5854 100644 --- a/src/reflector/port_reservation.cpp +++ b/src/reflector/port_reservation.cpp @@ -2,6 +2,7 @@ #include "error.h" #include "logger.h" +#include "util/start_lifetime_as.h" #include #include @@ -61,9 +62,11 @@ std::optional PortReservation::Create(const IpAddress& source_i close(fd); return std::nullopt; } + // getsockname filled `bound`'s bytes; begin the family struct's lifetime over them so the port + // read is from a real object, not a cast to one that was never created there. const uint16_t port = v6 - ? ntohs(reinterpret_cast(&bound)->sin6_port) - : ntohs(reinterpret_cast(&bound)->sin_port); + ? ntohs(start_lifetime_as(&bound)->sin6_port) + : ntohs(start_lifetime_as(&bound)->sin_port); return PortReservation{fd, port}; } diff --git a/src/reflector/tcp_socket.cpp b/src/reflector/tcp_socket.cpp index 63d2664..be66d3b 100644 --- a/src/reflector/tcp_socket.cpp +++ b/src/reflector/tcp_socket.cpp @@ -6,6 +6,7 @@ #include "platform.h" #include "util/fd_util.h" #include "util/narrow_cast.h" +#include "util/start_lifetime_as.h" #include #include @@ -82,7 +83,9 @@ Logger& GetLogger() noexcept { GetLogger().Error("Cannot read local endpoint for fd {}: {}", fd, Error::FromErrno()); return std::nullopt; } - return IpEndpoint::FromSockaddr(reinterpret_cast(&addr)); + // getsockname filled the bytes; begin a sockaddr's lifetime over them for FromSockaddr's + // sa_family read (it memcpy's the rest out itself). + return IpEndpoint::FromSockaddr(start_lifetime_as(&addr)); } } // namespace @@ -253,7 +256,7 @@ std::optional TcpSocket::Accept() noexcept { ::close(client); return std::nullopt; } - return TcpSocket{client, *local, IpEndpoint::FromSockaddr(reinterpret_cast(&peer))}; + return TcpSocket{client, *local, IpEndpoint::FromSockaddr(start_lifetime_as(&peer))}; } bool TcpSocket::FinishConnect() noexcept { diff --git a/src/reflector/util/start_lifetime_as.h b/src/reflector/util/start_lifetime_as.h index 8684146..770fc81 100644 --- a/src/reflector/util/start_lifetime_as.h +++ b/src/reflector/util/start_lifetime_as.h @@ -17,6 +17,10 @@ namespace reflector { // memmove implicitly creates the implicit-lifetime object (P0593) and the self-copy keeps the bytes. // is_trivially_copyable_v is the exact precondition — it implies implicit-lifetime (so std accepts it // too) and is precisely what memmove can reconstitute. +// +// Mutable storage only: the fallback's memmove writes, so there is no const overload. To read a +// const-qualified or foreign-owned buffer as a T, memcpy it into a local instead (see +// IpAddress::FromSockaddr). template requires std::is_trivially_copyable_v [[nodiscard]] T* start_lifetime_as(void* p) noexcept {