From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Unlock udp(4) somove()
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: tech@openbsd.org
Date: Fri, 19 Jul 2024 21:21:15 +0300

> On 19 Jul 2024, at 20:50, Alexander Bluhm <alexander.bluhm@gmx.net> wrote:
> 
> On Fri, Jul 19, 2024 at 07:21:40PM +0300, Vitaliy Makkoveev wrote:
>> I don't know is EPIPE correct here. What about to check `so_pcb' and set
>> error to EPIPE in somove?
>> 
>>        if (error) {
>>                if (sosp->so_snd.sb_state & SS_CANTSENDMORE ||
>> 		    sosp->so_pcb == NULL)
>>                        error = EPIPE;
>>                goto release;
>>        }
> 
> Good idea.
> 
>>  nextpkt:
>> -	if (so->so_error) {
>> -		error = so->so_error;
>> +	mtx_enter(&so->so_rcv.sb_mtx);
>> +	mtx_enter(&sosp->so_snd.sb_mtx);
> 
> Can we move locking mutex before nextpkt?
> 
>> 	if (m == NULL) {
>> +		mtx_leave(&sosp->so_snd.sb_mtx);
>> 		sbdroprecord(so, &so->so_rcv);
>> +		mtx_leave(&so->so_rcv.sb_mtx);
>> 		if (so->so_proto->pr_flags & PR_WANTRCVD)
>> 			pru_rcvd(so);
>> 		goto nextpkt;
> 
> Then we don't have to unlock here.

Are we 100% sure in tcp_rcvd() and the future tcp_rcvd()? We also call
it from soreceive() path where we release mutex first.

Anyway I want to keep pru_rcvd() calls consistent. Both uipc_rcvd() and
route_rcvd() don’t have sleep points, so it looks like we could avoid
mutexes re-locking, but uipc_rcvd() takes both mutexes on so_rcv and
so_snd. So there is hypothetical problem with somove() and unix(4)
sockets.