netdev - Re: [PATCH net-next 3/3] tcp: implement head drops in backlog queue

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CANn89i+=zn0pTWO79hAdWV1MBsjkdHb+2ia9dnx+L_AtPLg8OA@mail.gmail.com>
Date:   Wed, 21 Nov 2018 17:01:10 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     Yuchung Cheng <ycheng@...gle.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Jean-Louis Dupond <jean-louis@...ond.be>,
        Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [PATCH net-next 3/3] tcp: implement head drops in backlog queue

On Wed, Nov 21, 2018 at 4:55 PM Yuchung Cheng <ycheng@...gle.com> wrote:
>
> To clarify I do think this patch set is overall useful so I only
> wanted to discuss the specifics of the head drop.
>
> It occurs to me we check the limit differently (one w/ 64KB more), so
> we may overcommit and trim more often than necessary?

Keep in mind that the normal limit might be hit while backlog is empty.

So the loop might have nothing to remove.

We only have an extra 64KB credit for this very particular case, so
that we still can queue this packet
_if_ the additional credit was not already consumed.

We do not want to add back the terrible bug fixed by :

commit 8eae939f1400326b06d0c9afe53d2a484a326871
Author: Zhu Yi <yi.zhu@...el.com>
Date:   Thu Mar 4 18:01:40 2010 +0000

    net: add limit for socket backlog

    We got system OOM while running some UDP netperf testing on the loopback
    device. The case is multiple senders sent stream UDP packets to a single
    receiver via loopback on local host. Of course, the receiver is not able
    to handle all the packets in time. But we surprisingly found that these
    packets were not discarded due to the receiver's sk->sk_rcvbuf limit.
    Instead, they are kept queuing to sk->sk_backlog and finally ate up all
    the memory. We believe this is a secure hole that a none privileged user
    can crash the system.

    The root cause for this problem is, when the receiver is doing
    __release_sock() (i.e. after userspace recv, kernel udp_recvmsg ->
    skb_free_datagram_locked -> release_sock), it moves skbs from backlog to
    sk_receive_queue with the softirq enabled. In the above case, multiple
    busy senders will almost make it an endless loop. The skbs in the
    backlog end up eat all the system memory.

    The issue is not only for UDP. Any protocols using socket backlog is
    potentially affected. The patch adds limit for socket backlog so that
    the backlog size cannot be expanded endlessly.

    Reported-by: Alex Shi <alex.shi@...el.com>
    Cc: David Miller <davem@...emloft.net>
    Cc: Arnaldo Carvalho de Melo <acme@...stprotocols.net>
    Cc: Alexey Kuznetsov <kuznet@....inr.ac.ru
    Cc: "Pekka Savola (ipv6)" <pekkas@...core.fi>
    Cc: Patrick McHardy <kaber@...sh.net>
    Cc: Vlad Yasevich <vladislav.yasevich@...com>
    Cc: Sridhar Samudrala <sri@...ibm.com>
    Cc: Jon Maloy <jon.maloy@...csson.com>
    Cc: Allan Stephens <allan.stephens@...driver.com>
    Cc: Andrew Hendry <andrew.hendry@...il.com>
    Signed-off-by: Zhu Yi <yi.zhu@...el.com>
    Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
    Acked-by: Arnaldo Carvalho de Melo <acme@...hat.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>