netdev - Re: [PATCH net-next v2] tcp: Dump bound-only sockets in inet

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZWZnQL1tnjJ9R8Er@debian>
Date: Tue, 28 Nov 2023 23:18:40 +0100
From: Guillaume Nault <gnault@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
	David Ahern <dsahern@...nel.org>,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Michal Kubecek <mkubecek@...e.cz>
Subject: Re: [PATCH net-next v2] tcp: Dump bound-only sockets in inet_diag.

On Tue, Nov 28, 2023 at 11:14:28AM +0100, Eric Dumazet wrote:
> On Fri, Nov 24, 2023 at 12:11 AM Guillaume Nault <gnault@...hat.com> wrote:
> >
> > Walk the hashinfo->bhash2 table so that inet_diag can dump TCP sockets
> > that are bound but haven't yet called connect() or listen().
> >
> > This allows ss to dump bound-only TCP sockets, together with listening
> > sockets (as there's no specific state for bound-only sockets). This is
> > similar to the UDP behaviour for which bound-only sockets are already
> > dumped by ss -lu.
> >
> > The code is inspired by the ->lhash2 loop. However there's no manual
> > test of the source port, since this kind of filtering is already
> > handled by inet_diag_bc_sk(). Also, a maximum of 16 sockets are dumped
> > at a time, to avoid running with bh disabled for too long.
> >
> > No change is needed for ss. With an IPv4, an IPv6 and an IPv6-only
> > socket, bound respectively to 40000, 64000, 60000, the result is:
> >
> >   $ ss -lt
> >   State  Recv-Q Send-Q Local Address:Port  Peer Address:PortProcess
> >   UNCONN 0      0            0.0.0.0:40000      0.0.0.0:*
> >   UNCONN 0      0               [::]:60000         [::]:*
> >   UNCONN 0      0                  *:64000            *:*
> 
> 
> Hmm...   "ss -l" is supposed to only list listening sockets.
> 
> So this change might confuse some users ?
> 

On the other hand I can't find a more sensible solution. The problem is
that "ss -l" sets both the TCPF_LISTEN and the TCPF_CLOSE flags. And
since we don't have a way to express "bound but not yet listening"
sockets, these sockets fall into the CLOSE category. So we're really
just returning what ss asked for.

If we can't rely on TCPF_CLOSE, then I don't see what kind of filter we
could use to request a dump of these TCP sockets. Using "-a" doesn't
help as it just sets all the TCPF_* flags (appart from
TCPF_NEW_SYN_RECV). Adding a new option wouldn't help either as we
couldn't map it to any of the TCPF_* flags. In any case, we still need
to rely on TCPF_CLOSE.

So maybe we can just improve the ss man page for "-l" and explain that
it also lists closed sockets, which includes the bound-only ones
(this is already true for non-TCP sockets anyway). We could also tell
the user to run "ss state listening" for getting listening sockets
exclusively (or we could implement a new option, like "-L", to make
that shorter if necessary).

What do you think?