netdev - Re: [PATCH net] ipv6: don't use CHECKSUM_PARTIAL on MSG_MORE/UDP

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <1445421086.1167705.416135513.62D75196@webmail.messagingengine.com>
Date:	Wed, 21 Oct 2015 11:51:26 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Vlad Yasevich <vyasevich@...il.com>, netdev@...r.kernel.org
Cc:	sd@...sysnail.net, bcodding@...hat.com
Subject: Re: [PATCH net] ipv6: don't use CHECKSUM_PARTIAL on MSG_MORE/UDP_CORK
 sockets

Hi Vlad,

On Tue, Oct 20, 2015, at 23:39, Vlad Yasevich wrote:
> On 10/20/2015 10:38 AM, Hannes Frederic Sowa wrote:
> > MSG_MORE might cause the packet to get fragmented in the end when
> > passed down to the flush function and the transhdrlen check alone is
> > not sufficient to protect against fragmentation. Instead check if the
> > socket user intends to add more data to the socket on the first packet.
> > 
> > This broke checksum calculation for UDPv6 for NFS protocols.
> > 
> > Fixes: 32dce968dd987 ("ipv6: Allow for partial checksums on non-ufo packets")
> > Cc: Vlad Yasevich <vyasevich@...il.com>
> > Tested-by: Sabrina Dubroca <sd@...sysnail.net>
> > Tested-by: Benjamin Coddington <bcodding@...hat.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@...essinduktion.org>
> > ---
> >  net/ipv6/ip6_output.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index 61d403e..95c5780 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -1317,6 +1317,7 @@ emsgsize:
> >  	 * sums only work when transhdrlen is set.
> >  	 */
> >  	if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
> > +	    !(flags & MSG_MORE) &&
> >  	    length + fragheaderlen < mtu &&
> >  	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
> >  	    !exthdrlen)
> > 
> 
> Hmm... so while this solves this problem by simply avoiding the
> combination of
> skb #1 having CHECKSUM_PARTIAL and others having CHECKSUM_NONE, I think
> the actual
> problem is a bit deeper.
> The above combination seems to work for me since udp6_hwcsum_outgoing()
> corrects
> the checksum.  However, my testing so far has been on nics that have
> NETIF_F_V6_CSUM,
> but without UFO support.

With nfs tests we never branch into setting up or extending an UDP UFO
packet, also because on the test system UFO is disabled on all
interfaces.

I thought about relaxing this check in future when we simply make sure
we don't do fragmentation based based on the length while taking all
fragments into account.

> On such systems a simple test of using MSG_MORE an IPv6 udp socket
> sending 200 bytes
> followed by 2000 bytes works correctly.

Did you make sure it actually fragmented and the checksums are correct?

> I am now wondering if this might be UFO related instead and looking for a
> nic that
> has UFO support.

So far as I can see it has nothing to do with UFO. I will do more
investigation now. Thanks for bringing this up!

Bye,
Hannes
--
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