CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/07/20 11:26:19 Modified files: sys/kern : uipc_socket.c sys/netinet: udp_usrreq.c sys/sys: socketvar.h Log message: Unlock udp(4) somove(). Socket splicing belongs to sockets buffers. udp(4) sockets are fully switched to fine-grained buffers locks, so use them instead of exclusive solock(). Always schedule somove() thread to run as we do for tcp(4) case. This brings delay to packet processing, but it is comparable wit non splicing case where soreceive() threads are always scheduled. So, now spliced udp(4) sockets rely on sb_lock() of `so_rcv' buffer together with `sb_mtx' mutexes of both buffers. Shared solock() only required around pru_send() call, so the most of somove() thread runs simultaneously with network stack. Also document 'sosplice' structure locking. Feedback, tests and OK from bluhm.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/06/28 15:30:24 Modified files: sys/kern : uipc_socket2.c uipc_usrreq.c sys/miscfs/fifofs: fifo_vnops.c Log message: Restore original EPIPE and ENOTCONN errors priority in the uipc_send() path changed in rev 1.206. At least acme-client(1) is not happy with this change. Reported by claudio. Tests and ok by bluhm.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/06/26 06:23:36 Modified files: sys/kern : uipc_usrreq.c Log message: Push socket re-lock to the vnode(9) release path within unp_detach(). The only reason to re-lock dying `so' is the lock order with vnode(9) lock, thus `unp_gc_lock' rwlock(9) could be taken after solock(). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/06/14 02:32:22 Modified files: sys/kern : uipc_socket.c sys/net: rtsock.c Log message: Switch AF_ROUTE sockets to the new locking scheme. At sockets layer only mark buffers as SB_MTXLOCK. At PCB layer only protect `so_rcv' with corresponding `sb_mtx' mutex(9). SS_ISCONNECTED and SS_CANTRCVMORE bits are redundant for AF_ROUTE sockets. Since SS_CANTRCVMORE modifications performed with both solock() and `sb_mtx' held, the 'unlocked' SS_CANTRCVMORE check in rtm_senddesync() is safe. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/20 03:11:21 Modified files: sys/kern : vfs_init.c sys/miscfs/fuse: fuse_vfsops.c Log message: Drop MNT_LOCAL flag in corresponding `vfsconflist' fuse(4) entry instead of cleaning it in fusefs_mount(). ok claudio
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/17 13:11:14 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/sys: socketvar.h Log message: Turn sblock() to `sb_lock' rwlock(9) wrapper for all sockets. Unify behaviour to all sockets. Now sblock() should be always taken before solock() in all involved paths as sosend(), soreceive(), sorflush() and sosplice(). sblock() is fine-grained lock which serializes socket send and receive routines on `so_rcv' or `so_snd' buffers. There is no big problem to wait netlock while holding sblock(). This unification removes a lot of temporary "sb_flags & SB_MTXLOCK" code from sockets layer. This unification makes straight "solock()" and "sblock()" lock order, no more solock() -> sblock() -> sounlock() -> solock() -> sbunlock() -> sounlock() chains in sosend() and soreceive() paths. This unification brings witness(4) support for sblock(), include NFS involved sockets, which is useful. Since the witness(4) support was introduced to sblock() with this diff, some new witness reports appeared. bulk(1) tests by tb, ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/17 13:02:04 Modified files: sys/kern : uipc_socket.c sys/net: pfkeyv2.c Log message: Switch AF_KEY sockets to the new locking scheme. The simplest case. Nothing to change in sockets layer, only set SB_MTXLOCK on socket buffers. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/17 12:58:26 Modified files: sys/net: pfkeyv2.c Log message: Fix uninitialized memory access in pfkeyv2_sysctl(). pfkeyv2_sysctl() reads the SA type from uninitialized memory if it is not provided by the caller of sysctl(2) because of a missing length check. >From Carsten Beckmann. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/07 08:27:11 Modified files: sys/miscfs/fuse: fuse_vfsops.c Log message: Clear MNT_LOCAL flag on FUSE file system. It can be local or remote, but kernel can't tell the difference. >From Kirill A. Korinsky ok claudio mpi
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/03 11:43:10 Modified files: sys/kern : uipc_socket.c uipc_socket2.c uipc_usrreq.c sys/miscfs/fifofs: fifo_vnops.c sys/sys: socketvar.h Log message: Push solock() down to sosend() and remove it from soreceive() paths fro unix(4) sockets. Push solock() deep down to sosend() and remove it from soreceive() paths for unix(4) sockets. The transmission of unix(4) sockets already half-unlocked because connected peer is not locked by solock() during sbappend*() call. Use `sb_mtx' mutex(9) and `sb_lock' rwlock(9) to protect both `so_snd' and `so_rcv'. Since the `so_snd' is protected by `sb_mtx' mutex(9) the re-locking is not required in uipc_rcvd(). Do direct `so_rcv' dispose and cleanup in sofree(). This sockets is almost dead and unlinked from everywhere include spliced peer, so concurrent sotask() thread will just exit. This required to keep locks order between `i_lock' and `sb_lock'. Also this removes re-locking from sofree() for all sockets. SB_OWNLOCK became redundant with SB_MTXLOCK, so remove it. SB_MTXLOCK was kept because checks against SB_MTXLOCK within sb*() routines are mor consistent. Feedback and ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/02 15:26:52 Modified files: sys/kern : uipc_socket2.c Log message: Quick fix previous one. socantrcvmore() should raise assertion if `so_rcv' has SB_MTXLOCK flag clean, not SB_OWNLOCK. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/02 11:10:55 Modified files: sys/kern : uipc_usrreq.c Log message: Don't re-lock sockets in uipc_shutdown(). No reason to lock peer. It can't be or became listening socket, both sockets can't be in the middle of connecting or disconnecting. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/05/02 05:55:31 Modified files: sys/kern : uipc_socket.c Log message: Pass `sosp' instead of `so' to sblock() when locking `so_snd' within sosplice(). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/04/30 11:59:15 Modified files: sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c Log message: Push solock() down to sosend() for SOCK_RAW sockets. Raw sockets are the simplest inet sockets, so use them to start landing `sb_mtx' mutex(9) protection for `so_snd' buffer. Now solock() is taken only around pru_send*(), the rest of sosend() serialized by sblock() and `sb_mtx'. The unlocked SS_ISCONNECTED check is fine, because rip{,6}_send() check it. Also, previously the SS_ISCONNECTED could be lost due to solock() release around following m_getuio(). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/04/15 15:31:29 Modified files: sys/kern : uipc_socket.c Log message: Don't take solock() in soreceive() for udp(4) sockets. These sockets are not connection oriented, they don't call pru_rcvd(), but they have splicing ability and they set `so_error'. Splicing ability is the most problem. However, we can hold `sb_mtx' around `ssp_socket' modifications together with solock(). So the `sb_mtx' is pretty enough to isspiced() check in soreceive(). The unlocked `so_sp' dereference is fine, because we set it only once for the whole socket life-time and we do this before `ssp_socket' assignment. We also need to take sblock() before splice sockets, so the sosplice() and soreceive() are both serialized. Since `sb_mtx' required to unsplice sockets too, it also serializes somove() with soreceive() regardless on somove() caller. The sosplice() was reworked to accept standalone sblock() for udp(4) sockets. soreceive() performs unlocked `so_error' check and modification. Previously, we have no ability to predict which concurrent soreceive() or sosend() thread will fail and clean `so_error'. With this unlocked access we could have sosend() and soreceive() threads which fails together. `so_error' stored to local `error2' variable because `so_error' could be overwritten by concurrent sosend() thread. Tested and ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/04/11 07:32:51 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/sys: socketvar.h Log message: Don't take solock() in soreceive() for SOCK_RAW inet sockets. For inet sockets solock() is the netlock wrapper, so soreceive() could be performed simultaneous with exclusively locked code paths. These sockets are not connection oriented, they don't call pru_rcvd(), they can't be spliced, they don't set `so_error'. Nothing to protect with solock() in soreceive() path. `so_rcv' buffer protected by `sb_mtx' mutex(9), but since it released, sblock() required to serialize concurrent soreceive() and sorflush() threads. Current sblock() is some kind of rwlock(9) implementation, so introduce `sb_lock' rwlock(9) and use it directly for that purpose. The sorflush() and callers were refactored to avoid solock() for raw inet sockets. This was done to avoid packet processing stop. Tested and ok bluhm.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/04/11 02:33:37 Modified files: sys/kern : sys_socket.c Log message: Take solock_shared() in soo_stat(). Only unix(4) and tcp(4) sockets set (*pru_sence)() handler. The rest of soo_stat() is the read only access. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/04/10 06:04:41 Modified files: sys/kern : uipc_socket.c uipc_socket2.c uipc_usrreq.c sys/sys: socketvar.h Log message: Remove `head' socket re-locking in sonewconn(). uipc_attach() releases solock() because it should be taken after `unp_gc_lock' rwlock(9) which protects the `unp_link' list. For this reason, the listening `head' socket should be unlocked too while sonewconn() calls uipc_attach(). This could be reworked because now `so_rcv' sockbuf relies on `sb_mtx' mutex(9). The last one `unp_link' foreach loop within unp_gc() discards sockets previously marked as UNP_GCDEAD. These sockets are not accessed from the userland. The only exception is the sosend() threads of connected sending peers, but they only sbappend*() mbuf(9) to `so_rcv'. So it's enough to unlink mbuf(9) chain with `sb_mtx' held and discard lockless. Please note, the existing SS_NEWCONN_WAIT logic was never used because the listening unix(4) socket protected from concurrent unp_detach() by vnode(9) lock, however `head' re-locked all times. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/04/02 06:21:40 Modified files: sys/kern : uipc_socket.c Log message: Remove wrong "temporary udp error" comment in filt_so{read,write}(). Not only udp(4) sockets set and check `so_error'. No functional changes. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/31 08:01:28 Modified files: sys/kern : uipc_socket.c Log message: Allow listen(2) only on sockets of type SOCK_STREAM or SOCK_SEQPACKET. listen(2) man(1) page clearly prohibits sockets of other types. Reported-by: syzbot+00450333592fcd38c...@syzkaller.appspotmail.com ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/31 07:50:00 Modified files: sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c sys/nfs: nfs_socket.c nfs_syscalls.c Log message: Mark `so_rcv' sockbuf of udp(4) sockets as SB_OWNLOCK. sbappend*() and soreceive() of SB_MTXLOCK marked sockets uses `sb_mtx' mutex(9) for protection, meanwhile buffer usage check and corresponding sbwait() sleep still serialized by solock(). Mark udp(4) as SB_OWNLOCK to avoid solock() serialization and rely to `sb_mtx' mutex(9). The `sb_state' and `sb_flags' modifications must be protected by `sb_mtx' too. ok bluhm
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 11:35:01PM +0100, Alexander Bluhm wrote: > On Thu, Mar 28, 2024 at 01:21:17AM +0300, Vitaliy Makkoveev wrote: > > On Wed, Mar 27, 2024 at 10:51:09PM +0100, Alexander Bluhm wrote: > > > On Wed, Mar 27, 2024 at 10:26:27PM +0300, Vitaliy Makkoveev wrote: > > > > On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote: > > > > > > > > > > Observing two regress hangs in the kernel on netio. Both seems make > > > > > use > > > > > of unix sockets. Could this be the culprit? > > > > > > > > > > regress/lib/libc/fread > > > > > regress/usr.bin/ssh (scp.sh) > > > > > > > > Sorry for delay. > > > > > > > > It was exposed that `sb_mtx' should not be released between `so_rcv' > > > > usage check and corresponding sbwait() sleep. Otherwise wakeup() could > > > > be lost sometimes. > > > > > > > > This diff fixed regress tests. It introduces sbunlock_locked() and > > > > sbwait_locked() to perform with `sb_mtx' held. > > > > > > Do we really need a restart_unlocked label? > > > Instead of goto restart_unlocked you could call solock_shared(so) > > > and fall through goto restart; > > > > > > > This was the first version, but later I decided 'restart_unlocked' is > > better. No problems to return it back. > > > > > I don't like pr_protocol == AF_UNIX check in generic soreceive() > > > function. Could we check SB_MTXLOCK instead? > > > > > > > Me too. I used "pr_protocol == AF_UNIX" check because sosend() does the > > same. The existing SB_MTXLOCK used by some inet sockets and I strictly > > want to move them separately of this fix. > > > > I propose to introduce SB_OWNLOCK flag and check it. I wanted to use > > this flag for socket with standalone sblock() ability to modify > > sblock()/sbwait() behaviour. I need this flag to convert all > > SB_MTXLOCK'ed sockets separately. After conversion, both SB_MTXLOCK and > > SB_OWNLOCK will be removed. > > > > I propose to introduce SB_OWNLOCK right now and use it instead of > > "pr_protocol == AF_UNIX" check to keep inet sockets as is. I have no > > objections to convert them too, but separately. > > sb_flags is short, can you use 4 digits 0x for the flags? > And please use a space after #define, then the diff is less ugly. > Done. Thanks.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/27 16:47:53 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/sys: socketvar.h Log message: Introduce SB_OWNLOCK to mark sockets which `so_rcv' buffer modified outside socket lock. `sb_mtx' mutex(9) used for this case and it should not be released between `so_rcv' usage check and corresponding sbwait() sleep. Otherwise wakeup() could be lost sometimes. ok bluhm
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 10:51:09PM +0100, Alexander Bluhm wrote: > On Wed, Mar 27, 2024 at 10:26:27PM +0300, Vitaliy Makkoveev wrote: > > On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote: > > > > > > Observing two regress hangs in the kernel on netio. Both seems make use > > > of unix sockets. Could this be the culprit? > > > > > > regress/lib/libc/fread > > > regress/usr.bin/ssh (scp.sh) > > > > Sorry for delay. > > > > It was exposed that `sb_mtx' should not be released between `so_rcv' > > usage check and corresponding sbwait() sleep. Otherwise wakeup() could > > be lost sometimes. > > > > This diff fixed regress tests. It introduces sbunlock_locked() and > > sbwait_locked() to perform with `sb_mtx' held. > > Do we really need a restart_unlocked label? > Instead of goto restart_unlocked you could call solock_shared(so) > and fall through goto restart; > This was the first version, but later I decided 'restart_unlocked' is better. No problems to return it back. > I don't like pr_protocol == AF_UNIX check in generic soreceive() > function. Could we check SB_MTXLOCK instead? > Me too. I used "pr_protocol == AF_UNIX" check because sosend() does the same. The existing SB_MTXLOCK used by some inet sockets and I strictly want to move them separately of this fix. I propose to introduce SB_OWNLOCK flag and check it. I wanted to use this flag for socket with standalone sblock() ability to modify sblock()/sbwait() behaviour. I need this flag to convert all SB_MTXLOCK'ed sockets separately. After conversion, both SB_MTXLOCK and SB_OWNLOCK will be removed. I propose to introduce SB_OWNLOCK right now and use it instead of "pr_protocol == AF_UNIX" check to keep inet sockets as is. I have no objections to convert them too, but separately. Index: sys/kern/uipc_socket.c === RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.322 diff -u -p -r1.322 uipc_socket.c --- sys/kern/uipc_socket.c 26 Mar 2024 09:46:47 - 1.322 +++ sys/kern/uipc_socket.c 27 Mar 2024 22:12:55 - @@ -161,7 +161,7 @@ soalloc(const struct protosw *prp, int w } break; case AF_UNIX: - so->so_rcv.sb_flags |= SB_MTXLOCK; + so->so_rcv.sb_flags |= SB_MTXLOCK | SB_OWNLOCK; break; } @@ -903,12 +903,23 @@ restart: } SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1"); SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1"); - sb_mtx_unlock(>so_rcv); - sbunlock(so, >so_rcv); - error = sbwait(so, >so_rcv); - if (error) { + + if (so->so_rcv.sb_flags & SB_OWNLOCK) { + sbunlock_locked(so, >so_rcv); sounlock_shared(so); - return (error); + error = sbwait_locked(so, >so_rcv); + sb_mtx_unlock(>so_rcv); + if (error) + return (error); + solock_shared(so); + } else { + sb_mtx_unlock(>so_rcv); + sbunlock(so, >so_rcv); + error = sbwait(so, >so_rcv); + if (error) { + sounlock_shared(so); + return (error); + } } goto restart; } Index: sys/kern/uipc_socket2.c === RCS file: /cvs/src/sys/kern/uipc_socket2.c,v retrieving revision 1.145 diff -u -p -r1.145 uipc_socket2.c --- sys/kern/uipc_socket2.c 26 Mar 2024 09:46:47 - 1.145 +++ sys/kern/uipc_socket2.c 27 Mar 2024 22:12:55 - @@ -523,6 +523,18 @@ sbmtxassertlocked(struct socket *so, str * Wait for data to arrive at/drain from a socket buffer. */ int +sbwait_locked(struct socket *so, struct sockbuf *sb) +{ + int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH; + + MUTEX_ASSERT_LOCKED(>sb_mtx); + + sb->sb_flags |= SB_WAIT; + return msleep_nsec(>sb_cc, >sb_mtx, prio, "sbwait", + sb->sb_timeo_nsecs); +} + +int sbwait(struct socket *so, struct sockbuf *sb) { uint64_t timeo_nsecs; @@ -573,20 +585,23 @@ out: } void -sbunlock(struct socket *so, struct sockbuf *sb) +sbunlock_locked(struct socket *so, struct sockbuf *sb) { - int dowakeup = 0; + MUTEX_ASSERT_LOCKED(>sb_mtx); - mtx_enter(>sb_mtx); sb->sb_flags &= ~SB_LOCK; if (sb->sb_flags & SB_WANT) {
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote: > > Observing two regress hangs in the kernel on netio. Both seems make use > of unix sockets. Could this be the culprit? > > regress/lib/libc/fread > regress/usr.bin/ssh (scp.sh) Sorry for delay. It was exposed that `sb_mtx' should not be released between `so_rcv' usage check and corresponding sbwait() sleep. Otherwise wakeup() could be lost sometimes. This diff fixed regress tests. It introduces sbunlock_locked() and sbwait_locked() to perform with `sb_mtx' held. Index: sys/kern/uipc_socket.c === RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.322 diff -u -p -r1.322 uipc_socket.c --- sys/kern/uipc_socket.c 26 Mar 2024 09:46:47 - 1.322 +++ sys/kern/uipc_socket.c 27 Mar 2024 19:17:52 - @@ -834,6 +834,7 @@ bad: if (mp) *mp = NULL; +restart_unlocked: solock_shared(so); restart: if ((error = sblock(so, >so_rcv, SBLOCKWAIT(flags))) != 0) { @@ -903,12 +904,23 @@ restart: } SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1"); SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1"); - sb_mtx_unlock(>so_rcv); - sbunlock(so, >so_rcv); - error = sbwait(so, >so_rcv); - if (error) { + + if (so->so_proto->pr_protocol == AF_UNIX) { + sbunlock_locked(so, >so_rcv); sounlock_shared(so); - return (error); + error = sbwait_locked(so, >so_rcv); + sb_mtx_unlock(>so_rcv); + if (error) + return (error); + goto restart_unlocked; + } else { + sb_mtx_unlock(>so_rcv); + sbunlock(so, >so_rcv); + error = sbwait(so, >so_rcv); + if (error) { + sounlock_shared(so); + return (error); + } } goto restart; } Index: sys/kern/uipc_socket2.c === RCS file: /cvs/src/sys/kern/uipc_socket2.c,v retrieving revision 1.145 diff -u -p -r1.145 uipc_socket2.c --- sys/kern/uipc_socket2.c 26 Mar 2024 09:46:47 - 1.145 +++ sys/kern/uipc_socket2.c 27 Mar 2024 19:17:52 - @@ -523,6 +523,18 @@ sbmtxassertlocked(struct socket *so, str * Wait for data to arrive at/drain from a socket buffer. */ int +sbwait_locked(struct socket *so, struct sockbuf *sb) +{ + int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH; + + MUTEX_ASSERT_LOCKED(>sb_mtx); + + sb->sb_flags |= SB_WAIT; + return msleep_nsec(>sb_cc, >sb_mtx, prio, "sbwait", + sb->sb_timeo_nsecs); +} + +int sbwait(struct socket *so, struct sockbuf *sb) { uint64_t timeo_nsecs; @@ -573,20 +585,23 @@ out: } void -sbunlock(struct socket *so, struct sockbuf *sb) +sbunlock_locked(struct socket *so, struct sockbuf *sb) { - int dowakeup = 0; + MUTEX_ASSERT_LOCKED(>sb_mtx); - mtx_enter(>sb_mtx); sb->sb_flags &= ~SB_LOCK; if (sb->sb_flags & SB_WANT) { sb->sb_flags &= ~SB_WANT; - dowakeup = 1; + wakeup(>sb_flags); } - mtx_leave(>sb_mtx); +} - if (dowakeup) - wakeup(>sb_flags); +void +sbunlock(struct socket *so, struct sockbuf *sb) +{ + mtx_enter(>sb_mtx); + sbunlock_locked(so, sb); + mtx_leave(>sb_mtx); } /* Index: sys/sys/socketvar.h === RCS file: /cvs/src/sys/sys/socketvar.h,v retrieving revision 1.126 diff -u -p -r1.126 socketvar.h --- sys/sys/socketvar.h 26 Mar 2024 09:46:47 - 1.126 +++ sys/sys/socketvar.h 27 Mar 2024 19:17:53 - @@ -320,6 +320,7 @@ int sblock(struct socket *, struct sockb /* release lock on sockbuf sb */ void sbunlock(struct socket *, struct sockbuf *); +void sbunlock_locked(struct socket *, struct sockbuf *); #defineSB_EMPTY_FIXUP(sb) do { \ if ((sb)->sb_mb == NULL) { \ @@ -367,6 +368,7 @@ int sbcheckreserve(u_long, u_long); intsbchecklowmem(void); intsbreserve(struct socket *, struct sockbuf *, u_long); intsbwait(struct socket *, struct sockbuf *); +intsbwait_locked(struct socket *, struct sockbuf *); void soinit(void); void soabort(struct socket *); intsoaccept(struct socket *, struct mbuf *);
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/25 11:43:10 Modified files: sys/kern : init_sysent.c syscalls.c sys/sys: syscall.h syscallargs.h Log message: regen
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/25 11:42:34 Modified files: sys/kern : syscalls.master Log message: Unlock shutdown(2). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/25 07:01:49 Modified files: sys/dev/wscons : wsevent.c wseventvar.h wskbd.c wsmouse.c wsmux.c wstpad.c Log message: Add 'ws_' prefix to 'wseventvar' structure members. No functional changes. ok miod
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/22 19:35:57 Modified files: regress/sys/kern/unixsockets: unixsock_test.c Log message: Fix main() definition.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/22 11:34:11 Modified files: sys/kern : uipc_socket.c uipc_usrreq.c sys/sys: socketvar.h Log message: Use sorflush() instead of direct unp_scan(..., unp_discard) to discard dead unix(4) sockets. The difference in direct unp_scan() and sorflush() is the mbuf(9) chain. For the first case it is still linked to the `so_rcv', for the second it is not. This is required to make `sb_mtx' mutex(9) the only `so_rcv' sockbuf protection and remove socket re-locking from the most of uipc_*send() paths. The unlinked mbuf(9) chain doesn't require any protection, so this allows to perform sleeping unp_discard() lockless. Also, the mbuf(9) chain of the discarded socket still contains addresses of file descriptors and it is much safer to unlink it before FRELE() them. This is the reason to commit this diff standalone. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/17 13:47:08 Modified files: sys/kern : uipc_usrreq.c Log message: Do UNP_CONNECTING and UNP_BINDING flags check in uipc_listen() and return EINVAL if set. This prevents concurrent solisten() thread to make this socket listening while socket is unlocked. Reported-by: syzbot+4acfcd73d15382a3e...@syzkaller.appspotmail.com ok mpi
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/03/05 10:48:01 Modified files: sys/net: if_wg.c wg_noise.c wg_noise.h Log message: Convert `t_lock', `r_keypair_lock' and `c_lock' rwlock(9)s to corresponding mutex(9)es. ifq_start() and following wg_qstart() could be called from software interrupt context if bandwidth control is enabled in pf.conf(5). Remove sleep points provided by rwlock(9)s from wg(4) output start routine. looks ok claudio
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/02/12 15:48:27 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/sys: socketvar.h Log message: Pass protosw instead of domain structure to soalloc() to get real `pr_type'. The corresponding domain is referenced as `pr_domain'. Otherwise dp->dom_protosw->pr_type of inet sockets always points to inetsw[0]. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/02/11 14:36:49 Modified files: sys/kern : uipc_socket.c Log message: Release `sb_mtx' mutex(9) before sbunlock(). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/02/11 11:14:27 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/netinet: ip_divert.c ip_mroute.c raw_ip.c udp_usrreq.c sys/netinet6 : ip6_divert.c ip6_mroute.c raw_ip6.c sys/sys: socketvar.h Log message: Use `sb_mtx' instead of `inp_mtx' in receive path for inet sockets. In soreceve(), we only touch `so_rcv' socket buffer, which has it's own `sb_mtx' mutex(9) for protection. So, we can avoid solock() in this path - it's enough to hold `sb_mtx' in soreceive() and around corresponding sbappend*(). But not right now :) This time we use shared netlock for some inet sockets in the soreceive() path. To protect `so_rcv' buffer we use `inp_mtx' mutex(9) and the pru_lock() to acquire this mutex(9) in socket layer. But the `inp_mtx' mutex belongs to the PCB. We initialize socket before PCB, tcp(4) sockets could exist without PCB, so use `sb_mtx' mutex(9) to protect sockbuf stuff. This diff mechanically replaces `inp_mtx' by `sb_mtx' in the receive path. Only for sockets which already use `inp_mtx'. All other sockets left as is. They will be converted later. Since the `sb_mtx' is optional, the new SB_MTXLOCK flag introduced. If this flag is set on `sb_flags', the `sb_mtx' mutex(9) should be taken. New sb_mtx_lock() and sb_mtx_unlock() was introduced to hide this check. They are temporary and will be replaced by mtx_enter() when all this area will be converted to `sb_mtx' mutex(9). Also, the new sbmtxassertlocked() function introduced to throw corresponding assertion for SB_MTXLOCK marked buffers. This time only sbappendaddr() calls it. This function is also temporary and will be replaced by MTX_ASSERT_LOCKED() later. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/02/05 13:21:39 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/nfs: nfs_socket.c nfs_syscalls.c Log message: Use `sb_mtx' mutex(9) to protect `sb_timeo_nsecs'. In most places solock() is still held because other 'sockbuf' members require it, but in so{g,s}etopt() paths solock() is avoided. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/02/05 11:27:47 Modified files: sys/net: if.c Log message: Don't send route messages while rebooting after panic. Syskaller exposed [1] that if_downall() tries to send route messages and triggers panic again but in knote(9) layer. 1. https://syzkaller.appspot.com/bug?extid=d19060a65721eb432a72 ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/02/03 15:50:09 Modified files: sys/kern : uipc_socket.c uipc_socket2.c uipc_syscalls.c sys/miscfs/fifofs: fifo_vnops.c sys/netinet: ip_divert.c ip_divert.h ip_var.h raw_ip.c udp_usrreq.c udp_var.h sys/netinet6 : ip6_divert.c ip6_mroute.c ip6_var.h raw_ip6.c sys/sys: mutex.h protosw.h socketvar.h Log message: Rework socket buffers locking for shared netlock. Shared netlock is not sufficient to call so{r,w}wakeup(). The following sowakeup() modifies `sb_flags' and knote(9) stuff. Unfortunately, we can't call so{r,w}wakeup() with `inp_mtx' mutex(9) because sowakeup() also calls pgsigio() which grabs kernel lock. However, `so*_filtops' callbacks only perform read-only access to the socket stuff, so it is enough to hold shared netlock only, but the klist stuff needs to be protected. This diff introduces `sb_mtx' mutex(9) to protect sockbuf. This time `sb_mtx' used to protect only `sb_flags' and `sb_klist'. Now we have soassertlocked_readonly() and soassertlocked(). The first one is happy if only shared netlock is held, meanwhile the second wants `so_lock' or pru_lock() be held together with shared netlock. To keep soassertlocked*() assertions soft, we need to know mutex(9) state, so new mtx_owned() macro was introduces. Also, the new optional (*pru_locked)() handler brings the state of pru_lock(). Tests and ok from bluhm.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/26 11:24:58 Modified files: sys/kern : init_sysent.c syscalls.c sys/sys: syscall.h syscallargs.h Log message: regen
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/26 11:24:23 Modified files: sys/kern : syscalls.master uipc_socket.c Log message: Unlock listen(2). `somaxconn_local' and `sominconn_local' used respectively to cache values as we do in other places. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/23 09:57:53 Modified files: sys/net: pipex.c pipex_local.h Log message: Remove `pipex_rd_head6' and `ps6_rn[2]'. They are not used. ok yasuoka
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/18 01:48:33 Modified files: sys/kern : kern_sysctl.c Log message: Use solock() instead of netlock within fill_file(). This makes all socket types protected. The netlock is still used while fill_file() called through *table.inpt_queue walkthroughs, but this is the inet sockets case. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/18 01:46:41 Modified files: sys/net: if_wg.c Log message: Use `nowake' as tsleep_nsec(9) ident. It has no corresponding wakeup(9). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/15 08:47:37 Modified files: sys/kern : kern_exit.c kern_fork.c kern_proc.c kern_sysctl.c sys/sys: proc.h Log message: Introduce priterator(), the `ps_list' iterator. Some of `allprocess' list walkthroughs have context switch within, so make exit1() wait until the last reference released. Reported-by: syzbot+0e9dda76c42c82c62...@syzkaller.appspotmail.com ok bluhm claudio
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/03 18:32:06 Modified files: sys/dev/pv : hypervic.c Log message: Revert previous. splx(9) can call kvp_get_ip_info() from any place with netlock held and cause recursive lock acquisition issue.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2024/01/01 11:47:02 Modified files: sys/net: if_gif.c if_mpe.c if_mpip.c if_wg.c Log message: Call if_counters_alloc() before if_attach(). ok bluhm sashan
Re: CVS: cvs.openbsd.org: src
> On 23 Dec 2023, at 13:15, Alexander Bluhm wrote: > > On Fri, Dec 22, 2023 at 04:01:50PM -0700, Vitaliy Makkoveev wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: m...@cvs.openbsd.org2023/12/22 16:01:50 >> >> Modified files: >> sys/net: if.c if_aggr.c if_bpe.c if_etherip.c if_gif.c >> if_gre.c if_mpe.c if_mpip.c if_mpw.c if_pflow.c >> if_pfsync.c if_pppx.c if_sec.c if_tpmr.c >> if_trunk.c if_tun.c if_var.h if_veb.c if_vlan.c >> if_vxlan.c if_wg.c >> sys/netinet: ip_carp.c >> >> Log message: >> Always allocate per-CPU statistics counters for network interface >> descriptor. > > It looks like this breaks interface attach. > It should be backed out. > Thanks for backout and sorry for delay.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/22 16:01:50 Modified files: sys/net: if.c if_aggr.c if_bpe.c if_etherip.c if_gif.c if_gre.c if_mpe.c if_mpip.c if_mpw.c if_pflow.c if_pfsync.c if_pppx.c if_sec.c if_tpmr.c if_trunk.c if_tun.c if_var.h if_veb.c if_vlan.c if_vxlan.c if_wg.c sys/netinet: ip_carp.c Log message: Always allocate per-CPU statistics counters for network interface descriptor. We have the mess in network interface statistics. Only pseudo drivers do per-CPU counters allocation, all other network devices use the old `if_data'. The network stack partially uses per-CPU counters and partially use `if_data', but the protection is inconsistent: some times counters accessed with exclusive netlock, some times with shared netlock, some times with kernel lock, but without netlock, some times with another locks. To make network interfaces statistics more consistent, always allocate per-CPU counters at interface attachment time and use it instead of `if_data'. At this step only move counters allocation to the if_attach() internals. The `if_data' removal will be performed with the following diffs to make review and tests easier. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/19 13:34:10 Modified files: sys/net: if_pflow.c Log message: Initialize `sc_outputtask' before interface attachment. if_alloc_sadl() has sleep point, so the uninitialized `sc_outputtask` could be accessed through ioctl(2) interface. ok sashan bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/16 15:17:08 Modified files: sys/miscfs/fuse: fuse_device.c Log message: Make `fuse_rd_filtops' mpsafe. Introduce `fd_lock' rwlock(9) and use it for `fd_fbufs_in' fuse buffers queue and `fd_rklist' knotes list protection. Tested by Rafael Sadowski. Discussed with and ok from bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/16 15:16:02 Modified files: sys/net: if_pflow.c if_pflow.h Log message: Rework pflowioctl() lock dances. Release netlock and take `sc_lock' rwlock(9) just in the beginning of pflowioctl() and do corresponding operations in the end. Use `sc_lock' to protect `sc_dying'. We need to release netlock not only to keep locks order with `sc_lock' rwlock(9), but also because pflowioctl() calls some operations like socreate() or soclose() on udp(4) socket. Current implementation has many relocking places which breaks atomicy, so merge them into one. The `sc_lock' rwlock(9) is taken during all pflowioctl() call, so `sc_dying' atomicy is not broken. Not the ideal solution, but better then we have now. Tested by Hrvoje Popovski. Discussed with and ok from sashan
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/12 05:38:52 Modified files: sys/net: if_pflow.c Log message: slyle(9) fix. No functional changes.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/11 17:03:31 Modified files: sys/net: if_pflow.c if_pflow.h Log message: Turn `pflowstats' statistics counters into per-CPU counters to make them mpsafe. The weird interactions around `pflow_flows' and `sc_gcounter' replaced by simple `pflow_flows' increment. Since the flow sequence is the 32 bits integer, the `sc_gcounter' type replaced by the type of uint32_t. ok bluhm sashan
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/11 07:25:09 Modified files: sys/net: if_pflow.c if_pflow.h Log message: Turn `pflow_softc' list into SMR list. Since the revision 1.1182 of net/pf.c netlock is not taken while export_pflow() called from pf_purge_states(). Current locks order requires netlock to be taken before PF_LOCK(), so there is no reason to turn it back into this path only for optional export_pflow() call. The `pflowif_list' foreach loop has no context switch within, so SMR list is better than mutex(9). Tested by Hrvoje Popovski. ok sashan bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/08 16:15:44 Modified files: sys/net: if_pflow.c Log message: Add spaces around '='. style(9) fix, no functional changes.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/08 16:13:40 Modified files: sys/net: if_pflow.c if_pflow.h Log message: Introduce `sc_mtx' mutex(9) to protect the most of pflow_softc structure. Protect the `send_nam', `sc_flowsrc' and `sc_flowdst' pflow_softc members by existing `sc_lock' rwlock(9). This partially fixes locking inconsistency of pflow_softc. The following work will be done with separate diffs. Also, pass `sc' instead of NULL to pflow_get_mbuf() while calling from pflow_sendout_ipfix_tmpl(). This fixes the NULL dereference. ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/03 03:51:17 Modified files: sys/net: rtsock.c Log message: Make rtm_senddesync_timer() timeout(9) handler mpsafe. solock() protects the socket and the socket's PCB data. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/03 03:50:26 Modified files: sys/netinet: ip_ipsp.c Log message: Make ipsp_ids_gc() timeout(9) handler mpsafe. `ipsec_flows_mtx' mutex(9) protects related data. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/12/01 13:30:22 Modified files: sys/net: pipex.c Log message: pipex(4) layer is completely mp-safe, move the pipex_timer() timeout(9) handler out of kernel lock. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/11/09 01:53:20 Modified files: sys/net: if_pflow.c Log message: Remove delayed timeout(9) initialization. timeout_set*() only assign members of passed timeout structure, this delayed initialization provides nothing but makes code weird. ok kn
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/10/25 03:36:47 Modified files: sys/dev: vscsi.c Log message: Make `vscsi_filtops' mpsafe. filt_vscsiread() checks `sc_ccb_i2t' protected by `sc_state_mtx' mutex(9), so use it to protect `sc_klist' knotes list too. ok claudio
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/10/23 04:22:06 Modified files: sys/net: if_wg.c Log message: Prevent wg(4) stuck on peer destruction. While interface going down and output stopped, packets could rest in `if_snd' queue. So the (!ifq_empty(>sc_if.if_snd)) condition will always be true and wg_peer_destroy() will sleep until interface became up and stuck packets transmitted. Check IFF_RUNNING flag within (!ifq_empty(>sc_if.if_snd)) loop in wg_peer_destroy(). If the flag is not set that means interface is down, so drain the `if_snd' queue manually to prevent wg_peer_destroy() stuck. Problem reported and fix tested by Kirill Miazine. ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/10/05 12:46:14 Modified files: usr.sbin/dhcpd : dhcpd.8 dhcpd.c Log message: Do log output to stderr while running dhcpd(8) in foreground to make behaviour in accordance with man page. Introduce '-v' option to make output more verbose. Do a little refactoring to make code more consistent with other daemons like ospfd(8), httpd(8), relayd(8), etc. Feedback from bluhm benno ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/26 13:55:24 Modified files: sys/dev: midi.c midivar.h Log message: Use existing `audio_lock' mutex(9) to make `midi{read,write}_filtops' MP safe. knote_locked(9) will not grab kernel lock, so call it directly from interrupt handlers instead of scheduling software interrupts. feedback and ok ratchov
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/26 02:30:13 Modified files: sys/dev/pv : vmt.c Log message: Use shared netlock to protect ifnet data within vmt_tclo_broadcastip(). Execute vmt_tclo_tick() timeout handler in process context to allow context switch within vmt_tclo_broadcastip(). ok yasuoka
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/23 07:01:13 Modified files: sys/dev/pv : hypervic.c Log message: Use shared netlock to protect if_list and ifa_list walkthrough and ifnet data access within kvp_get_ip_info(). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/22 16:12:32 Modified files: sys/dev: hotplug.c Log message: Introduce `hotplug_mtx' mutex(9) and make `hotplugread_filtops' MP safe. Use this mutex(9) to protect `evqueue_head', `evqueue_tail' and `evqueue_count'. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/22 14:03:05 Modified files: sys/kern : subr_log.c Log message: Make `logread_filterops' MP safe. For that purpose use `log_mtx' mutex(9) protecting message buffer. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/13 08:24:37 Modified files: sys/dev/pci/drm/include/drm: drm_device.h Log message: Replace sys/selinfo.h header with sys/event.h. drm_device.h has no selinfo stuff, but the `note' member of 'drm_device' structure with type of klist. ok jsg
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/11 06:10:47 Modified files: sys/dev/ic : aac.c Log message: Remove unnecessary includes. ok jsg
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/11 02:40:25 Modified files: sys/dev/ic : aacvar.h sys/dev/pci: aac_pci.c Log message: Kill unused `aac_select'. Build test performed with uncommented aac(4) in GENERIC. ok jsg
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/08 14:00:28 Modified files: sys/dev: hotplug.c sys/dev/wscons : wseventvar.h sys/isofs/cd9660: cd9660_vnops.c sys/miscfs/fuse: fuse_device.c fuse_vnops.c sys/msdosfs: msdosfs_vnops.c sys/nfs: nfs_kq.c sys/sys: vnode.h sys/tmpfs : tmpfs_vnops.c sys/ufs/ufs: ufs_vnops.c Log message: Remove the remnants of the leftover selinfo from vnode(9) layer. Just mechanical 'selinfo' to 'klist' replacement in 'vnode' structure because knote(9) API is already used. headers added where is was required. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/09/01 14:24:29 Modified files: sys/dev/usb: if_umb.c Log message: Call rtm_send() with netlock held to protect dereference of sockaddr structure data returned by rtable_getsource(). Netlock can't be pushed within rtm_send() because we have paths where caller already holds it. tested by jca ok bluhm jca
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/08/08 16:07:25 Modified files: sys/kern : uipc_socket.c Log message: Merge SO_BINDANY cases from both switch blocks within sosetopt(). This time SO_LINGER case is separated, so there is no reason for dedicated switch block. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/08/08 16:06:27 Modified files: sys/kern : uipc_socket.c Log message: Merge SO_SND* with corresponding SO_RCV* cases within sosetopt(). The only difference is the socket buffer. As bonus, in the future solock() will be easily replaced by sblock() instead pushing it down to each SO_SND* and SO_RCV* case. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/08/03 03:49:09 Modified files: sys/kern : uipc_socket.c uipc_syscalls.c sys/net: bfd.c if_vxlan.c if_wg.c sys/nfs: krpc_subr.c nfs_socket.c nfs_syscalls.c Log message: Move solock() down to sosetopt(). A part of standalone sblock() work. This movement required because buffers related SO_SND* and SO_RCV* socket options should be protected with sblock(). However, standalone sblock() has different lock order with solock() and `so_snd' and `so_rcv' buffers. At least sblock() for `so_snd' buffer will always be taken before solock() in the sosend() path. The (*pr_ctloutput)() call was removed from the SOL_SOCKET level 'else' branch. Except the SO_RTABLE case where it handled in the special way, this is null op call. For SO_SND* and SO_RCV* cases solock() will be replaced by sblock() in the future. Feedback from bluhm Tested by bluhm naddy ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/07/28 03:33:16 Modified files: sys/net: rtsock.c Log message: Compare m_pullup(9) return value against NULL instead of 0.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/07/27 16:20:51 Modified files: sys/net: rtsock.c Log message: Fix routing message size check in route_output(). `rtm_hdrlen' type is u_short, so add sizeof(rtm->rtm_hdrlen) instead of 1 to its offset within rt_msghdr structure. ok claudio
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/07/22 08:30:39 Modified files: sys/kern : uipc_socket.c Log message: Add `sb_state' output to sobuf_print(). It contains SS_CANTSENDMORE, SS_ISSENDING, SS_CANTRCVMORE and SS_RCVATMARK bits. Also do `sb_flags' output as hex, it contains flags too. ok kn bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/07/12 10:10:45 Modified files: sys/net: bfd.c Log message: Fix solock()/sounlock() usage. This time solock() doesn't return value and sounlock() hasn't second parameter. Bi-directional Forwarding Detection is disabled by default, so it was forgotten when solock()/sounlock() were changed. Build test done with BFD option. ok phessler claudio
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/07/04 16:28:24 Modified files: sys/kern : uipc_socket.c uipc_socket2.c sys/sys: socketvar.h Log message: Introduce SBL_WAIT and SBL_NOINTR sbwait() flags. This refactoring is another step to make standalone socket buffers locking. sblock() uses M_WAITOK and M_NOWAIT flags passed as the third argument together with the SB_NOINTR flag on the `sb_flags' to control sleep behaviour. To perform uninterruptible acquisition, SB_NOINTR flag should be set before sblock() call. `sb_flags' modification requires to hold solock() around sblock()/sbunlock() that makes standalone call impossible. Also `sb_flags' modifications outside sblock()/sbunlock() makes uninterruptible acquisition code huge enough. This time only sorflush() does this (and forgets to restore SB_NOINTR flag, so shutdown(SHUT_RDWR) call permanently modifies socket locking behaviour) and this looks not the big problem. But with the standalone socket buffer locking it will be many such places, so this huge construction is unwanted. Introduce new SBL_NOINTR flag passed as third sblock() argument. The sblock() acquisition will be uninterruptible when existing SB_NOINTR flag is set on `sb_flags' or SBL_NOINTR was passed. The M_WAITOK and M_NOWAIT flags belongs to malloc(9). It has no M_NOINTR flag and there is no reason to introduce it. So for consistency reasons introduce new SBL_WAIT and use it together with SBL_NOINTR instead of M_WAITOK and M_NOINTR respectively. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/06/30 05:52:11 Modified files: sys/kern : uipc_socket.c Log message: Use "newcon" instead of "netlck" as identifier of the sleep reason while awaiting concurrent sonewconn() threads termination. ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/06/30 03:58:30 Modified files: sys/net: pf_if.c pf_ioctl.c pf_ruleset.c sys/sys: malloc.h Log message: Introduce M_PF type for pf(4) related memory allocations. Currently used M_TEMP and M_IFADDR types are unreasonable for that purpose. This dedicated statistics simplify the future pf(4) unlocking work by decreasing search area of possible memory leaks. ok bluhm sashan
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/06/27 15:02:13 Modified files: sys/net: if.c sys/sys: malloc.h Log message: Introduce M_IFGROUP type of memory allocation. M_TEMP is unreasonable for interface groups data allocations. ok kn claudio bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/06/14 08:30:08 Modified files: sys/netinet: ip_mroute.c sys/netinet6 : ip6_mroute.c Log message: Add missing kernel lock around (*if_ioctl)(). ok bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/06/12 15:19:54 Modified files: sys/net: if.c Log message: Move nd6_ifdetach() out of netlock. In this point, the interface is disconnected from everywhere. No need to hold netlock for dummy 'nd_ifinfo' release. Netlock is also not needed for TAILQ_EMPTY(>if_*hooks) assertions. ok kn bluhm
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/18 04:24:28 Modified files: sys/kern : init_sysent.c syscalls.c sys/sys: syscall.h syscallargs.h Log message: regen
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/18 04:23:19 Modified files: sys/kern : kern_sysctl.c syscalls.master uipc_domain.c Log message: Backout sysctl(2) unlocking. Lock order issue was triggered in UVM layer.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/18 03:59:44 Modified files: sys/kern : uipc_domain.c sys/netinet: in_proto.c ip_input.c sys/sys: protosw.h Log message: Revert ip_sysctl() unlocking. Lock order issue was triggered in UVM layer.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/16 14:09:27 Modified files: sys/kern : uipc_mbuf.c Log message: Always set maximum queue length to passed in the IFQCTL_MAXLEN case. This is not the fast path, so dropping mq->mq_maxlen check doesn't introduce any performance impact, but makes code MP consistent. Discussed with and ok from bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/16 13:44:55 Modified files: sys/sys: protosw.h Log message: Replace tab by space after #define in PR_* definitions. ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/16 13:36:00 Modified files: sys/kern : uipc_domain.c sys/netinet: in_proto.c ip_input.c sys/sys: protosw.h Log message: Introduce temporary PR_MPSYSCTL flag to mark (*pr_sysctl)() handler MP safe. We have may of them, so use flag instead of pushing kernel lock within. Unlock ip_sysctl(). Still take kernel lock within IPCTL_MRTSTATS case. It looks like `mrtstat' protection is inconsistent, so keep locking as it was. Since `mrtstat' are counters, it make sense to rework them into per CPU counters with separate diffs. Feedback and ok from bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/04 03:41:15 Modified files: sys/kern : init_sysent.c syscalls.c sys/sys: syscall.h syscallargs.h Log message: regen
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/05/04 03:40:36 Modified files: sys/kern : kern_sysctl.c syscalls.master uipc_domain.c Log message: Push kernel lock deep down to sys_sysctl(). At least network subset of sysctl(8) MIBs relies on netlock or another locks and doesn't require kernel lock, so unlock it. The protocols layer *_sysctl()s are left under kernel lock and will be sequentially unlocked later. ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/04/28 14:03:14 Modified files: sys/dev/dt : dt_prov_static.c sys/net: route.c sys/sys: refcnt.h Log message: Add rtentry refcnt type to dt(4). ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/04/27 08:41:09 Modified files: sys/net: route.c Log message: Remove kernel lock from rtfree(9). Route timers and route labels protected by corresponding mutexes. `ifa' uses references counting for protection. rt_mpls_clear() could be called lockless because this is the last reference of `rt'. ok bluhm@ kn@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/04/27 05:11:04 Modified files: sys/net: route.c Log message: Add `rttimer_mtx' to the locking description. No functional changes.
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/04/26 13:54:35 Modified files: sys/net: if.c pf_ioctl.c route.c route.h Log message: Introduce `rtlabel_mtx' mutex(9) to protect route labels storage. This time kernel and net locks are held in various combination to protect it. We don't want to put kernel lock to all the places. Netlock also can't be used because rtfree(9) which calls rtlabel_unref() has unknown netlock state within. This new `rtlabel_mtx' mutex(9) protects `rt_labels' list and `label' entry dereference. Since we don't export 'rt_label' structure, keep this lock private to net/route.c. For this reason rtlabel_id2name() now copies label string to externally passed buffer instead of returning address of `rt_labels' list data. This is the way which rtlabel_id2sa() already works. ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/04/24 03:20:09 Modified files: sys/kern : uipc_socket.c Log message: Don't check `so_sp' within sofree(). The following isspliced() and issplicedback() already have this check. ok bluhm@
CVS: cvs.openbsd.org: src
CVSROOT:/cvs Module name:src Changes by: m...@cvs.openbsd.org2023/04/22 14:51:56 Modified files: sys/net: pfkeyv2.c sys/netinet: ip_spd.c Log message: Call pfkeyv2_sysctl_policydumper() with shared netlock. It performs read-olny access to netlock protected data, so the radix tree will not be modified during spd_table_walk() run. Also change netlock assertion within spd_table_add() and ipsec_delete_policy() to exclusive. These are correlating functions which modifies radix tree, so make us sure spd_table_walk() run with shared netlock is safe. Feedback and ok by bluhm@