refactor: read populated kernel structs via start_lifetime_as#49
Merged
Conversation
Where the code reinterpret_cast populated storage to a struct and read through it — the family sockaddr in ToSockaddr and the getsockname / accept reads, and the netlink message/attribute walks — begin the struct's lifetime over those bytes with start_lifetime_as instead. The read then goes through a real object rather than one that was never created there. The netlink walks bless each derived pointer before its first dereference: the NLMSG_*/RTA_* macros only compute addresses and read fields of the already-blessed current struct, so nlmsghdr, its NLMSG_DATA body, nlmsgerr, the rtattr chain, and the IFA_FLAGS payload are each wrapped in turn. CollectChangedInterfaces now takes a mutable span<std::byte> since the shim reuses that storage. Left as plain casts by design: casts handed to a syscall (the kernel touches raw bytes, no C++ access), byte->char reads for a string_view (a permitted alias direction), and the bpf/sock_filter casts fed to setsockopt/ioctl. Full gate green: native unit, docker debug/release, docker valgrind unit, e2e, and e2e under valgrind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit of the codebase's
reinterpret_cast/memcpysites for the ones that reinterpret populated storage as a struct and then read/write through it — formally UB (no object of that type was created there). Those move ontoreflector::start_lifetime_as, which begins the struct's lifetime over the existing bytes so the access lands on a real object.Converted:
IpAddress::ToSockaddr— the family sockaddr the member writes go to (post-memset).port_reservation/tcp_socket— thegetsocknameandacceptreads ofsin_port/sa_family.CollectChangedInterfacesmonitor +NetlinkDump/ResolveViaNetlinkresolver): every struct read is blessed before its first dereference —nlmsghdr(init + eachNLMSG_NEXT), theNLMSG_DATAbodies (ifaddrmsg/ifinfomsg),nlmsgerr, thertattrchains (IFLA_RTA/IFA_RTA+ eachRTA_NEXT), and theIFA_FLAGSuint32_tpayload. The macros only compute addresses and read fields of the already-blessed current struct, so per-step blessing keeps every access on a live object.CollectChangedInterfacesnow takes a mutablespan<std::byte>(the shim reuses storage).Left as plain casts, deliberately:
bind/getsockname/connect/recvfrom/accept) — the kernel reads/writes raw bytes outside the C++ abstract machine; nosockaddrobject is accessed in our code.reinterpret_cast<const char*>(byte_span.data())forstring_viewconstruction —charmay alias any object, and this is the permitted direction (reading bytes as chars).bpf_insn/sock_filtercasts — fed tosetsockopt/ioctl(syscall boundary).memcpyinto a local, sinceFromSockaddrre-reads the same storage as asockaddrregardless.start_lifetime_as.hgains a note that it's mutable-storage-only (the fallback'smemmovewrites; no const overload).Full gate green: native unit (ASan/UBSan) 847/847, docker debug/release 840/840, docker valgrind 0 errors, e2e 33/33 plain and under valgrind.