cron2 has uploaded a new patch set (#4) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/454?usp=email )
The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: Fix unaligned access in macOS, FreeBSD, Solaris hwaddr ...................................................................... Fix unaligned access in macOS, FreeBSD, Solaris hwaddr The undefined behaviour USAN clang checker found this. This fix is a bit messy but so are the original structures. Since the API on Solaris/Illuminos does not return the AF_LINK sockaddr type we are interested in, there is little value in fixing the code on that platform to iterate through a list that does not contain the element we are looking for. Add includes stddef.h for offsetof and integer.h for max_int. Change-Id: Ia797c8801fa9a9bc10b6674efde5fdbd7132e4a8 Signed-off-by: Arne Schwabe <a...@rfc2549.org> Acked-by: Gert Doering <g...@greenie.muc.de> Message-Id: <20231231173431.31356-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27885.html Signed-off-by: Gert Doering <g...@greenie.muc.de> --- M src/openvpn/route.c 1 file changed, 46 insertions(+), 11 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/54/454/4 diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 6cc112c..0e6667f 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -24,6 +24,7 @@ /* * Support routines for adding/deleting network routes. */ +#include <stddef.h> #ifdef HAVE_CONFIG_H #include "config.h" @@ -40,6 +41,7 @@ #include "win32.h" #include "options.h" #include "networking.h" +#include "integer.h" #include "memdbg.h" @@ -3639,11 +3641,15 @@ rgi->flags |= RGI_NETMASK_DEFINED; } +#if !defined(TARGET_SOLARIS) + /* Illumos/Solaris does not provide AF_LINK entries when calling the + * SIOCGIFCONF API, so there is little sense to trying to figure out a + * MAC address from an API that does not provide that information */ + /* try to read MAC addr associated with interface that owns default gateway */ if (rgi->flags & RGI_IFACE_DEFINED) { struct ifconf ifc; - struct ifreq *ifr; const int bufsize = 4096; char *buffer; @@ -3668,22 +3674,50 @@ for (cp = buffer; cp <= buffer + ifc.ifc_len - sizeof(struct ifreq); ) { - ifr = (struct ifreq *)cp; -#if defined(TARGET_SOLARIS) - const size_t len = sizeof(ifr->ifr_name) + sizeof(ifr->ifr_addr); -#else - const size_t len = sizeof(ifr->ifr_name) + max(sizeof(ifr->ifr_addr), ifr->ifr_addr.sa_len); -#endif + struct ifreq ifr = { 0 }; + /* this is not always using an 8 byte alignment that struct ifr + * requires. Need to memcpy() to a strict ifr to force 8-byte + * alignment required for member access */ + memcpy(&ifr, cp, sizeof(struct ifreq)); + const size_t len = sizeof(ifr.ifr_name) + max(sizeof(ifr.ifr_addr), ifr.ifr_addr.sa_len); - if (!ifr->ifr_addr.sa_family) + if (!ifr.ifr_addr.sa_family) { break; } - if (!strncmp(ifr->ifr_name, rgi->iface, IFNAMSIZ)) + if (!strncmp(ifr.ifr_name, rgi->iface, IFNAMSIZ)) { - if (ifr->ifr_addr.sa_family == AF_LINK) + if (ifr.ifr_addr.sa_family == AF_LINK) { - struct sockaddr_dl *sdl = (struct sockaddr_dl *)&ifr->ifr_addr; + /* This is a confusing member access on multiple levels. + * + * struct sockaddr_dl is 20 bytes (on macOS and NetBSD, + * larger on other BSDs) in size and has + * 12 bytes space for the Ethernet interface name + * (max 16 bytes) and hw address (6 bytes) + * + * So if the interface name is more than 6 byte, the + * location of hwaddr extends beyond the struct. + * + * This struct is embedded into ifreq that has + * 16 bytes for a sockaddr and also expects this + * struct to potentially extend beyond the bounds of + * the struct. + * + * We only copied 32 bytes (size of ifr at least on macOS + * might differ on other platforms again) from cp to ifr. + * + * But as hwaddr might extend but sdl might extend beyond + * ifr's. So we need recalculate how large the actual size + * of the embedded dl_sock actually is and then also need + * to copy it since it also most likely does not have the + * proper alignment required to access the struct. + */ + const size_t sock_dl_len = max_int((int) (sizeof(struct sockaddr_dl)), + (int) (ifr.ifr_addr.sa_len)); + + struct sockaddr_dl *sdl = gc_malloc(sock_dl_len, true, &gc); + memcpy(sdl, cp + offsetof(struct ifreq, ifr_addr), sock_dl_len); memcpy(rgi->hwaddr, LLADDR(sdl), 6); rgi->flags |= RGI_HWADDR_DEFINED; } @@ -3691,6 +3725,7 @@ cp += len; } } +#endif /* if !defined(TARGET_SOLARIS) */ done: if (sockfd >= 0) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/454?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia797c8801fa9a9bc10b6674efde5fdbd7132e4a8 Gerrit-Change-Number: 454 Gerrit-PatchSet: 4 Gerrit-Owner: plaisthos <arne-open...@rfc2549.org> Gerrit-Reviewer: cron2 <g...@greenie.muc.de> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-MessageType: newpatchset
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
