Re: [PATCH net] udp: fix behavior of wrong checksums
From: Eric Dumazet <hidden>
Date: 2015-06-02 06:38:13
On Sun, 2015-05-31 at 21:42 -0700, David Miller wrote:
From: Eric Dumazet <redacted> Date: Sat, 30 May 2015 09:16:53 -0700quoted
From: Eric Dumazet <edumazet@google.com> We have two problems in UDP stack related to bogus checksums : 1) We return -EAGAIN to application even if receive queue is not empty. This breaks applications using edge trigger epoll() 2) Under UDP flood, we can loop forever without yielding to other processes, potentially hanging the host, especially on non SMP. This patch is an attempt to make things better. We might in the future add extra support for rt applications wanting to better control time spent doing a recv() in a hostile environment. For example we could validate checksums before queuing packets in socket receive queue. Signed-off-by: Eric Dumazet <edumazet@google.com>Yeah this looks correct, applied and queued up for -stable. Thanks!
Thanks David
It seems MSG_PEEK support is broken with checksums and truncated data:
Multiple threads could operate on same skb and
__udp_lib_checksum_complete() / __skb_checksum_complete() can do bad
things.
Tom, any idea how this could be fixed ? It looks you added this bug in
3.16
/*
* If checksum is needed at all, try to do it while copying the
* data. If the data is truncated, or if we only want a partial
* coverage checksum (UDP-Lite), do it before the copy.
*/
if (copied < ulen || UDP_SKB_CB(skb)->partial_cov) {
if (udp_lib_checksum_complete(skb))
goto csum_copy_err;
}
...
__sum16 __skb_checksum_complete(struct sk_buff *skb)
{
__wsum csum;
__sum16 sum;
csum = skb_checksum(skb, 0, skb->len, 0);
/* skb->csum holds pseudo checksum */
sum = csum_fold(csum_add(skb->csum, csum));
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
netdev_rx_csum_fault(skb->dev);
}
/* Save full packet checksum */
skb->csum = csum;
skb->ip_summed = CHECKSUM_COMPLETE;
skb->csum_complete_sw = 1;
skb->csum_valid = !sum;
return sum;
}
Maybe adding a 'readonly' bool param to __skb_checksum_complete() ?