CVS: cvs.openbsd.org: src

2024-05-20 Thread Vitaliy Makkoveev
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

2024-05-17 Thread Vitaliy Makkoveev
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

2024-05-17 Thread Vitaliy Makkoveev
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

2024-05-17 Thread Vitaliy Makkoveev
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

2024-05-07 Thread Vitaliy Makkoveev
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

2024-05-03 Thread Vitaliy Makkoveev
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

2024-05-02 Thread Vitaliy Makkoveev
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

2024-05-02 Thread Vitaliy Makkoveev
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

2024-05-02 Thread Vitaliy Makkoveev
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

2024-04-30 Thread Vitaliy Makkoveev
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

2024-04-15 Thread Vitaliy Makkoveev
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

2024-04-11 Thread Vitaliy Makkoveev
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

2024-04-11 Thread Vitaliy Makkoveev
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

2024-04-10 Thread Vitaliy Makkoveev
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

2024-04-02 Thread Vitaliy Makkoveev
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

2024-03-31 Thread Vitaliy Makkoveev
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

2024-03-31 Thread Vitaliy Makkoveev
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

2024-03-27 Thread Vitaliy Makkoveev
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

2024-03-27 Thread Vitaliy Makkoveev
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

2024-03-27 Thread Vitaliy Makkoveev
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

2024-03-27 Thread Vitaliy Makkoveev
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

2024-03-25 Thread Vitaliy Makkoveev
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

2024-03-25 Thread Vitaliy Makkoveev
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

2024-03-25 Thread Vitaliy Makkoveev
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

2024-03-22 Thread Vitaliy Makkoveev
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

2024-03-22 Thread Vitaliy Makkoveev
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

2024-03-17 Thread Vitaliy Makkoveev
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

2024-03-05 Thread Vitaliy Makkoveev
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

2024-02-12 Thread Vitaliy Makkoveev
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

2024-02-11 Thread Vitaliy Makkoveev
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

2024-02-11 Thread Vitaliy Makkoveev
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

2024-02-05 Thread Vitaliy Makkoveev
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

2024-02-05 Thread Vitaliy Makkoveev
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

2024-02-03 Thread Vitaliy Makkoveev
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

2024-01-26 Thread Vitaliy Makkoveev
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

2024-01-26 Thread Vitaliy Makkoveev
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

2024-01-23 Thread Vitaliy Makkoveev
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

2024-01-18 Thread Vitaliy Makkoveev
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

2024-01-18 Thread Vitaliy Makkoveev
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

2024-01-15 Thread Vitaliy Makkoveev
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

2024-01-03 Thread Vitaliy Makkoveev
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

2024-01-01 Thread Vitaliy Makkoveev
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

2023-12-23 Thread Vitaliy Makkoveev
> 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

2023-12-22 Thread Vitaliy Makkoveev
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

2023-12-19 Thread Vitaliy Makkoveev
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

2023-12-16 Thread Vitaliy Makkoveev
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

2023-12-16 Thread Vitaliy Makkoveev
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

2023-12-12 Thread Vitaliy Makkoveev
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

2023-12-11 Thread Vitaliy Makkoveev
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

2023-12-11 Thread Vitaliy Makkoveev
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

2023-12-08 Thread Vitaliy Makkoveev
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

2023-12-08 Thread Vitaliy Makkoveev
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

2023-12-03 Thread Vitaliy Makkoveev
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

2023-12-03 Thread Vitaliy Makkoveev
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

2023-12-01 Thread Vitaliy Makkoveev
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

2023-11-09 Thread Vitaliy Makkoveev
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

2023-10-25 Thread Vitaliy Makkoveev
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

2023-10-23 Thread Vitaliy Makkoveev
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

2023-10-05 Thread Vitaliy Makkoveev
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

2023-09-26 Thread Vitaliy Makkoveev
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

2023-09-26 Thread Vitaliy Makkoveev
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

2023-09-23 Thread Vitaliy Makkoveev
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

2023-09-22 Thread Vitaliy Makkoveev
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

2023-09-22 Thread Vitaliy Makkoveev
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

2023-09-13 Thread Vitaliy Makkoveev
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

2023-09-11 Thread Vitaliy Makkoveev
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

2023-09-11 Thread Vitaliy Makkoveev
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

2023-09-08 Thread Vitaliy Makkoveev
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

2023-09-01 Thread Vitaliy Makkoveev
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

2023-08-08 Thread Vitaliy Makkoveev
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

2023-08-08 Thread Vitaliy Makkoveev
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

2023-08-03 Thread Vitaliy Makkoveev
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

2023-07-28 Thread Vitaliy Makkoveev
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

2023-07-27 Thread Vitaliy Makkoveev
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

2023-07-22 Thread Vitaliy Makkoveev
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

2023-07-12 Thread Vitaliy Makkoveev
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

2023-07-04 Thread Vitaliy Makkoveev
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

2023-06-30 Thread Vitaliy Makkoveev
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

2023-06-30 Thread Vitaliy Makkoveev
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

2023-06-27 Thread Vitaliy Makkoveev
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

2023-06-14 Thread Vitaliy Makkoveev
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

2023-06-12 Thread Vitaliy Makkoveev
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

2023-05-18 Thread Vitaliy Makkoveev
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

2023-05-18 Thread Vitaliy Makkoveev
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

2023-05-18 Thread Vitaliy Makkoveev
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

2023-05-16 Thread Vitaliy Makkoveev
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

2023-05-16 Thread Vitaliy Makkoveev
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

2023-05-16 Thread Vitaliy Makkoveev
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

2023-05-04 Thread Vitaliy Makkoveev
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

2023-05-04 Thread Vitaliy Makkoveev
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

2023-04-28 Thread Vitaliy Makkoveev
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

2023-04-27 Thread Vitaliy Makkoveev
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

2023-04-27 Thread Vitaliy Makkoveev
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

2023-04-26 Thread Vitaliy Makkoveev
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

2023-04-24 Thread Vitaliy Makkoveev
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

2023-04-22 Thread Vitaliy Makkoveev
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@



CVS: cvs.openbsd.org: src

2023-04-20 Thread Vitaliy Makkoveev
CVSROOT:/cvs
Module name:src
Changes by: m...@cvs.openbsd.org2023/04/20 15:43:17

Modified files:
sys/net: rtsock.c 

Log message:
Call sysctl_source() with shared netlock. It performs read-only
access to netlock protected data.

ok kn@ bluhm@



CVS: cvs.openbsd.org: src

2023-04-18 Thread Vitaliy Makkoveev
CVSROOT:/cvs
Module name:src
Changes by: m...@cvs.openbsd.org2023/04/18 16:01:24

Modified files:
sys/dev/usb: if_umb.c 
sys/net: if.c if_var.h rtsock.c 

Log message:
Remove kernel lock from ifa_ifwithaddr() and ifa_ifwithdstaddr().
Netlock protects `if_list', `ifa_list' and returned `ifa' dereference,
so put netlock assertion within.

Please note, rtable_setsource() doesn't destroy data pointed by
`ar_source'. This is the `ifa_addr' data belongs to `ifa' and exclusive
netlock is required to destroy it. So the kernel lock is not required
within rt_setsource(). Take netlock by rt_setsource() caller to make
`ifa' dereference safe.

Suggestions and ok by bluhm@



CVS: cvs.openbsd.org: src

2023-04-18 Thread Vitaliy Makkoveev
CVSROOT:/cvs
Module name:src
Changes by: m...@cvs.openbsd.org2023/04/18 16:00:19

Modified files:
sys/net: if.c if_var.h 

Log message:
Document `ifnetlist' locking.

We use both kernel and net lock for protect `ifnetlist'. This means we
do modification with both locks held, but for read-only access only one
lock required. Some places doing `ifnetlist' foreach loop are protected
by kernel lock and context switch can't be introduced there. This is the
exception, so "XXXSMP:" comment added.

Proposed and ok by bluhm@



CVS: cvs.openbsd.org: src

2023-04-18 Thread Vitaliy Makkoveev
CVSROOT:/cvs
Module name:src
Changes by: m...@cvs.openbsd.org2023/04/18 04:19:17

Modified files:
sys/net: art.h rtable.c 

Log message:
Rename 'art_root' structure member `source' to `ar_source' to be in
accordance with all other 'art_root' structure members.

Proposed by bluhm@

ok bluhm@ kn@



  1   2   3   4   5   >