From patchwork Tue Sep 7 19:26:09 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 64055 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 7FB47B6EEB for ; Wed, 8 Sep 2010 05:26:21 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757781Ab0IGT0R (ORCPT ); Tue, 7 Sep 2010 15:26:17 -0400 Received: from mail-fx0-f46.google.com ([209.85.161.46]:35787 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757519Ab0IGT0P (ORCPT ); Tue, 7 Sep 2010 15:26:15 -0400 Received: by fxm16 with SMTP id 16so242711fxm.19 for ; Tue, 07 Sep 2010 12:26:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=2pvjy+QzVl+KiLrcrn3XlN4aIie6q75OzAfDddcRku8=; b=iAfDwmPUsUDQUg9bpX9sATzaGvY/O4y+Svsoia5sEnD1re7FSyBHC430azONwVTn/y NruhEE34ixKrTZQHJF7r3kgRTyH41scEXusMMnWaBCbm49/AAqXfg8YngmpbCcKMSibi pfYHFQYaPglcGU+b/J0i48jXf9rIaFkuyjpMw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=Zss+Yx19Z4cUENIxbS0OEb22WEFJMEhLqGfxWqGL1OIzmYCW13qkET7QXVixi+WZEE aft9cr3ZMwp7zwEmmGlBdvVGf/Yt3Z0uYwEOysAH6S4tXrM9agcDM5okRlAy3ncVeiHg nLxpeGDYbcDyv+fJMWniICcJQJbTBIkGFKunw= Received: by 10.223.116.6 with SMTP id k6mr212836faq.49.1283887573688; Tue, 07 Sep 2010 12:26:13 -0700 (PDT) Received: from [10.150.51.211] (gw0.net.jmsp.net [212.23.165.14]) by mx.google.com with ESMTPS id a7sm3061229faa.45.2010.09.07.12.26.11 (version=SSLv3 cipher=RC4-MD5); Tue, 07 Sep 2010 12:26:12 -0700 (PDT) Subject: Re: 2.6.34: Problem with UDP traffic on lo + poll(?) From: Eric Dumazet To: Krzysztof =?UTF-8?Q?Ol=C4=99dzki?= , David Miller Cc: netdev@vger.kernel.org In-Reply-To: <1283877391.2313.62.camel@edumazet-laptop> References: <1283802132.2585.4.camel@edumazet-laptop> <4C854737.5040503@ans.pl> <1283804955.2585.12.camel@edumazet-laptop> <4C8552B1.8020806@ans.pl> <4C855385.7030203@ans.pl> <4C865C21.5010803@ans.pl> <1283877391.2313.62.camel@edumazet-laptop> Date: Tue, 07 Sep 2010 21:26:09 +0200 Message-ID: <1283887569.2634.95.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le mardi 07 septembre 2010 à 18:36 +0200, Eric Dumazet a écrit : > Hmm, I have a pretty good idea of what the problem is, and will post a > fix soon ;) David, if you feel this is too invasive for stable, we can make UDP rehash the socket in case we dont want to change ip4_datagram_connect() [PATCH] inet: Dont set inet_rcv_saddr in connect() So the problem is that the sequence : socket(PF_INET, SOCK_DGRAM, IPPROTO_IP) connect(fd, {sa_family=AF_INET, sin_port=htons(xx), sin_addr=inet_addr("1.2.3.4")}, 28) 1) Does an implicit inet_autobind() (using an INADDR_ANY address, and selecting a random port). 2) Then does an ip4_datagram_connect() to specify the address/port of remote end point. Problem is ip4_datagram_connect() also sets inet->inet_rcv_saddr (from INADDR_ANY to IP source address, given the current route to remote end point). Only the first connect() on the socket does this. Following ones dont change the (possibly wrong) source address. This breaks the secondary UDP hash, based on (ADDRESS, port), that was computed by inet_autobind(). This also potentially breaks multiple connect() to change remote endpoints, because old source address might be non usable for packets to new destination. If route happens to change, then we should automatically change our source address too, at next sendmsg() call, and UDP code deals with this just fine. If an application needs to specify a precise source address, it must use bind() system call. connect() man page only refers to remote address, not local one. Reported-by: Krzysztof Olędzki Signed-off-by: Eric Dumazet Tested-by: Krzysztof Piotr Oledzki --- net/ipv4/datagram.c | 9 +++++++++ 1 file changed, 9 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c index f055094..8a17241 100644 --- a/net/ipv4/datagram.c +++ b/net/ipv4/datagram.c @@ -60,10 +60,19 @@ int ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len) ip_rt_put(rt); return -EACCES; } +/* + * Should connect() change inet_rcv_saddr ? + * It should not IMHO, because we want to specify the peer to which + * datagrams are to be sent, regardless of our source address that might + * change in the future, after a route change. + * To specify our source address, bind() is the right API. + */ +#if 0 if (!inet->inet_saddr) inet->inet_saddr = rt->rt_src; /* Update source address */ if (!inet->inet_rcv_saddr) inet->inet_rcv_saddr = rt->rt_src; +#endif inet->inet_daddr = rt->rt_dst; inet->inet_dport = usin->sin_port; sk->sk_state = TCP_ESTABLISHED;