HomeFreeBSD

inpcb: Remove bogus SO_REUSEPORT(_LB) checks in in_pcbbind()

Description

inpcb: Remove bogus SO_REUSEPORT(_LB) checks in in_pcbbind()

This check for SO_REUSEPORT was added way back in commit 52b65dbe85faf.
Per the commit log, this commit restricted this port-stealing check to
unicast addresses, and then only if the existing socket does not have
SO_REUSEPORT set. In other words, if there exists a socket bound to
INADDR_ANY, and we bind a socket to INADDR_ANY with the same port, then
the two sockets need not be owned by the same user if the existing
socket has SO_REUSEPORT set.

This is a surprising semantic; bugzilla PR 7713 gives some additional
context. That PR makes a case for the behaviour described above when
binding to a multicast address. But, the SO_REUSEPORT check is only
applied when binding to a non-multicast address, so it doesn't really
make sense. In the PR the committer notes that "unicast applications
don't set SO_REUSEPORT", which makes some sense, but also refers to
"multicast applications that bind to INADDR_ANY", which sounds a bit
suspicious.

OpenBSD performs the multicast check, but not the SO_REUSEPORT check.
DragonflyBSD removed the SO_REUSEPORT (and INADDR_ANY) checks back in
2014 (commit 0323d5fde12a4). NetBSD explicitly copied our logic and
still has it.

The plot thickens: 20 years later, SO_REUSEPORT_LB was ported from
DragonflyBSD: this option provides similar semantics to SO_REUSEPORT,
but for unicast addresses it causes incoming connections/datagrams to be
distributed among all sockets in the group. This commit (1a43cff92a20d)
inverted the check for SO_REUSEPORT while adding one for
SO_REUSEPORT_LB; this appears to have been inadvertent. However:

  • apparently no one has noticed that the semantics were changed;
  • sockets belonging to different users can now be bound to the same port so long as they belong to a single lbgroup bound to INADDR_ANY, which is not correct.

Simply remove the SO_REUSEPORT(_LB) checks, as their original
justification was dubious and their current implementation is wrong; add
some tests.

Reviewed by: glebius
MFC after: 1 month
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D47832

Details

Provenance
markjAuthored on Dec 12 2024, 2:06 PM
Reviewer
glebius
Differential Revision
D47832: inpcb: Remove bogus SO_REUSEPORT(_LB) checks in in_pcbbind()
Parents
rGa600aabe9b04: inpcb: Close some SO_REUSEPORT_LB races
Branches
Unknown
Tags
Unknown