| Message ID | 20241211171349.8892-1-gert@greenie.muc.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v3] forward: Fix potential unaligned access in drop_if_recursive_routing | expand |
Thanks for that. We had quite a discussion on IRC on why this is needed
and whether it's a bug in clang - turns out it is not, the linux
implementation of IN6_ARE_ADDR_EQUAL() uses 4x 32bit compare, which is
sensitive to alignment, while all the rest of the world uses memcmp()
which does not care... so this code now does the same on Linux,
and does not crash on "unaligned ipv6 header in an outgoing TAP packet"
(the buf is aligned, but the ethernet header misaligns everything).
Your patch has been applied to the master and release/2.6 branch
(minor bug).
Mildly tested on a FreeBSD and a Linux t_client setup (and, of course,
the buildbot army which uncovered this in the "tap" client tests).
Uncrustify complains about multline comment not having a "*", so
fixed on the fly.
commit 387c2076af14a0f1ba97b6ca0175d81d1e8391a5 (master)
commit 62d41dec8db8c87f9b005c168827bb3387ea5c65 (release/2.6)
Author: Frank Lichtenheld
Date: Wed Dec 11 18:13:48 2024 +0100
forward: Fix potential unaligned access in drop_if_recursive_routing
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241211171349.8892-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20241211171349.8892-1-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index d50b24c..2c72001 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1390,8 +1390,6 @@ if (proto_ver == 4) { - const struct openvpn_iphdr *pip; - /* make sure we got whole IP header */ if (BLEN(buf) < ((int) sizeof(struct openvpn_iphdr) + ip_hdr_offset)) { @@ -1404,18 +1402,16 @@ return; } - pip = (struct openvpn_iphdr *) (BPTR(buf) + ip_hdr_offset); + struct openvpn_iphdr *pip = (struct openvpn_iphdr *) (BPTR(buf) + ip_hdr_offset); /* drop packets with same dest addr as gateway */ - if (tun_sa.addr.in4.sin_addr.s_addr == pip->daddr) + if (memcmp(&tun_sa.addr.in4.sin_addr.s_addr, &pip->daddr, sizeof(pip->daddr)) == 0) { drop = true; } } else if (proto_ver == 6) { - const struct openvpn_ipv6hdr *pip6; - /* make sure we got whole IPv6 header */ if (BLEN(buf) < ((int) sizeof(struct openvpn_ipv6hdr) + ip_hdr_offset)) { @@ -1428,9 +1424,10 @@ return; } + struct openvpn_ipv6hdr *pip6 = (struct openvpn_ipv6hdr *) (BPTR(buf) + ip_hdr_offset); + /* drop packets with same dest addr as gateway */ - pip6 = (struct openvpn_ipv6hdr *) (BPTR(buf) + ip_hdr_offset); - if (IN6_ARE_ADDR_EQUAL(&tun_sa.addr.in6.sin6_addr, &pip6->daddr)) + if (OPENVPN_IN6_ARE_ADDR_EQUAL(&tun_sa.addr.in6.sin6_addr, &pip6->daddr)) { drop = true; } diff --git a/src/openvpn/proto.h b/src/openvpn/proto.h index 7b94fbc..ac70134 100644 --- a/src/openvpn/proto.h +++ b/src/openvpn/proto.h @@ -83,6 +83,12 @@ #define SIZE_ETH_TO_8021Q_HDR (sizeof(struct openvpn_8021qhdr) \ - sizeof(struct openvpn_ethhdr)) +/** Version of IN6_ARE_ADDR_EQUAL that is guaranteed to work for + unaligned access. E.g. Linux uses 32bit compares which are + not safe if the struct is unaligned. */ +#define OPENVPN_IN6_ARE_ADDR_EQUAL(a, b) \ + (memcmp(a, b, sizeof(struct in6_addr)) == 0) + struct openvpn_iphdr { #define OPENVPN_IPH_GET_VER(v) (((v) >> 4) & 0x0F) #define OPENVPN_IPH_GET_LEN(v) (((v) & 0x0F) << 2)