> On 15 Feb 2024, at 20:08, Alexander Bluhm <alexander.bluhm@gmx.net> 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.