netdev - Re: [BUG] behaviour mismatch between ipv4 and ipv6 in UDP rx path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1297889942.2645.22.camel@edumazet-laptop>
Date:	Wed, 16 Feb 2011 21:59:02 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Chris Friesen <chris.friesen@...band.com>
Cc:	netdev@...r.kernel.org, Herbert Xu <herbert@...dor.apana.org.au>
Subject: Re: [BUG]  behaviour mismatch between ipv4 and ipv6 in UDP rx path

Le mercredi 16 février 2011 à 14:18 -0600, Chris Friesen a écrit :
> Hi,
> 
> I sent this out a week ago but didn't see a reply, so I'm sending it out
> again.
> 
> One of our guys is seeing occasional dropped ipv4 packets coming in on
> an ipv6 udp socket obtained via socket(AF_INET6,  SOCK_DGRAM, IPPROTO_UDP).
> 
> Here's what he says:
> 
> 
> "The problem happens when release_sock() goes down an interesting code
> path.  If (sk->sk_backlog.tail) is non-NULL then release_sock() invokes
> __release_sock() which loops over all queue packets and invokes the
> socket's backlog receive function for each previously queued packet.
> 
> Now for the interesting part.  The UDPv6 backlog receive function (in
> net/ipv6/udp.c, udpv6_queue_rcv_skb()) invokes xfrm6_policy_check() to
> confirm that the packet is allowed, but the problem is that it calls
> this function regardless of whether the packet is IPv4 or IPv6.  The
> xfrm6_policy_check() function then assumes that it is an IPv6 packet and
> tries to match a policy based on its packet header... but that clearly
> won't work because the addresses that it finds when it decodes the skb
> are completely bogus."
> 
> 
> Looking at the ipv4 code, git commit 9382177 split __udp_queue_rcv_skb()
> out of udp_queue_rcv_skb().  It was done for locking purposes, but it
> also means that backlog_rcv is bound to __udp_queue_rcv_skb(), which
> doesn't call xfrm4_policy_check().
> 
> 
> Should a new function __udpv6_queue_rcv_skb() be split out from
> udpv6_queue_rcv_skb() and bound to backlog_rcv to resolve the xfrm
> issue?  What about the locking that was the reason for the split in the
> ipv4 case--is there a similar problem with ipv6?
> 


Yes, please submit a patch ?

Ideally, __udp_queue_rcv_skb() should be the common .backlog

In practice, because of sock_rps_save_rxhash() and MIB counters, I
suspect a __udp6_queue_rcv_skb() is OK.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html