Thread (3 messages) 3 messages, 2 authors, 2015-06-02

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 -0700
quoted
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() ?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help