Restrict enabling TCP-FASTOPEN to end-points in CLOSED or LISTEN state

[tuexen]

Summary

Enabling TCP-FASTOPEN on an end-point which is in a state later than CLOSED or LISTEN, is a bug in the application. So it should not work. Also the TCP code does not (and needs not to) handle this.

This addresses also a bug found by syzkaller, which can also be reproduced by the following packetdrill script:

// http://syzkaller.nplab.de:10000/file?name=crashes%2f808457b108a581dd99facfcb23418f4a0136025f%2flog18
//01:02:45 executing program 0:
//r0 = socket$inet_tcp(0x2, 0x1, 0x0)
//setsockopt$inet_tcp_TCP_FUNCTION_BLK(r0, 0x6, 0x2000, &(0x7f00000001c0)={'rack\x00'}, 0x24)
//setsockopt$sock_int(r0, 0xffff, 0x10000, &(0x7f0000000040)=0x1, 0x4)
//bind(r0, &(0x7f00000002c0)=@in={0x10, 0x2, 0x2}, 0x10)
//setsockopt$inet_tcp_int(r0, 0x6, 0x22, &(0x7f0000000000)=0x1, 0x4)
//sendto(r0, 0x0, 0x0, 0x0, &(0x7f0000000140)=@in={0x10, 0x2, 0x0, @broadcast}, 0x10)
//syz_emit_ethernet(0x3a, &(0x7f0000000080)={@local, @random="9bddc73ea17a", [], {@ipv4={0x800, {{0x5, 0x4, 0x0, 0x0, 0x2c, 0x0, 0x0, 0x0, 0x6, 0x0, @local}, @tcp={{0x2, 0x2, 0x41424344, 0x41424344, 0x0, 0x0, 0x6, 0x83, 0x0, 0x0, 0x0, {[@sack_perm={0x4, 0x2}]}}}}}}})
//getsockopt$sock_accept_filter(r0, 0xffff, 0x1000, &(0x7f0000000400), &(0x7f0000000180)=0x100)
//setsockopt$inet_tcp_buf(r0, 0x6, 0x401, &(0x7f00000000c0)="487e3f1c48334a6607ae2be8624f8aacdf890138924f828973631c5fd08146f34a271af85553238365dba3e109e5a9bb45b241048c82e4762eccb175dec7b60e0d0c665339e5d6e2821c08b5af44e0dd8416c55616e619a1e0b18f34a3575ea0a935854751ff2444d3", 0x69)
--no_cleanup

 0.000 `kldload -n tcp_rack tcp_bbr`
+0.000 `sysctl -w net.inet.tcp.fastopen.client_enable=0`
+0.000 `sysctl -w net.inet.tcp.fastopen.client_enable=1`
+0.000 `sysctl -w net.inet.tcp.functions_default=bbr`

+0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0.000 setsockopt(3, IPPROTO_TCP, TCP_FUNCTION_BLK, {function_set_name="rack", pcbcnt=0}, 36) = 0
+0.000 setsockopt(3, SOL_SOCKET, SO_REUSEPORT_LB, [1], 4) = 0
+0.000 bind(3, ..., ...) = 0
+0.000 setsockopt(3, IPPROTO_TCP, TCP_LOG, [1], 4) = 0
+0.000 sendto(3, ..., 0, 0, ..., ...) = 0
+0.000 > S   0:0(0)        win 65535 <mss 1460,nop,wscale 6,sackOK,TS val 0 ecr 0>
+0.000 < SF  0:0(0)        win     0 <sackOK,eol,eol>
+0.000 > S.  0:0(0)  ack 1 win 65535 <mss 1460,sackOK,eol,eol>
+0.000 getsockopt(3, SOL_SOCKET, SO_ACCEPTFILTER, {af_name="", af_arg=""}, [256]) = -1 EINVAL (Invalid argument)
+0.000 setsockopt(3, IPPROTO_TCP, TCP_FASTOPEN, {enable=0x487e3f1c, psk=48334a6607ae2be8624f8aacdf890138}, 18) = 0
+0.000 close(3) = 0
//01:04:45 executing program 0:
//syz_emit_ethernet(0x36, &(0x7f0000000080)={@local, @random="9bddc73ea17a", [], {@ipv4={0x800, {{0x5, 0x4, 0x0, 0x0, 0x28, 0x0, 0x0, 0x0, 0x6, 0x0, @local={0xac, 0x14, 0x0}, @broadcast}, @tcp={{0x2, 0x2, 0x41424344, 0x41424344, 0x0, 0x0, 0x5, 0x1}}}}}})
// From the core:
// tp->t_flags = 0x80000e01 (TF_FASTOPEN|TF_NEEDFIN|TF_NEEDSYN|TF_SACK_PERMIT|TF_ACKNOW)
// tp->tp->t_rxtshift = 0
+0.000 < F   0:0(0)        win     0

[rscheff]

Is EINVAL the correct return code? Or should this be EAGAIN / EBUSY?

OTOH, when a listening socket forks into a connected, it's too late to change on the newly connected socket - or can a socket transition back from CLOSED to LISTEN, after a session has concluded? (I know this is not typically done, and I haven't used such recycling of sockets myself either).

IMHO, if a once connected socket can never transition back to listen (or re-used to connect to a the same/different server), EINVAL is a reasonable choice. But if a currently established (and similar states) session could later transition back to LISTEN, EBUSY may be the more appropriate choice (and perhaps EAGAIN during active and passive close states)?

[rrs]

It would not be EAGAIN/EBUSY since you can't do it again and expect it to succeed. I think
EINVAL is ok here.

[tuexen]

In D25115#553203, @rscheff wrote:

Is EINVAL the correct return code? Or should this be EAGAIN / EBUSY?

OTOH, when a listening socket forks into a connected, it's too late to change on the newly connected socket - or can a socket transition back from CLOSED to LISTEN, after a session has concluded? (I know this is not typically done, and I haven't used such recycling of sockets myself either).

A TCP socket in the CLOSED state can transition (via a listen() call) to a TCP socket in the LISTEN state. It can't go back.

IMHO, if a once connected socket can never transition back to listen (or re-used to connect to a the same/different server), EINVAL is a reasonable choice. But if a currently established (and similar states) session could later transition back to LISTEN, EBUSY may be the more appropriate choice (and perhaps EAGAIN during active and passive close states)?

It is not defined what happens after a TCP connections enters the closed state. So I want to be on the safe side.

EINVAL is also returned for the IPV6_ONLY socket option, which can only be used on unbound sockets, when called too late.