Commit Graph

343 Commits

Author SHA1 Message Date
Gleb Smirnoff
afad340a14 inpcb: garbage collect INP_LOCK_INIT(), used only once in sctp
Reviewed by:		tuexen
Differential revision:	https://reviews.freebsd.org/D33543
2022-01-03 10:20:30 -08:00
Michael Tuexen
2de2ae331b sctp: improve sctp_pathmtu_adjustment()
Allow the resending of DATA chunks to be controlled by the caller,
which allows retiring sctp_mtu_size_reset() in a separate commit.
Also improve the computaion of the overhead and use 32-bit integers
consistently.
Thanks to Timo Voelker for pointing me to the code.

MFC after:	3 days
2021-12-30 15:16:05 +01:00
Michael Tuexen
989453da05 sctp: cleanup the SCTP_MAXSEG socket option.
This patch makes the handling of the SCTP_MAXSEG socket option
compliant with RFC 6458 (SCTP socket API) and fixes an issue
found by syzkaller.

Reported by:	syzbot+a2791b89ab99121e3333@syzkaller.appspotmail.com
MFC after:	3 days
2021-12-27 23:40:31 +01:00
Michael Tuexen
54912d47b6 sctp: unbreak NOINET6 builds.
PR:		260119
Reported by:	kostikbel
MFC after:	1 week
2021-12-04 19:16:18 +01:00
Michael Tuexen
d79676fb13 sctp: inherit IP level socket options from listening socket
Ensure that TTL and TOS values set on a listener get inheritet
to the accepted sockets.

PR:		260119
MFC after:	1 week
2021-12-03 22:44:01 +01:00
Michael Tuexen
3c1ba6f394 sctp: improve consistency, no functional change intended 2021-11-26 12:53:43 +01:00
Michael Tuexen
762ae0ec8d sctp: Simplify stream scheduler usage
Callers are getting the stcb send lock, so just KASSERT that.
No need to signal this when calling stream scheduler functions.
No functional change intended.

MFC after:	1 week
2021-09-21 17:13:57 +02:00
Michael Tuexen
4181fa2a20 sctp: minor cleanup, no functional change
MFC after:	1 week
2021-09-12 19:21:15 +02:00
Mark Johnston
2d5c48eccd sctp: Tighten up locking around sctp_aloc_assoc()
All callers of sctp_aloc_assoc() mark the PCB as connected after a
successful call (for one-to-one-style sockets).  In all cases this is
done without the PCB lock, so the PCB's flags can be corrupted.  We also
do not atomically check whether a one-to-one-style socket is a listening
socket, which violates various assumptions in solisten_proto().

We need to hold the PCB lock across all of sctp_aloc_assoc() to fix
this.  In order to do that without introducing lock order reversals, we
have to hold the global info lock as well.

So:
- Convert sctp_aloc_assoc() so that the inp and info locks are
  consistently held.  It returns with the association lock held, as
  before.
- Fix an apparent bug where we failed to remove an association from a
  global hash if sctp_add_remote_addr() fails.
- sctp_select_a_tag() is called when initializing an association, and it
  acquires the global info lock.  To avoid lock recursion, push locking
  into its callers.
- Introduce sctp_aloc_assoc_connected(), which atomically checks for a
  listening socket and sets SCTP_PCB_FLAGS_CONNECTED.

There is still one edge case in sctp_process_cookie_new() where we do
not update PCB/socket state correctly.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31908
2021-09-11 10:15:21 -04:00
Michael Tuexen
3ea2cdd45e sctp: add explicit cast, no functional change intended
MFC after:	3 days
2021-09-09 19:13:47 +02:00
Mark Johnston
4250aa1188 sctp: Clear assoc socket references when freeing a PCB
This restores behaviour present in the first import of SCTP.  Commit
ceaad40ae7 commented this out and commit
62fb761ff2 removed it.  However, once
sctp_inpcb_free() returns, the socket reference is gone no matter what,
so we need to clear it.

Reported by:	syzbot+30dd69297fcbc5f0e10a@syzkaller.appspotmail.com
Reported by:	syzbot+7b2f9d4bcac1c9569291@syzkaller.appspotmail.com
Reported by:	syzbot+ed3e651f7d040af480a6@syzkaller.appspotmail.com
Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31886
2021-09-09 08:33:26 -04:00
Michael Tuexen
58a7bf124c sctp: cleanup timewait handling for vtags
MFC after:	1 week
2021-09-09 01:18:58 +02:00
Mark Johnston
ee4731179c sctp: Fix a lock order reversal in sctp_swap_inpcb_for_listen()
When port reuse is enabled in a one-to-one-style socket, sctp_listen()
may call sctp_swap_inpcb_for_listen() to move the PCB out of the "TCP
pool".  In so doing it will drop the PCB lock, yielding an LOR since we
now hold several socket locks.  Reorder sctp_listen() so that it
performs this operation before beginning the conversion to a listening
socket.  Also modify sctp_swap_inpcb_for_listen() to return with PCB
write-locked, since that's what sctp_listen() expects now.

Reviewed by:	tuexen
Fixes:	bd4a39cc93 ("socket: Properly interlock when transitioning to a listening socket")
MFC after:	1 month
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31879
2021-09-08 11:41:19 -04:00
Mark Johnston
6e3af6321b sctp: Fix lock recursion in sctp_swap_inpcb_for_listen()
After commit bd4a39cc93 we now hold the global inp info lock across
the call to sctp_swap_inpcb_for_listen(), which attempts to acquire it
again.  Since sctp_swap_inpcb_for_listen()'s sole caller is
sctp_listen(), we can simply change it to not try to acquire the lock.

Reported by:	syzbot+a76b19ea2f8e1190c451@syzkaller.appspotmail.com
Reported by:	syzbot+a1b6cef257ad145b7187@syzkaller.appspotmail.com
Reviewed by:	tuexen
Fixes:	bd4a39cc93 ("socket: Properly interlock when transitioning to a listening socket")
MFC after:	1 month
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31878
2021-09-08 11:41:18 -04:00
Michael Tuexen
aab1d593b2 sctp: minor cleanups, no functional change intended 2021-09-08 15:13:49 +02:00
Mark Johnston
c17b531bed sctp: Fix races around sctp_inpcb_free()
sctp_close() and sctp_abort() disassociate the PCB from its socket.
As a part of this, they attempt to free the PCB, which may end up
lingering.  Fix some bugs in this area:

- For some reason, sctp_close() and sctp_abort() set
  SCTP_PCB_FLAGS_SOCKET_GONE using an atomic compare-and-set without the
  PCB lock held.  This is racy since sctp_flags is normally updated
  without atomics, using the PCB lock to synchronize.  So, the update
  can be lost, which can cause all sort of races with other SCTP
  components which look for the _GONE flag.  Fix the problem simply by
  acquiring the PCB lock in order to set the flag.  Note that we have to
  drop and re-acquire the lock again in sctp_inpcb_free(), but I don't
  see a good way around that for now.  If it's a real problem, the _GONE
  flag could be split out of sctp_flags and into a dedicated sctp_inpcb
  field.
- In sctp_inpcb_free(), load sctp_socket after acquiring the PCB lock,
  to avoid possible races with parallel sctp_inpcb_free() calls.
- Add an assertion sctp_inpcb_free() to verify that _ALLGONE is not set.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31811
2021-09-07 11:19:29 -04:00
Mark Johnston
d35be50f57 sctp: Hold association locks across socket wakeups when freeing
At this point we do not hold the inpcb lock, so the only thing holding
the socket reference live is the TCB lock, which needs to be acquired by
sctp_inpcb_free() in order to destroy associations.  Defer the unlock to
until after we dereference the socket reference.

Reported by:	syzbot+1d0f2c4675de76a4cf1e@syzkaller.appspotmail.com
Reported by:	syzbot+fabee77954fe69d3a5ad@syzkaller.appspotmail.com
Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31754
2021-09-01 10:27:51 -04:00
Mark Johnston
65f30a39e1 sctp: Release the socket reference when detaching an association
Later in sctp_free_assoc(), when we clean up chunk lists,
sctp_free_spbufspace() is used to reset the byte count in the socket
send buffer.  However, if the PCB is going away, the socket may already
have been detached from the PCB, in which case this becomes a use-after
free.  Clear the socket reference from the association before detaching
it from the PCB, if the PCB has already lost its socket reference.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31753
2021-09-01 10:27:31 -04:00
Mark Johnston
457abbb857 sctp: Implement sctp_inpcb_bind_locked()
This will be used by sctp_listen() to avoid dropping locks when
performing an implicit bind.  No functional change intended.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31757
2021-09-01 10:06:18 -04:00
Mark Johnston
4a36122b1d sctp: Fix racy UNBOUND flag check in sctp_inpcb_bind()
SCTP needs to avoid binding a given socket twice.  The check used to
avoid this is racy since neither the inpcb lock nor the global info lock
is held.  Fix it by synchronizing using the global info lock.  In
particular, sctp_inpcb_bind() may drop the inpcb lock in some cases, but
the info lock is sufficient to prevent double insertion into PCB hash
tables.

Reported by:	syzbot+548a8560d959669d0e12@syzkaller.appspotmail.com
Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31734
2021-08-31 07:44:42 -04:00
Mark Johnston
2496d812a9 sctp: Simplify the free port search in sctp_inpcb_bind()
Eliminate a flag variable and reduce indentation.  No functional change
intended.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31733
2021-08-31 07:43:39 -04:00
Mark Johnston
93908fce72 sctp: Avoid unnecessary refcount bumps in sctp_inpcb_bind()
We only drop the inp lock when binding to a specific port.  So, only
acquire an extra reference when required.  This simplifies error
handling a bit.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31732
2021-08-31 07:43:27 -04:00
Mark Johnston
0d29e4bc01 sctp: Remove always-false checks in sctp_inpcb_bind()
No functional change intended.

Reviewed by:	tuexen
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D31731
2021-08-31 07:43:13 -04:00
Michael Tuexen
105b68b42d sctp: Fix errno in case of association setup failures
Do not report always ETIMEDOUT, but only when appropriate. In
other cases report ECONNABORTED.

MFC after:	3 days
2021-07-09 23:19:25 +02:00
Michael Tuexen
c7f048ab35 sctp: initialize sequence numbers for ECN correctly
MFC after:	3 days
Reported by:	Junseok Yang (for the userland stack)
2021-06-27 20:14:48 +02:00
Michael Tuexen
8b3d0f6439 sctp: improve address list scanning
If the alternate address has to be removed, force the stack to
find a new one, if it is still needed.

MFC after:	3 days
2021-05-03 02:50:05 +02:00
John Baldwin
5a50eb6585 Don't pass RFPROC to kproc_create(), it is redundant.
Reviewed by:	tuexen, kib
MFC after:	1 week
Sponsored by:	Netflix
Differential Revision:	https://reviews.freebsd.org/D29206
2021-03-12 09:48:10 -08:00
Michael Tuexen
5ac839029d sctp: clear a pointer to a net which will be removed
MFC after:	3 days
2021-02-21 13:06:05 +01:00
Michael Tuexen
b954d81662 Ensure variables are initialized before used.
MFC after:		3 days
2020-10-06 11:29:08 +00:00
Michael Tuexen
6176f9d6df Remove dead stores reported by clang static code analysis
MFC after:		3 days
2020-10-06 11:08:52 +00:00
Michael Tuexen
b15f541113 Improve the input validation and processing of cookies.
This avoids setting the association in an inconsistent
state, which could result in a use-after-free situation.
This can be triggered by a malicious peer, if the peer
can modify the cookie without the local endpoint recognizing
it.
Thanks to Ned Williamson for reporting the issue.

MFC after:		3 days
2020-09-29 09:36:06 +00:00
Michael Tuexen
fbc6840bae Minor cleanup.
MFC after:		3 days
2020-09-28 14:11:53 +00:00
Michael Tuexen
b6db274d1e Whitespace changes.
MFC after:		3 days
2020-09-24 12:26:06 +00:00
Mateusz Guzik
662c13053f net: clean up empty lines in .c and .h files 2020-09-01 21:19:14 +00:00
Michael Tuexen
f5d30f7f76 Improve the handling of concurrent send() calls for SCTP sockets,
especially when having the explicit EOR mode enabled.

Reported by:		Megan2013678@protonmail.com
Reported by:		syzbot+bc02585076c3cc977f9b@syzkaller.appspotmail.com
MFC after:		3 days
2020-08-16 11:50:37 +00:00
Michael Tuexen
205f3e1597 Clear the pointer to the socket when closing it also in case of
an ungraceful operation.
This fixes a use-after-free bug found and reported by Taylor
Brandstetter of Google by testing the userland stack.

MFC after:		1 week
2020-07-23 19:43:49 +00:00
Michael Tuexen
8745f898c4 Add reference counts for inp/stcb/net when timers are running.
This avoids a use-after-free reported for the userland stack.
Thanks to Taylor Brandstetter for suggesting a patch for
the userland stack.

MFC after:		1 week
2020-07-19 12:34:19 +00:00
Michael Tuexen
05bceec68e Remove code which is not needed.
MFC after:		1 week
2020-07-18 13:10:02 +00:00
Michael Tuexen
7f0ad2274b Improve the locking of address lists by adding some asserts and
rearranging the addition of address such that the lock is not
given up during checking and adding.

MFC after:		1 week
2020-07-17 15:09:49 +00:00
Mark Johnston
e6db509d10 Move the definition of SCTP's system_base_info into sctp_crc32.c.
This file is the only SCTP source file compiled into the kernel when
SCTP_SUPPORT is configured.  sctp_delayed_checksum() references a couple
of counters defined in system_base_info, so the change allows these
counters to be referenced in a kernel compiled without "options SCTP".

Submitted by:	tuexen
MFC with:	r362338
2020-06-22 14:01:31 +00:00
Michael Tuexen
d60bdf8569 Remove usage of empty macro.
MFC after:		1 week
2020-06-13 21:23:26 +00:00
Michael Tuexen
2f9e6db0be More cleanups due to ifdef cleanup done upstream
MFC after:		1 week
2020-06-12 16:31:13 +00:00
Michael Tuexen
28397ac1ed Non-functional changes due to upstream cleanup.
MFC after:		1 week
2020-06-11 13:34:09 +00:00
Michael Tuexen
5fb132abbb Whitespace cleanups and removal of a stale comment.
MFC after:		1 week
2020-06-08 20:23:20 +00:00
Michael Tuexen
3f53d62236 Fix typo in comment.
Submitted by Orgad Shaneh for the userland stack.
MFC after:		1 week
2020-06-06 21:26:34 +00:00
Michael Tuexen
2cf3347109 Non-functional changes due to cleanup (upstream removing of Panda support)
of the code

MFC after:		1 week
2020-06-06 18:20:09 +00:00
Michael Tuexen
999f86d67d Replace snprintf() by SCTP_SNPRINTF() and let SCTP_SNPRINTF() map
to snprintf() on FreeBSD. This allows to check for failures of snprintf()
on platforms other than FreeBSD kernel.
2020-05-19 07:23:35 +00:00
Michael Tuexen
821bae7cf3 Revert r361209:
cem noted that on FreeBSD snprintf() can not fail and code should not
check for that.

A followup commit will replace the usage of snprintf() in the SCTP
sources with a variadic macro SCTP_SNPRINTF, which will simply map to
snprintf() on FreeBSD and do a checking similar to r361209 on
other platforms.
2020-05-19 07:21:11 +00:00
Michael Tuexen
bca1802890 Cleanup, no functional change intended.
MFC after:		3 days
2020-05-18 18:42:43 +00:00
Michael Tuexen
e708e2a4f4 Handle failures of snprintf().
MFC after:		3 days
2020-05-18 10:07:01 +00:00