Hi, On Mon, Jan 30, 2023 at 06:29:33PM +0100, Arne Schwabe wrote: > The undefined behaviour USAN clang checker found this. > > This fix is a bit messy but so are the original structures.
I wonder... > + struct sockaddr_dl sdl = { 0 }; > + memcpy(&sdl, &ifr.ifr_addr, sizeof(ifr.ifr_addr)); > + memcpy(rgi->hwaddr, LLADDR(&sdl), 6); > rgi->flags |= RGI_HWADDR_DEFINED; ... why you keep the "sockaddr_dl" intermediate - this was useful for "we use it as a pointer", but doing two memcpy() with a huge explanation above looks weird. LLADDR on FreeBSD is #define LLADDR(s) ((caddr_t)((s)->sdl_data + (s)->sdl_nlen)) (to operate on a "struct sockaddr_dl") struct ifaddr has struct ifaddr { struct sockaddr *ifa_addr; /* address of interface */ struct sockaddr *ifa_dstaddr; /* other end of p-to-p link */ struct sockaddr *ifa_netmask; /* used to determine subnet */ ... so the first memcpy() isn't actually copying the MAC address around, just a bunch of pointers, and then we dereference the first of them for the second memcpy(), using the dereferencing macro for a "sockaddr_dl" structure. FreeBSD also has #define IF_LLADDR(ifp) \ LLADDR((struct sockaddr_dl *)((ifp)->if_addr->ifa_addr)) which sounds like "feed in an *ifr, out comes a pointer for the memcpy() to rgi->addr". I'll check the other BSDs if LL_IFADDR() is available everywhere, but if not, we can just do the same thing (struct ifr is aligned) and get rid of the "sdl" and the first memcpy(). (Am I overlooking something?) gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel