TCP crash, possible regression

[axeld]

I just booted into my main Haiku VM after an update to todays revision, and was greeted by this KDL.

[axeld]

Seems to be reproducible 100% on this VM; I got it three times in a row now.

[axeld]

In ​net_socket.cpp, line 792 is an unchecked access to net_socket::parent, which seems to be NULL in this case.

[waddlesplash]

But "parent" is a weak pointer, and we then check whether we actually got a reference on the next line. So that does not look like it can be the problem?

It almost looks like the socket itself might be NULL.

[axeld]

Yes, indeed.

[waddlesplash]

Nothing in _MarkEstablished() invokes socket_aborted directly. This looks like a tail call from something else.

The only thing in the whole tree that calls set_aborted() is TCPEndpoint::_Close. But that invokes has_parent first, so I'd expect that to have crashed instead. So this is strange.

[waddlesplash]

Ah, I think I see the problem. There's a major API footgun here.

[waddlesplash]

Fixed in hrev57645.

[axeld]

I guess you only fixed a symptom, although it doesn't happen directly on boot anymore. Soon after it still crashes again. Stacktrace attached.

[axeld]

Crash with hrev57659

[waddlesplash]

This looks like a completely different problem; or maybe the two are related but the relation isn't clear. This looks to be a use-after-free.

[axeld]

I can open another ticket for it, if you prefer.

[waddlesplash]

I guess it doesn't make much difference so we might as well stick with this one.

[waddlesplash]

How have we gotten to socket_dequeue_connected from tcp_receive_data? The usual path for accept (from a debug build, so no tail calls):

 7 ffffffff81240cc0 (+  48) ffffffff81ee304b   </boot/system/add-ons/kernel/network/stack> socket_dequeue_connected(net_socket*, net_socket**) + 0xb9
 8 ffffffff81240cf0 (+  80) ffffffff81cef9b3   </boot/system/add-ons/kernel/network/protocols/tcp> TCPEndpoint::Accept(net_socket**) + 0x125
 9 ffffffff81240d40 (+  32) ffffffff81ced70d   </boot/system/add-ons/kernel/network/protocols/tcp> tcp_accept(net_protocol*, net_socket**) + 0x23
10 ffffffff81240d60 (+  64) ffffffff81ee38f2   </boot/system/add-ons/kernel/network/stack> socket_accept[clone .localalias] (net_socket*, sockaddr*, unsigned int*, net_socket**) + 0x52
11 ffffffff81240da0 (+  48) ffffffff81eed127   </boot/system/add-ons/kernel/network/stack> stack_interface_accept(net_socket*, sockaddr*, unsigned int*, net_socket**) + 0x3c
12 ffffffff81240dd0 (+ 112) ffffffff801f8bb4   <kernel_x86_64> common_accept(int, sockaddr*, unsigned int*, bool) + 0x76

[waddlesplash]

Actually, an even better question is: how have we gotten to socket_dequeue_connected without a socket_accept appearing anywhere in the stack trace? tcp_accept/TCPEndpoint::Accept isn't called anywhere within the module, and the only thing that calls it in the network module (as far as I can tell) is socket_accept, which has more logic after it calls this hook and couldn't be tail-call-optimized away. So what's going on here?

[waddlesplash]

Ah, I see the problem: your TCP addon is from "non-packaged". In hrev57408 the size of net_socket_module_info changed as some methods were removed. So that's the issue.

[axeld]

Duh, I feel stupid now! Nice catch! I had forgotten about this; I tested turning off SACK. This also fixes my 'git fetch' issues that I did not look into yet, thanks a lot!