pf: add INVARIANTS visibility and use it to sidestep a panic
If this pans out we can still release the lock that we are supposed
to unlock here. We do not know the reason why this happens yet,
but we avoid the risk of chasing a pointer which is no longer dependable.
Also add mutex owned and recursed state checks in the output and
avoid unlocking if we no longer own the lock.
pf: add INVARIANTS visibility and use it to sidestep a panic
If this pans out wer can still release the lock that we're supposed
to unlock here. We do not know the reason and if the state is actually
still locked, but we avoid the risk of chasing a pointer which is no
longer dependable.
Also add mutex owned and recursed state checks in the output and
avoid unlocking if we no longer own the lock.
pf: add INVARIANTS visibility and use it to sidestep a panic
If this pans out wer can still release the lock that we're supposed
to unlock here. We do not know the reason and if the state is actually
still locked, but we avoid the risk of chasing a pointer which is no
longer dependable.
pf: do a lock dance in pf_unlink_state()
Both pf_test() and pf_test6() can end up in a panic while
executing PF_UNLOCK_STATE which points to the state being
removed while it is in use.
The PF_LOCK_STATE in the removal subroutine makes sure
that pf_test/pf_test6 are no longer holding the state
and we can safely test and set PFTM_UNLINK.
The other bits of the OpenBSD commit probably apply as well
but for now make sure that this particular panic comes to
and end.
Based on: https://github.com/openbsd/src/commit/9d9f4dc6c83
inpcb: Move the definition of struct inpcblbgroup to in_pcb_var.h
It's only needed for in_pcb.c and in6_pcb.c, so can go to the private
header.
No functional change intended.
Reported by: glebius
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
(cherry picked from commit ca94f92c23fd09b28ac3398657ae2ae9367bcdf5)
inpcb: Add FIB-aware inpcb lookup
Allow protocol layers to look up an inpcb belonging to a particular FIB.
This is indicated by setting INPLOOKUP_FIB; if it is set, the FIB to be
used is obtained from the specificed mbuf or ifnet.
No functional change intended.
Reviewed by: glebius, melifaro
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D48662
(cherry picked from commit da806e8db685eead02bc67888b16ebac6badb6b6)
inpcb: Add a flags parameter to in_pcbbind()
Add a flag, INPBIND_FIB, which means that the inpcb is local to its FIB
number. When this flag is specified, duplicate bindings are permitted,
so long as each FIB contains at most one inpcb bound to the same
address/port. If an inpcb is bound with this flag, it'll have the
INP_BOUNDFIB flag set.
No functional change intended.
Reviewed by: glebius
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D48661
(cherry picked from commit bbd0084baf7539c7042ce94f8c6770210f83f765)
inpcb: Imbue in(6)_pcblookup_local() with a FIB parameter
This is to enable a mode where duplicate inpcb bindings are permitted,
and we want to look up an inpcb with a particular FIB. Thus, add a
"fib" parameter to in_pcblookup() and related functions, and plumb it
through.
A fib value of RT_ALL_FIBS indicates that the lookup should ignore FIB
numbers when searching. Otherwise, it should refer to a valid FIB
number, and the returned inpcb should belong to the specific FIB. For
now, just add the fib parameter where needed, as there are several
layers to plumb through.
No functional change intended.
Reviewed by: glebius
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
[3 lines not shown]
inpcb: Further restrict binding to a port owned by a different UID
See commit 4f02a7d739b3 for more background.
I cannot see a good reason to continue ignoring mismatching UIDs when
binding to INADDR_ANY. Looking at the sdr.V2.4a7n sources (mentioned in
bugzilla PR 7713), there is a CANT_MCAST_BIND hack wherein the
application binds to INADDR_ANY instead of a multicast address, but
CANT_MCAST_BIND isn't defined for FreeBSD builds.
It seems unlikely that we still have a use-case for allowing sockets
from different UIDs to bind to the same port when binding to the
unspecified address. And, as noted in D47832, applications like sdr
would have been broken by the inverted SO_REUSEPORT check removed in
that revision, apparently without any bug reports. Let's break
compatibility and simply disallow this case outright.
Also, add some comments, remove a hack in a regression test which tests
this funtionality, and add a new regression test to exercise the
[8 lines not shown]
inpcb: Close some SO_REUSEPORT_LB races
For a long time, the inpcb lookup path has been lockless in the common
case: we use net_epoch to synchronize lookups. However, the routines
which update lbgroups were not careful to synchronize with unlocked
lookups. I believe that in the worst case this can result in spurious
connection aborts (I have a regression test case to exercise this), but
it's hard to be certain.
Modify in_pcblbgroup* routines to synchronize with unlocked lookup:
- When removing inpcbs from an lbgroup, do not shrink the array.
The maximum number of lbgroup entries is INPCBLBGROUP_SIZMAX (256),
and it doesn't seem worth the complexity to shrink the array when a
socket is removed.
- When resizing an lbgroup, do not insert it into the hash table until
it is fully initialized; otherwise lookups may observe a partially
constructed lbgroup.
- When adding an inpcb to the group, increment the counter after adding
the array entry, using a release store. Otherwise it's possible for
[10 lines not shown]
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.
[27 lines not shown]
inpcb: Factor out parts of in6_pcbbind() and in_pcbbind_setup()
A large portion of these functions just determines whether the inpcb can
bind to the address/port. This portion has no side effects, so is a
good candidate to move into its own helper function. This patch does
so, making the callers less complicated and reducing indentation.
While moving this code, also make some changes:
- Load socket options (SO_REUSEADDR etc.) only once. There is nothing
preventing another thread from toggling the socket options, so make
this function easier to reason about by avoiding races.
- When checking whether the bind address is an interface address, make a
separate sockaddr rather than temporarily modifying the one passed to
in_pcbbind().
Reviewed by: ae, glebius
MFC after: 1 month
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
[3 lines not shown]
inpcb: Make some cosmetic improvements to in_pcbbind()
- Use the local var "laddr" instead of sin->sin_addr in one block.
- Use in_nullhost() instead of explicit comparisons with INADDR_ANY.
- Combine multiple socket options checks into one.
- Fix indentation.
- Remove some unhelpful comments.
This is in preparation for some simplification and bug-fixing.
No functional change intended.
Reviewed by: glebius
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D47451
(cherry picked from commit 45a77bf23fa2f36bf2169f7ba2a33b31f4c35adb)
inpcb: Remove some unused parameters in internal hash lookup functions
in_pcblookup_hash_wild_* looks up unconnected inpcbs, so there is no
point in passing the foreign address and port, and indeed those
parameters are not used. So, remove them.
No functional change intended.
MFC after: 1 week
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D47385
(cherry picked from commit 21d7ac8c79a34cf3b7205d0c32014ee39f1f28ab)
netinet: Update a comment for in_localip()
The function in_localip() was changed to return bool but the comment was
left unchanged.
Fixes: c8ee75f2315e Use network epoch to protect local IPv4 addresses hash
MFC after: 3 days
(cherry picked from commit a5e380e51cdba64a392846a4eeda000f948f42ce)
iwlwifi: adjust a debug comment referring to a PR
A FreeBSD specific comment asked people to report to a PR if they see
this. By now we got enough feedback and also left this in a release.
Simply point to the PR so people can check the status but not longer
ask to submit a report to the PR.
Sponsored by: The FreeBSD Foundation
PR: 274382
(cherry picked from commit 4a4eee553307a2e02c6ed4796d575bfce2857049)
ifconfig: fix reporting optics on most 100g interfaces
This fixes a bug where optics on 100G and faster NICs
were not properly reported.
(cherry picked from commit 709348c21351a783ff0025519d1f7cf884771077)
