Re: CID 1452946, 1452957: Uninitialized scalar variable (bridge_ipsec)

[Alexander Bluhm]

On Wed, Aug 16, 2017 at 05:18:09PM +0200, Mike Belopuhov wrote:
> Hi,
> 
> In may this year, the condition that would make this break do the
> right thing got removed and now if a short packet is sent to an
> ipsec-enabled bridge, various things like 'spi' and 'off' are left
> uninitialized, but thankfully the gettdb call that follows will
> most likely fail when presented with a random spi value.  But it's
> a nasty bug nevertheless.
> 
> OK?

OK bluhm@

> 
> diff --git sys/net/if_bridge.c sys/net/if_bridge.c
> index 0e048205475..33d4753fd6b 100644
> --- sys/net/if_bridge.c
> +++ sys/net/if_bridge.c
> @@ -1404,11 +1404,11 @@ bridge_ipsec(struct bridge_softc *sc, struct ifnet 
> *ifp,
>  
>   if (dir == BRIDGE_IN) {
>   switch (af) {
>   case AF_INET:
>   if (m->m_pkthdr.len - hlen < 2 * sizeof(u_int32_t))
> - break;
> + goto skiplookup;
>  
>   ip = mtod(m, struct ip *);
>   proto = ip->ip_p;
>   off = offsetof(struct ip, ip_p);
>  
> @@ -1425,11 +1425,11 @@ bridge_ipsec(struct bridge_softc *sc, struct ifnet 
> *ifp,
>  
>   break;
>  #ifdef INET6
>   case AF_INET6:
>   if (m->m_pkthdr.len - hlen < 2 * sizeof(u_int32_t))
> - break;
> + goto skiplookup;
>  
>   ip6 = mtod(m, struct ip6_hdr *);
>  
>   /* XXX We should chase down the header chain */
>   proto = ip6->ip6_nxt;

CID 1452946, 1452957: Uninitialized scalar variable (bridge_ipsec)

[Mike Belopuhov]

Hi,

In may this year, the condition that would make this break do the
right thing got removed and now if a short packet is sent to an
ipsec-enabled bridge, various things like 'spi' and 'off' are left
uninitialized, but thankfully the gettdb call that follows will
most likely fail when presented with a random spi value.  But it's
a nasty bug nevertheless.

OK?

diff --git sys/net/if_bridge.c sys/net/if_bridge.c
index 0e048205475..33d4753fd6b 100644
--- sys/net/if_bridge.c
+++ sys/net/if_bridge.c
@@ -1404,11 +1404,11 @@ bridge_ipsec(struct bridge_softc *sc, struct ifnet *ifp,
 
if (dir == BRIDGE_IN) {
switch (af) {
case AF_INET:
if (m->m_pkthdr.len - hlen < 2 * sizeof(u_int32_t))
-   break;
+   goto skiplookup;
 
ip = mtod(m, struct ip *);
proto = ip->ip_p;
off = offsetof(struct ip, ip_p);
 
@@ -1425,11 +1425,11 @@ bridge_ipsec(struct bridge_softc *sc, struct ifnet *ifp,
 
break;
 #ifdef INET6
case AF_INET6:
if (m->m_pkthdr.len - hlen < 2 * sizeof(u_int32_t))
-   break;
+   goto skiplookup;
 
ip6 = mtod(m, struct ip6_hdr *);
 
/* XXX We should chase down the header chain */
proto = ip6->ip6_nxt;