netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Krzysztof Olędzki" <ole@ans.pl>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>, netdev@vger.kernel.org
Subject: Re: [PATCH] inet: dont set inet_rcv_saddr in connect()
Date: Tue, 07 Sep 2010 23:52:02 +0200	[thread overview]
Message-ID: <4C86B402.5000205@ans.pl> (raw)
In-Reply-To: <1283895316.2634.248.camel@edumazet-laptop>

On 2010-09-07 23:35, Eric Dumazet wrote:
> Le mardi 07 septembre 2010 à 12:59 -0700, David Miller a écrit :
>
>> Eric, please just delete the code block instead of leaving it
>> there inside of an #if 0 block.
>>
>> If there is information conveyed by the unused code, add that
>> information to the nice comment you're adding :-)
>>
>
> Indeed ;)
>
> [PATCH] inet: dont set inet_rcv_saddr in connect()
>
> The following 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) 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(). If more than 10 sockets are linked in
> primary hash chain, we can drop incoming packets because searches are
> done in wrong secondary hash chain.
>
> 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 <ole@ans.pl>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>

Best regards,

			Krzysztof Olędzki

  reply	other threads:[~2010-09-07 21:52 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-06 17:11 2.6.34: Problem with UDP traffic on lo + poll(?) Krzysztof Oledzki
2010-09-06 19:42 ` Eric Dumazet
2010-09-06 19:55   ` Krzysztof Olędzki
2010-09-06 20:29     ` Eric Dumazet
2010-09-06 20:44       ` Krzysztof Olędzki
2010-09-06 20:48         ` Krzysztof Olędzki
2010-09-07 15:37           ` Krzysztof Olędzki
2010-09-07 16:36             ` Eric Dumazet
2010-09-07 19:20               ` Krzysztof Olędzki
2010-09-07 19:26               ` Eric Dumazet
2010-09-07 19:59                 ` David Miller
2010-09-07 21:35                   ` [PATCH] inet: dont set inet_rcv_saddr in connect() Eric Dumazet
2010-09-07 21:52                     ` Krzysztof Olędzki [this message]
2010-09-08  2:16                       ` David Miller
2010-09-08  4:13                         ` Eric Dumazet
2010-09-08  2:34                     ` Brian Haley
2010-09-08  3:34                       ` David Miller
2010-09-08  4:42                         ` Eric Dumazet
2010-09-08  5:51                           ` David Miller
2010-09-08  4:57                       ` Eric Dumazet
2010-09-08  5:36                         ` David Miller
2010-09-08  5:52                           ` Eric Dumazet
2010-09-08 10:10                             ` [PATCH] udp: add rehash on connect() Eric Dumazet
2010-09-08 15:06                               ` Krzysztof Olędzki
2010-09-08 15:17                                 ` Eric Dumazet
2010-09-08 15:29                                   ` Krzysztof Olędzki
2010-09-08 15:08                               ` [PATCH v2] " Eric Dumazet
2010-09-08 16:52                                 ` Krzysztof Olędzki
2010-09-09  4:39                                   ` David Miller
2010-09-08 14:27                             ` [PATCH] inet: dont set inet_rcv_saddr in connect() Eric Dumazet
2010-09-07 21:28                 ` 2.6.34: Problem with UDP traffic on lo + poll(?) Krzysztof Olędzki
2010-09-07 21:39                   ` Eric Dumazet
2010-09-07 21:51                     ` Krzysztof Olędzki
2010-09-08  4:12                       ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C86B402.5000205@ans.pl \
    --to=ole@ans.pl \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).