Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Use `sb_mtx' instead of `inp_mtx' in receive path for inet sockets
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Mon, 12 Feb 2024 03:25:14 +0300

Download raw body.

Thread

> On 11 Feb 2024, at 21:35, Alexander Bluhm <alexander.bluhm@gmx.net> wrote:
> 
> On Sun, Feb 11, 2024 at 09:16:41PM +0300, Vitaliy Makkoveev wrote:
>>>> @@ -919,7 +929,7 @@ sbappendaddr(struct socket *so, struct s
>>>> 	struct mbuf *m, *n, *nlast;
>>>> 	int space = asa->sa_len;
>>>> 
>>>> -	soassertlocked(so);
>>>> +	sbmtxassertlocked(so, sb);
>>> 
>>> What about the other sbappend...() functions?  If you replace
>>> soassertlocked() also there, we see if you missed something.
>> 
>> Will replace it later, when corresponding sbappend*() will be protected
>> with `sb_mtx'. This time this conversion is useless, but grows the diff.
> 
> My point is that you place the assertion in the functions where you
> added the mutex protection.  This way assert will not find any bug.
> 
> The interesting thing is, whether the socket buffer fields are
> modified elsewhere without protection.  sbmtxassertlocked() knows
> which kind of socket needs the mutex.  So it can be placed in the
> other sbappend*() functions now.  Then unexpecteded calls from
> sockets with SB_MTXLOCK flag are reported.  If you know that
> everything is correct, you don't need the assert at all.
> 

This time only sbappendaddr() requires to hold sb_mtx for some
cases, so it uses sbmtxassertlocked() instead of soassertlocked().
I’ll change other sbappend*() with the upcoming diffs.