From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: Unlock udp(4) somove()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 19 Jul 2024 19:50:33 +0200

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;

And here
	if (m == NULL) {
		sbdroprecord(so, &so->so_rcv);
		if (so->so_proto->pr_flags & PR_WANTRCVD) {
			mtx_leave(&sosp->so_snd.sb_mtx);
			mtx_leave(&so->so_rcv.sb_mtx);
			pru_rcvd(so);
			mtx_enter(&so->so_rcv.sb_mtx);
			mtx_enter(&sosp->so_snd.sb_mtx);
		}
		goto nextpkt;

>  	/* Move several packets if possible. */
> -	if (!maxreached && nextrecord)
> +	if (!maxreached && nextrecord) {
> +		mtx_leave(&sosp->so_snd.sb_mtx);
> +		mtx_leave(&so->so_rcv.sb_mtx);
>  		goto nextpkt;
> +	}

Then we don't have to unlock here.

I will continue testing and review.

bluhm