Index | Thread | Search

From:
Vitaliy Makkoveev <otto@bsdbox.dev>
Subject:
Re: fstat socket buffer mutex
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
tech@openbsd.org
Date:
Sat, 9 Nov 2024 17:54:06 +0300

Download raw body.

Thread
> On 9 Nov 2024, at 17:50, Vitaliy Makkoveev <otto@bsdbox.dev> wrote:
> 
>> On 9 Nov 2024, at 17:38, Alexander Bluhm <bluhm@openbsd.org> wrote:
>> 
>> Hi,
>> 
>> I wonder if we should also lock so_snd in soo_stat().  Take so_rcv
>> and so_snd mutex together to provide consistent view to userland.
>> 
>> ok?
>> 
> 
> We set SS_CANTRCVMORE and SS_CANTSENDMORE independently from each
> other, no consistency between them.
> 
> I locked so_rcv to keep consistency between sb_cc and sb_state,
> but no locks required for so_snd because we only check sb_state.

solock_shared() was kept because tcp(4) sockets rely on it and I
see no reason to introduce if-tcp-solock branching here. In the
future I plan to push solock() down to pru_sence().

> 
>> bluhm
>> 
>> Index: kern/sys_socket.c
>> ===================================================================
>> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/sys_socket.c,v
>> diff -u -p -r1.65 sys_socket.c
>> --- kern/sys_socket.c	30 Apr 2024 17:59:15 -0000	1.65
>> +++ kern/sys_socket.c	9 Nov 2024 14:33:20 -0000
>> @@ -149,12 +149,14 @@ soo_stat(struct file *fp, struct stat *u
>> 	ub->st_mode = S_IFSOCK;
>> 	solock_shared(so);
>> 	mtx_enter(&so->so_rcv.sb_mtx);
>> +	mtx_enter(&so->so_snd.sb_mtx);
>> 	if ((so->so_rcv.sb_state & SS_CANTRCVMORE) == 0 ||
>> 	    so->so_rcv.sb_cc != 0)
>> 		ub->st_mode |= S_IRUSR | S_IRGRP | S_IROTH;
>> -	mtx_leave(&so->so_rcv.sb_mtx);
>> 	if ((so->so_snd.sb_state & SS_CANTSENDMORE) == 0)
>> 		ub->st_mode |= S_IWUSR | S_IWGRP | S_IWOTH;
>> +	mtx_leave(&so->so_snd.sb_mtx);
>> +	mtx_leave(&so->so_rcv.sb_mtx);
>> 	ub->st_uid = so->so_euid;
>> 	ub->st_gid = so->so_egid;
>> 	(void)pru_sense(so, ub);
>> 
>