sockets: remove the socket-on-stack hack from sorflush()

[glebius]

Summary

The hack can be tracked down to 4.4BSD, where copy was performed
under splimp() and then after splx() dom_dispose was called.
Stevens has a chapter on this function, but he doesn't answer why
this trick is necessary. Why can't we call into dom_dispose under
splimp()? Anyway, with multithreaded kernel the hack seems to be
necessary to avoid LORs between socket buffer lock and different
filesystem locks, especially network file systems.

The new socket buffers KPI sbcut() from 1d2df300e9b allow us to get
rid of the hack.

[markj]

Seems ok, but I'm still not sure about LORs. I would love to get rid of this hack though.

sys/kern/uipc_usrreq.c

2785

Isn't this the same as m = sbcut_locked(sb, sb->sb_ccc);?

2788

sbrelease_locked() asserts this already.

2792

So we still hold the IO lock here, and unp_dispose_mbuf() can close arbitrary descriptors, so a lot of code is reachable from this point (i.e., most implementations of fo_close). How can we be certain that there are no LORs possible here?

[glebius]

sys/kern/uipc_usrreq.c

2785

sbflush_internal() has a comment that suggests a problem exists here. It could be outdated, though.

2792

Good concern. I will update the revision.

[glebius]

sys/kern/uipc_usrreq.c

2788

sbrelease_locked() asserts this already.

2788

sbrelease_locked() asserts this already.

It does so after calling sbcut(). We want to make sure socket buffer is empty after sbcut done by us.

[markj]

sys/kern/uipc_usrreq.c

2785

I suspect it is outdated. Right now the code doesn't even panic in that case, unless INVARIANTS is on.

2786

[glebius]

• Drop the I/O lock earlier
• Don't be afraid to call sbcut_internal() with 0 length

[markj]

sys/kern/uipc_usrreq.c

2792

Actually, I don't see why we need to hold the io lock here at all. Can't it be dropped before calling pru_dispose? We marked the buffer with CANTRCVMORE already.

Hmm, no, it seems that soreceive_generic() removes mbufs from the recv socket buffer after copying them out to userspace, and to do that it needs to drop the socket buffer mutex. That's surprising to me, to be honest.

[glebius]

sys/kern/uipc_usrreq.c

2792

Other reason is to protect against a race between shutdown(2) and listen(2). Not something to be expected from a sane application though.

[glebius]

[markj]

In D35125#796306, @glebius wrote:

So it seems the most likely reason is simply that there was no sbcut()-like operation available.

sys/kern/uipc_usrreq.c

2792

An application may be sane and may also have file descriptor reuse bugs. :)