> On 25 Dec 2024, at 14:43, Alexander Bluhm <alexander.bluhm@gmx.net> wrote:
> 
> On Tue, Dec 24, 2024 at 10:21:55PM +0300, Vitaliy Makkoveev wrote:
>> @@ -358,20 +353,20 @@ sofree(struct socket *so, int keep_lock)
>> 		(*so->so_proto->pr_domain->dom_dispose)(so->so_rcv.sb_mb);
>> 	m_purge(so->so_rcv.sb_mb);
>> 
>> -	if (!keep_lock)
>> +	if (!keep_lock) {
>> 		sounlock(so);
>> 
>> #ifdef SOCKET_SPLICE
>> -	if (so->so_sp) {
>> -		/* Reuse splice idle, sounsplice() has been called before. */
>> -		timeout_set_flags(&so->so_sp->ssp_idleto, soreaper, so,
>> -		    KCLOCK_NONE, TIMEOUT_PROC | TIMEOUT_MPSAFE);
>> -		timeout_add(&so->so_sp->ssp_idleto, 0);
>> -	} else
>> +		if (so->so_sp) {
>> +			timeout_del_barrier(&so->so_sp->ssp_idleto);
>> +			task_del(sosplice_taskq, &so->so_sp->ssp_task);
>> +			taskq_barrier(sosplice_taskq);
>> +			pool_put(&sosplice_pool, so->so_sp);
>> +		}
>> #endif /* SOCKET_SPLICE */
>> -	{
>> -		pool_put(&socket_pool, so);
>> 	}
>> +
>> +	pool_put(&socket_pool, so);
>> }
>> 
>> static inline uint64_t
> 
> Here you move the pool_put(&sosplice_pool, so->so_sp) into the
> if (!keep_lock) block.
> 
> Do we leak the so->so_sp when in_pcbdetach() calls sofree(so, 1)?
> 
> bluhm

No we don’t. While following this path, we have two cases:

1. Socket was not yet accept()ed, and has no references from
userland. Such sockets can’t be spliced and so->so_sp is always NULL.

2. Socke was accept()ed or was created by socket(). Such sockets
could be spliced but they also have SS_NOFDREF bit clean. So
in_pcbdetach() and sofree(so, 1) will leave such sockets with
destroyed so->so_pcb. It is normal for tcp(4) and udp(4) sockets.

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;
        }