From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Paasch Subject: Re: [PATCH] net: do not allow changing SO_REUSEADDR/SO_REUSEPORT on bound sockets Date: Sun, 3 Jun 2018 12:54:40 -0700 Message-ID: References: <20180603174705.51802-1-zenczykowski@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: =?UTF-8?Q?Maciej_=C5=BBenczykowski?= , "David S . Miller" , Eric Dumazet , netdev To: =?UTF-8?Q?Maciej_=C5=BBenczykowski?= Return-path: Received: from mail-pl0-f67.google.com ([209.85.160.67]:42016 "EHLO mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750952AbeFCTyl (ORCPT ); Sun, 3 Jun 2018 15:54:41 -0400 Received: by mail-pl0-f67.google.com with SMTP id w17-v6so2463876pll.9 for ; Sun, 03 Jun 2018 12:54:41 -0700 (PDT) In-Reply-To: <20180603174705.51802-1-zenczykowski@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: Hello, On Sun, Jun 3, 2018 at 10:47 AM, Maciej =C5=BBenczykowski wrote: > From: Maciej =C5=BBenczykowski > > It is not safe to do so because such sockets are already in the > hash tables and changing these options can result in invalidating > the tb->fastreuse(port) caching. > > This can have later far reaching consequences wrt. bind conflict checks > which rely on these caches (for optimization purposes). > > Not to mention that you can currently end up with two identical > non-reuseport listening sockets bound to the same local ip:port > by clearing reuseport on them after they've already both been bound. as a side-note: Some time back I realized that one can also - on the active opener side - create two TCP connections with the same 5-tuple going out over the same interface. One simply needs to first create a connection with a socket that has SO_BINDTODEV set that specifies the same interface as the default route. The second socket (which doesn't uses SO_BINDTODEV) then can end up using the same source-port, if the range of available ports has been exhausted. This makes for some interesting packet-traces! :) This is because INET_MATCH in __inet_check_established only checks for !(sk->sk_bound_dev_if). inet_hash_connect() probably would need info of the route's outgoing interface (of the new socket) to decide whether or not there is a match. But even that wouldn't be failsafe as the routing could change later on... So, I dropped the ball on that. Not sure if it's a big deal or not... Cheers, Christoph > > There is unfortunately no EISBOUND error or anything similar, > and EISCONN seems to be misleading for a bound-but-not-connected > socket, so use EUCLEAN 'Structure needs cleaning' which AFAICT > is the closest you can get to meaning 'socket in bad state'. > (although perhaps EINVAL wouldn't be a bad choice either?) > > This does unfortunately run the risk of breaking buggy > userspace programs... > > Signed-off-by: Maciej =C5=BBenczykowski > Cc: Eric Dumazet > > Change-Id: I77c2b3429b2fdf42671eee0fa7a8ba721c94963b > --- > net/core/sock.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/net/core/sock.c b/net/core/sock.c > index 435a0ba85e52..feca4c98f8a0 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -728,9 +728,22 @@ int sock_setsockopt(struct socket *sock, int level, = int optname, > sock_valbool_flag(sk, SOCK_DBG, valbool); > break; > case SO_REUSEADDR: > - sk->sk_reuse =3D (valbool ? SK_CAN_REUSE : SK_NO_REUSE); > + val =3D (valbool ? SK_CAN_REUSE : SK_NO_REUSE); > + if ((sk->sk_family =3D=3D PF_INET || sk->sk_family =3D=3D= PF_INET6) && > + inet_sk(sk)->inet_num && > + (sk->sk_reuse !=3D val)) { > + ret =3D (sk->sk_state =3D=3D TCP_ESTABLISHED) ? -= EISCONN : -EUCLEAN; > + break; > + } > + sk->sk_reuse =3D val; > break; > case SO_REUSEPORT: > + if ((sk->sk_family =3D=3D PF_INET || sk->sk_family =3D=3D= PF_INET6) && > + inet_sk(sk)->inet_num && > + (sk->sk_reuseport !=3D valbool)) { > + ret =3D (sk->sk_state =3D=3D TCP_ESTABLISHED) ? -= EISCONN : -EUCLEAN; > + break; > + } > sk->sk_reuseport =3D valbool; > break; > case SO_TYPE: > -- > 2.17.1.1185.g55be947832-goog >