ovpn_peer_endpoints_update() builds the new remote endpoint in an on-stack struct sockaddr_storage that is left uninitialized. For IPv4 only sin_family/sin_addr/sin_port are written, leaving the 8-byte sin_zero padding as stack garbage (for IPv6, sin6_flowinfo is left uninitialized likewise). ovpn_peer_reset_sockaddr() -> ovpn_bind_from_sockaddr() then memcpy()s sizeof(struct sockaddr_in)/sizeof(struct sockaddr_in6) bytes - padding included - into bind->remote. That buffer is later hashed with jhash() over the same length to place the peer in the by_transp_addr table, so the garbage padding lands the floated peer in an essentially random bucket. Lockless lookups in ovpn_peer_get_by_transp_addr() build their key from a zero-initialized sockaddr_storage, compute a different bucket and fail to find the peer. This is also a plain use of uninitialized stack memory in jhash(). Build the floated endpoint with a designated initializer so the padding (sin_zero for IPv4, sin6_flowinfo for IPv6) is zeroed as part of the assignment. This keeps the padding out of the by_transp_addr hash key without memset-ing the whole sockaddr_storage on every received packet. Fixes: f0281c1d3732 ("ovpn: add support for updating local or remote UDP endpoint") Signed-off-by: Antonio Quartulli --- drivers/net/ovpn/peer.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index a330892e82bf..33fb0a75e600 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c @@ -222,9 +222,16 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb) */ local_ip = &ip_hdr(skb)->daddr; sa = (struct sockaddr_in *)&ss; - sa->sin_family = AF_INET; - sa->sin_addr.s_addr = ip_hdr(skb)->saddr; - sa->sin_port = udp_hdr(skb)->source; + /* use a designated initializer so the sin_zero padding + * is zeroed (it ends up in the by_transp_addr hash key) + * without memset-ing the whole sockaddr_storage on the + * RX fast path + */ + *sa = (struct sockaddr_in) { + .sin_family = AF_INET, + .sin_addr.s_addr = ip_hdr(skb)->saddr, + .sin_port = udp_hdr(skb)->source, + }; salen = sizeof(*sa); reset_cache = true; break; @@ -250,11 +257,19 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb) */ local_ip = &ipv6_hdr(skb)->daddr; sa6 = (struct sockaddr_in6 *)&ss; - sa6->sin6_family = AF_INET6; - sa6->sin6_addr = ipv6_hdr(skb)->saddr; - sa6->sin6_port = udp_hdr(skb)->source; - sa6->sin6_scope_id = ipv6_iface_scope_id(&ipv6_hdr(skb)->saddr, - skb->skb_iif); + /* use a designated initializer so the sin6_flowinfo + * padding is zeroed (it ends up in the by_transp_addr + * hash key) without memset-ing the whole + * sockaddr_storage on the RX fast path + */ + *sa6 = (struct sockaddr_in6) { + .sin6_family = AF_INET6, + .sin6_addr = ipv6_hdr(skb)->saddr, + .sin6_port = udp_hdr(skb)->source, + .sin6_scope_id = + ipv6_iface_scope_id(&ipv6_hdr(skb)->saddr, + skb->skb_iif), + }; salen = sizeof(*sa6); reset_cache = true; break; -- 2.54.0