RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of the local address to 0 (lnx)

[Florian Weimer]

* Florian Weimer:

> * Alan Bateman:
>
>> On 04/10/2019 16:25, Florian Weimer wrote:
>>> :
>>> @@ -873,10 +873,15 @@
>>>                   synchronized (stateLock) {
>>>                       ensureOpen();
>>>                       if (state == ST_CONNECTED)
>>>                           throw new AlreadyConnectedException();
>>>   +                    // ensure that the socket is bound
>>> +                    if (localAddress == null) {
>>> +                        bindInternal(null);
>>> +                    }
>>> +
>>>                       int n = Net.connect(family,
>>>                                           fd,
>>>                                           isa.getAddress(),
>>>                                           isa.getPort());
>>>                       if (n <= 0)
>>>
>>>
>>> Doesn't it introduce a race condition where the socket has a port and
>>> queue packets, but the kernel will not perform source address checking
>>> because it is unconnected?
>>>
>> The socket is unbound, this change to the connect method in Daniel's
>> patch just binds it before connecting. The kernel will do this anyway
>> so it's not strictly needed here (at least for the case that the
>> connect succeeds_.
>
> The kernel will do it atomically during connect, so there is no race
> condition.  With the change above, the port is selected and opened,
> packets can arrive (from arbitrary addresses), and only then the socket
> is bound to the remote address.

Ah, sorry, there is this in connect(SocketAddress):

                    // flush any packets already received.
                    boolean blocking = isBlocking();
                    if (blocking) {
                        IOUtil.configureBlocking(fd, false);
                    }
                    try {
                        ByteBuffer buf = ByteBuffer.allocate(100);
                        while (receive(fd, buf, false) > 0) {
                            buf.clear();
                        }
                    } finally {
                        if (blocking) {
                            IOUtil.configureBlocking(fd, true);
                        }
                    }

So any pending datagrams are cleared in userspace, and the race is
benign and not observable by applications.  (I forgot that this was
added in 2014, sorry.)

Thanks,
Florian