Page MenuHomeFreeBSD

tcp: Disallow re-connection of a connected socket
ClosedPublic

Authored by markj on Feb 11 2023, 4:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 14, 5:00 PM
Unknown Object (File)
Sun, Oct 27, 2:16 AM
Unknown Object (File)
Oct 4 2024, 11:48 PM
Unknown Object (File)
Oct 2 2024, 6:04 PM
Unknown Object (File)
Sep 28 2024, 11:59 PM
Unknown Object (File)
Sep 28 2024, 11:59 PM
Unknown Object (File)
Sep 26 2024, 10:06 PM
Unknown Object (File)
Sep 24 2024, 9:30 AM
Subscribers

Details

Summary

soconnectat() tries to ensure that one cannot connect a connected
socket. However, the check is racy and does not really prevent two
threads from attempting to connect the same TCP socket.

Modify tcp_connect() and tcp6_connect() to perform the check again, this
time synchronized by the inpcb lock, under which we call
soisconnecting().

Reported by: syzkaller

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 49688
Build 46578: arc lint + arc unit

Event Timeline

markj requested review of this revision.Feb 11 2023, 4:28 PM
  • Undo the modification to tcp6_usr_connect().

May I ask two additions to the existing patch?

  1. Your discovery of the racyness of soconnectat() is very important and it deserves a comment in the function code with reference to the fact that TCP solves the problem for itself.
  2. I'd suggest to brace the new check info __predict_false().
  • Add a comment to soconnectat().
  • Add __predict_false annotations.

May I ask two additions to the existing patch?

  1. Your discovery of the racyness of soconnectat() is very important and it deserves a comment in the function code with reference to the fact that TCP solves the problem for itself.
  2. I'd suggest to brace the new check info __predict_false().

BTW, this is already checked implicitly by UDP when it checks inp->inp_faddr.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 14 2023, 3:14 PM
This revision was automatically updated to reflect the committed changes.