tcp: Disallow re-connection of a connected socket
ClosedPublic
Actions

Authored by markj on Feb 11 2023, 4:28 PM.

Details

Reviewers

glebius

Group Reviewers

transport

Commits

rG636b19ead43a: tcp: Disallow re-connection of a connected socket

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 49689
Build 46579: arc lint + arc unit

Event Timeline

markj created this revision.Feb 11 2023, 4:28 PM

Herald added 1 blocking reviewer(s): transport. · View Herald TranscriptFeb 11 2023, 4:28 PM

Herald added subscribers: melifaro, imp. · View Herald Transcript

markj requested review of this revision.Feb 11 2023, 4:28 PM

Harbormaster completed remote builds in B49688: Diff 116986.Feb 11 2023, 4:28 PM

Undo the modification to tcp6_usr_connect().

Harbormaster completed remote builds in B49689: Diff 116987.Feb 11 2023, 4:29 PM

May I ask two additions to the existing patch?

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.
I'd suggest to brace the new check info __predict_false().

Add a comment to soconnectat().
Add __predict_false annotations.

Harbormaster completed remote builds in B49749: Diff 117142.Feb 13 2023, 9:43 PM

In D38507#877427, @glebius wrote:

May I ask two additions to the existing patch?

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.

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.

glebius accepted this revision.Feb 13 2023, 10:26 PM

This revision was not accepted when it landed; it landed in state Needs Review.Feb 14 2023, 3:14 PM

Closed by commit rG636b19ead43a: tcp: Disallow re-connection of a connected socket (authored by markj). · Explain Why

This revision was automatically updated to reflect the committed changes.

markj added a commit: rG636b19ead43a: tcp: Disallow re-connection of a connected socket.