Attention is currently required from: flichtenheld, plaisthos.
Hello cron2, flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/454?usp=email
to look at the new patch set (#2).
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.
Change-Id: Ia797c8801fa9a9bc10b6674efde5fdbd7132e4a8
Signed-off-by: Arne Schwabe <[email protected]>
---
M src/openvpn/route.c
1 file changed, 38 insertions(+), 11 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/54/454/2
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index ff64938..f8cfa53 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -3639,11 +3639,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 +3672,44 @@
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 */
+ 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 in size and has
+ * 12 bytes space for the hw address (6 bytes)
+ * and Ethernet interface name (max 16 bytes)
+ *
+ * So if the interface name is more than 6 byte, it
+ * 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.
+ *
+ * Since we only copied 32 bytes from cp to ifr but sdl
+ * might extend after ifr's end, we need to copy from
+ * cp directly to avoid missing out on extra bytes
+ * behind 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 +3717,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: 2
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
