netdev - Re: [PATCH] net: do not allow changing SO_REUSEADDR/SO

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 11 Jun 2018 20:09:19 -0300
From:   Marc Dionne <marc.c.dionne@...il.com>
To:     Maciej Żenczykowski <zenczykowski@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        Jeffrey Altman <jaltman@...istor.com>,
        Marc Dionne <marc@...istor.com>
Subject: Re: [PATCH] net: do not allow changing SO_REUSEADDR/SO_REUSEPORT on
 bound sockets

On Mon, Jun 11, 2018 at 7:29 PM, Maciej Żenczykowski
<zenczykowski@...il.com> wrote:
>> This change is a potential performance regression for us.  Our
>> networking code sets up multiple sockets used by multiple threads to
>> listen on the same udp port.  For the first socket, the bind is done
>> before setting SO_REUSEPORT so that we can get an error and detect
>> that the port is currently in use by a different process, possibly a
>> previous incarnation of the same application.
>>
>> With this change, the servers end up with a single listener socket and
>> thread, which can have a large impact on performance for a busy
>> server.
>>
>> While we can adjust for the new behaviour going forward, this could be
>> an unpleasant surprise for our customers who get this update when
>> moving to a new kernel version or through a backport to stable
>> kernels, if this is targeted for stable.
>
> Probably you can:
> fd1=socket()
> fd2=socket()
> bind(fd1,port=0)
> setsockopt(fd2,REUSEPORT,1)
> port = getsockname(fd1)
> close(fd1)
> bind(fd2,port)
>
> Although yeah there's a slight chance of a race (ie. 2nd bind failing,
> in which case close() and retry).

This would be a bit different since we want a specific port rather
than a wildcard, but yes, a temporary socket could be bound just to
check if the port is in use and closed afterwards.  The problem is
that since fd2 has REUSEPORT set, that second bind might well succeed
if there was a race and the other user of the port also has REUSEPORT
set, as could be the case if it's another instance of the same
application.

Marc