From: Vitaliy Makkoveev Subject: Re: Unsplice socket before start the teardown To: Alexander Bluhm Cc: tech@openbsd.org Date: Thu, 15 Feb 2024 21:12:52 +0300 > On 15 Feb 2024, at 20:08, Alexander Bluhm wrote: > > On Thu, Feb 15, 2024 at 07:29:53PM +0300, Vitaliy Makkoveev wrote: >> On Thu, Feb 15, 2024 at 01:20:15PM +0100, Alexander Bluhm wrote: >>> On Thu, Feb 15, 2024 at 12:28:28PM +0300, Vitaliy Makkoveev wrote: >>>> Use barriers instead of `ssp_idleto' task and timeout(9) >>>> re-initialization to wait concurrent somove() and soidle(). >>> >>> Why do you move the unsplice code before the solinger sleep? >>> >>> Are you sure that you don't change the semantics? Is all data still >>> spliced before the connection closes? >>> >>> bluhm >>> >> >> The socket is inaccessible from userland, but SO_LINGER keeps it alive, >> mostly to perform transmission of pending data. The new transmissions >> and the reception are impossible. However, this is not true for spliced >> sockets because they are still accessible by the spliced peer, but not >> forever. >> >> IIRC, only tcp_ctlinput() will wakeup us, so we leave this loop after >> TCP_LINGERTIME or if the remote peer was disconnected. >> >> So, for the spliced case, you can receive data and awake somove(), but >> reach `so_linger' timeout and follow the destruction path. Or you could >> pru_send() data to dying `sosp' which already reached `so_linger' >> timeout. You don't know is your spliced peer closed, so I see no >> difference, will it be unspliced now or after TCP_LINGERTIME seconds. >> >> Since the pru_detach() is not called, the remote also doesn't know that >> socket is closed. >> >> I wanted to move it from sofree(). Since I see no differences, the idea >> to do pru_detach() after disconnect and unsplice looked good. >> >> >> This diff keeps current notation, so the unsplice moved before sofree(). > > As there are a lot of corner cases that I have fixed in the years > before, I fear that movin code around may change the semantics. So > I like this versiom much more, I have thrown it on regress machine. > > Before sofree and unsplice was called from in_pcbdetach(). Are you > sure that this is no longer necessary? TCP can get a reset and > call tcp_close(). It has to unsplice then. in_pcbdetach() detaches and destroys PCB, but keeps socket alive. It sets so_pcb to NULL, but keeps SS_NOFDREF clean, so you return just after enter: sofree(struct socket *so, int keep_lock) { int persocket = solock_persocket(so); soassertlocked(so); if (so->so_pcb || (so->so_state & SS_NOFDREF) == 0) { if (!keep_lock) sounlock(so); return; } Otherwise, this this will be use-after-free issue, because the file descriptor is still accessible from userland.