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
| ||
|
Date: Sun, 28 Feb 2010 06:31:11 +0100 From: Eric Dumazet <eric.dumazet@...il.com> To: Zhu Yi <yi.zhu@...el.com> Cc: netdev@...r.kernel.org, David Miller <davem@...emloft.net> Subject: Re: [PATCH V2] net: add accounting for socket backlog Le vendredi 26 février 2010 à 17:27 +0800, Zhu Yi a écrit : > 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 patch fixed this problem by adding accounting for the socket > backlog. So that the backlog size can be restricted by protocol's choice > (i.e. UDP). > > Reported-by: Alex Shi <alex.shi@...el.com> > Cc: David Miller <davem@...emloft.net> > Cc: Eric Dumazet <eric.dumazet@...il.com> > Signed-off-by: Zhu Yi <yi.zhu@...el.com> > --- > V2: remove atomic operation for sk_backlog.len > limit UDP backlog size to 2*sk->sk_rcvbuf > > + > static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb) > { > + sk->sk_backlog.len -= skb->truesize; > return sk->sk_backlog_rcv(sk, skb); > } > I am afraid sk_backlog_rcv() is not always called with lock held, and not always called to process backlog (see TCP ucopy.prequeue) If you take a look at __release_sock() for example, we make the backlog private to the process before handling it (outside of lock_sock()) Therefore, I suggest doing the 'substraction' outside of sk_backlog_rcv(). diff --git a/net/core/sock.c b/net/core/sock.c index e1f6f22..57271cb 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1520,6 +1520,7 @@ static void __release_sock(struct sock *sk) do { sk->sk_backlog.head = sk->sk_backlog.tail = NULL; + sk->sk_backlog.len = 0; bh_unlock_sock(sk); do { Ah, I see __release_sock() is already doing a preemption check, please ignore my previous comment, when I said "__release_sock() could run forever with no preemption, even with a limit on backlog" Thanks -- 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
Powered by blists - more mailing lists