route(8) example for "out of prefix" default gateway

2022-11-08 Thread Stuart Henderson
Seems some hosting providers have annoying "out of prefix"
default gateways whuch are painful to configure
(https://marc.info/?t=16678224225=1=2), should
we give a pointer in route(8)?

Index: route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.104
diff -u -p -r1.104 route.8
--- route.8 29 Jul 2022 18:28:32 -  1.104
+++ route.8 9 Nov 2022 07:29:59 -
@@ -596,6 +596,14 @@ Delete the
 route to the 192.168.5.0/24 network:
 .Pp
 .Dl # route delete -inet 192.168.5.0/24
+.Pp
+Add a static
+.Xr inet6 4
+route to a host which is on the vio0 interface that is outside the prefix
+configured on the interface, and use that host as a default gateway:
+.Pp
+.Dl # route add -inet6 2001:db8:efef::1 -cloning -link -iface vio0
+.Dl # route add -inet6 default 2001:db8:efef::1
 .Sh DIAGNOSTICS
 .Bl -diag
 .It "%s: gateway %s flags %x"



Re: xenstore.c: return error number

2022-11-08 Thread Masato Asou
From: Martin Pieuchot 
Date: Tue, 8 Nov 2022 11:12:43 +

> On 01/11/22(Tue) 15:26, Masato Asou wrote:
>> Hi,
>> 
>> Return error number instead of call panic().
> 
> Makes sense to me.  Do you know how this error can occur?  Is is a logic
> error or are we trusting values produced by a third party?

You can reproduce this error by writing 1025 bytes from Dom0 and
reading this from OpenBSD on the DomU as follows:

dom0$ sudo xenstore-write /vm/408d6e98-1b95-43c2-803b-de24ee205631/data 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789abcde

domu$ hostctl /vm/408d6e98-1b95-43c2-803b-de24ee205631/data

--
ASOU Masato

>> comment, ok?
>> --
>> ASOU Masato
>> 
>> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
>> index 1e4f15d30eb..dc89ba0fa6d 100644
>> --- a/sys/dev/pv/xenstore.c
>> +++ b/sys/dev/pv/xenstore.c
>> @@ -118,6 +118,7 @@ struct xs_msg {
>>  struct xs_msghdr xsm_hdr;
>>  uint32_t xsm_read;
>>  uint32_t xsm_dlen;
>> +int  xsm_error;
>>  uint8_t *xsm_data;
>>  TAILQ_ENTRY(xs_msg)  xsm_link;
>>  };
>> @@ -566,9 +567,7 @@ xs_intr(void *arg)
>>  }
>>  
>>  if (xsm->xsm_hdr.xmh_len > xsm->xsm_dlen)
>> -panic("message too large: %d vs %d for type %d, rid %u",
>> -xsm->xsm_hdr.xmh_len, xsm->xsm_dlen, xsm->xsm_hdr.xmh_type,
>> -xsm->xsm_hdr.xmh_rid);
>> +xsm->xsm_error = EMSGSIZE;
>>  
>>  len = MIN(xsm->xsm_hdr.xmh_len - xsm->xsm_read, avail);
>>  if (len) {
>> @@ -800,7 +799,9 @@ xs_cmd(struct xs_transaction *xst, int cmd, const char 
>> *path,
>>  error = xs_geterror(xsm);
>>  DPRINTF("%s: xenstore request %d \"%s\" error %s\n",
>>  xs->xs_sc->sc_dev.dv_xname, cmd, path, xsm->xsm_data);
>> -} else if (mode == READ) {
>> +} else if (xsm->xsm_error != 0)
>> +error = xsm->xsm_error;
>> +else if (mode == READ) {
>>  KASSERT(iov && iov_cnt);
>>  error = xs_parse(xst, xsm, iov, iov_cnt);
>>  }
>> 


pause.3: Use Fn

2022-11-08 Thread Josiah Frentsos
Index: pause.3
===
RCS file: /cvs/src/lib/libc/gen/pause.3,v
retrieving revision 1.15
diff -u -p -r1.15 pause.3
--- pause.3 2 Aug 2022 01:23:23 -   1.15
+++ pause.3 8 Nov 2022 21:22:46 -
@@ -39,7 +39,8 @@
 .Fn pause void
 .Sh DESCRIPTION
 .Bf -symbolic
-Pause is made obsolete by
+.Fn pause
+is made obsolete by
 .Xr sigsuspend 2 .
 .Ef
 .Pp



Re: ssh-keygen(1): by default generate ed25519 key (instead of rsa)

2022-11-08 Thread Darren Tucker
On Tue, 8 Nov 2022 at 14:23, Joerg Sonnenberger  wrote:
> Am Tue, Nov 08, 2022 at 01:23:52PM +1100 schrieb Darren Tucker:
[...]
> > Not quite: the default value for IdentityFile has RSA before ED25519.
[...]
> I tried that first and it picked up id_ed25519 from the agent, even if
> both keys are accepted by the server.

It prefers keys present in the agent as those don't require entering a
passphrase.  It'll also prefer keys explicitly specified by the user
on the command line since that demonstrates user intent.  And the
behaviour is also modified by IdentitiesOnly.

> I guess that makes the answer a case of "it's complicated".

It is.  And IdentityFile works differently to most other options (it's
cumulative, not first-match) which was probably a mistake, but we're
kind of stuck with it.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: libcrypto: fix leak in BN_mpi2bn()

2022-11-08 Thread Theo Buehler
On Tue, Nov 08, 2022 at 11:06:43AM -0700, Todd C. Miller wrote:
> On Tue, 08 Nov 2022 18:33:48 +0100, Tobias Heider wrote:
> 
> > If ain == NULL then a points to newly malloced memory which should be
> > freed when BN_bin2bn() fails.
> 
> We don't have an "ain" function argument in LibreSSL so you will
> need a larger diff.  Perhaps something like this (untested).

ok tb



ypbind segmentation fault

2022-11-08 Thread Aleksej
There is a segmentation fault that occurs in ypbind when domainname is 
not set


Index: usr.sbin/ypbind/ypbind.c
===
RCS file: /cvs/src/usr.sbin/ypbind/ypbind.c,v
retrieving revision 1.76
diff -u -r1.76 ypbind.c
--- usr.sbin/ypbind/ypbind.c17 Jul 2022 03:12:20 -  1.76
+++ usr.sbin/ypbind/ypbind.c8 Nov 2022 11:04:29 -
@@ -345,7 +345,7 @@
struct dirent *dent;
 
 	yp_get_default_domain();

-   if (domain[0] == '\0') {
+   if (domain == NULL) {
fprintf(stderr, "domainname not set. Aborting.\n");
exit(1);
}



Re: libcrypto: fix leak in x509_name_ex_d2i()

2022-11-08 Thread Joel Sing
On 22-11-08 18:48:44, Tobias Heider wrote:
> nm.a is initialized to NULL until it gets alloced by x509_name_ex_new().
> The following 'goto err' should free nm.a before returning.
> 
> ok?

Unless I'm missing something, I do not believe this is correct -
nm is a union and nm.a is the same pointer as nm.x - nm.x is already
freed via X509_NAME_free(), which would make this a double free.

> Index: asn1/x_name.c
> ===
> RCS file: /cvs/src/lib/libcrypto/asn1/x_name.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 x_name.c
> --- asn1/x_name.c 25 Dec 2021 13:17:48 -  1.37
> +++ asn1/x_name.c 8 Nov 2022 17:45:08 -
> @@ -340,6 +340,7 @@ x509_name_ex_d2i(ASN1_VALUE **val, const
>   err:
>   if (nm.x != NULL)
>   X509_NAME_free(nm.x);
> + free(nm.a);
>   ASN1error(ERR_R_NESTED_ASN1_ERROR);
>   return 0;
>  }



Document global interface group list locking

2022-11-08 Thread Klemens Nanni
The per-interface group list is protected by the net lock and already
documented as such.

The global interface group list `ifg_head' is also protected by the net
lock and all access to it (all within if.c) take it accordingly.

OK?
---
 sys/net/if.c | 3 ++-
 sys/net/if_var.h | 8 
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index ad7d8ea5956..6a0ac9ee77f 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -226,7 +226,8 @@ voidif_idxmap_alloc(struct ifnet *);
 void   if_idxmap_insert(struct ifnet *);
 void   if_idxmap_remove(struct ifnet *);
 
-TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
+TAILQ_HEAD(, ifg_group) ifg_head =
+TAILQ_HEAD_INITIALIZER(ifg_head);  /* [N] list of interface groups */
 
 LIST_HEAD(, if_clone) if_cloners =
 LIST_HEAD_INITIALIZER(if_cloners); /* [I] list of clonable interfaces */
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 6272be882f4..275ea85bd2a 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -267,18 +267,18 @@ struct ifg_group {
u_intifg_refcnt;
caddr_t  ifg_pf_kif;
int  ifg_carp_demoted;
-   TAILQ_HEAD(, ifg_member) ifg_members;
-   TAILQ_ENTRY(ifg_group)   ifg_next;
+   TAILQ_HEAD(, ifg_member) ifg_members; /* [N] list of members per group 
*/
+   TAILQ_ENTRY(ifg_group)   ifg_next;/* [N] all groups are chained */
 };
 
 struct ifg_member {
-   TAILQ_ENTRY(ifg_member)  ifgm_next;
+   TAILQ_ENTRY(ifg_member)  ifgm_next; /* [N] all members are chained */
struct ifnet*ifgm_ifp;
 };
 
 struct ifg_list {
struct ifg_group*ifgl_group;
-   TAILQ_ENTRY(ifg_list)ifgl_next;
+   TAILQ_ENTRY(ifg_list)ifgl_next;  /* [N] all groups are chained */
 };
 
 #defineIFNET_SLOWTIMO  1   /* granularity is 1 second */
-- 
2.38.1



Re: libcrypto potential memory leak in OBJ_NAME_add error path

2022-11-08 Thread Theo Buehler
> Actually, it needs freeing only here. If lh_OBJ_NAME_error() returns 0,
> there was no error and a hash-entry containing NULL was replaced.

The below diff is ok if you want to commit.

PS: our initial diff causes a uaf caught by omalloc via:

$ openssl x509 -in /etc/ssl/cert.pem -out /dev/null

Index: objects/o_names.c
===
RCS file: /cvs/src/lib/libcrypto/objects/o_names.c,v
retrieving revision 1.22
diff -u -p -r1.22 o_names.c
--- objects/o_names.c   29 Jan 2017 17:49:23 -  1.22
+++ objects/o_names.c   8 Nov 2022 17:35:42 -
@@ -197,6 +197,7 @@ OBJ_NAME_add(const char *name, int type,
free(ret);
} else {
if (lh_OBJ_NAME_error(names_lh)) {
+   free(onp);
/* ERROR */
return (0);
}



Re: Document ifc_list immutability

2022-11-08 Thread Vitaliy Makkoveev
ok

> On 8 Nov 2022, at 21:38, Klemens Nanni  wrote:
> 
> On Tue, Nov 08, 2022 at 09:34:36PM +0300, Vitaliy Makkoveev wrote:
>>> On 8 Nov 2022, at 21:26, Klemens Nanni  wrote:
>>> 
>>> On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
> On 8 Nov 2022, at 21:08, Klemens Nanni  wrote:
> 
> Now properly.  How about a single comment block at the top instead of
> repeating it for every struct?
> 
> 
 
 You forgot to mark [I] `if_cloners’ within net/if.c.
>>> 
>>> Like this?  First locking commit in if.c.
>>> 
>>> 
>> 
>> Please, do this in consistency with other places:
>> 
>> LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
>>LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);/* [F] */
> 
> OK?
> 
> diff --git a/sys/net/if.c b/sys/net/if.c
> index 58a972b802c..a7b0b9bb8a3 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -228,8 +228,9 @@ void  if_idxmap_remove(struct ifnet *);
> 
> TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
> 
> -LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
> -int if_cloners_count;
> +LIST_HEAD(, if_clone) if_cloners =
> + LIST_HEAD_INITIALIZER(if_cloners); /* [I] list of clonable interfaces */
> +int if_cloners_count;/* [I] number of clonable interfaces */
> 
> struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("clonelk");
> 
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 28514a0bfcd..6272be882f4 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -73,6 +73,18 @@
>  * interfaces.  These routines live in the files if.c and route.c
>  */
> 
> +/*
> + *  Locks used to protect struct members in this file:
> + *   I   immutable after creation
> + *   d   protection left do the driver
> + *   c   only used in ioctl or routing socket contexts (kernel lock)
> + *   K   kernel lock
> + *   N   net lock
> + *
> + *  For SRP related structures that allow lock-free reads, the write lock
> + *  is indicated below.
> + */
> +
> struct rtentry;
> struct ifnet;
> struct task;
> @@ -82,7 +94,7 @@ struct cpumem;
>  * Structure describing a `cloning' interface.
>  */
> struct if_clone {
> - LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
> + LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
>   const char  *ifc_name;  /* name of device, e.g. `gif' */
>   size_t   ifc_namelen;   /* length of name */
> 
> @@ -99,17 +111,6 @@ struct if_clone {
>   .ifc_destroy= destroy,  
> \
> }
> 
> -/*
> - *  Locks used to protect struct members in this file:
> - *   I   immutable after creation
> - *   d   protection left do the driver
> - *   c   only used in ioctl or routing socket contexts (kernel lock)
> - *   K   kernel lock
> - *   N   net lock
> - *
> - *  For SRP related structures that allow lock-free reads, the write lock
> - *  is indicated below.
> - */
> /*
>  * Structure defining a queue for a network interface.
>  *



Re: Document ifc_list immutability

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 09:34:36PM +0300, Vitaliy Makkoveev wrote:
> > On 8 Nov 2022, at 21:26, Klemens Nanni  wrote:
> > 
> > On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
> >>> On 8 Nov 2022, at 21:08, Klemens Nanni  wrote:
> >>> 
> >>> Now properly.  How about a single comment block at the top instead of
> >>> repeating it for every struct?
> >>> 
> >>> 
> >> 
> >> You forgot to mark [I] `if_cloners’ within net/if.c.
> > 
> > Like this?  First locking commit in if.c.
> > 
> > 
> 
> Please, do this in consistency with other places:
> 
> LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
> LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);/* [F] */

OK?

diff --git a/sys/net/if.c b/sys/net/if.c
index 58a972b802c..a7b0b9bb8a3 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -228,8 +228,9 @@ voidif_idxmap_remove(struct ifnet *);
 
 TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
 
-LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
-int if_cloners_count;
+LIST_HEAD(, if_clone) if_cloners =
+   LIST_HEAD_INITIALIZER(if_cloners); /* [I] list of clonable interfaces */
+int if_cloners_count;  /* [I] number of clonable interfaces */
 
 struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("clonelk");
 
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..6272be882f4 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -73,6 +73,18 @@
  * interfaces.  These routines live in the files if.c and route.c
  */
 
+/*
+ *  Locks used to protect struct members in this file:
+ * I   immutable after creation
+ * d   protection left do the driver
+ * c   only used in ioctl or routing socket contexts (kernel lock)
+ * K   kernel lock
+ * N   net lock
+ *
+ *  For SRP related structures that allow lock-free reads, the write lock
+ *  is indicated below.
+ */
+
 struct rtentry;
 struct ifnet;
 struct task;
@@ -82,7 +94,7 @@ struct cpumem;
  * Structure describing a `cloning' interface.
  */
 struct if_clone {
-   LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
+   LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
const char  *ifc_name;  /* name of device, e.g. `gif' */
size_t   ifc_namelen;   /* length of name */
 
@@ -99,17 +111,6 @@ struct if_clone {
   .ifc_destroy = destroy,  \
 }
 
-/*
- *  Locks used to protect struct members in this file:
- * I   immutable after creation
- * d   protection left do the driver
- * c   only used in ioctl or routing socket contexts (kernel lock)
- * K   kernel lock
- * N   net lock
- *
- *  For SRP related structures that allow lock-free reads, the write lock
- *  is indicated below.
- */
 /*
  * Structure defining a queue for a network interface.
  *



Re: Document ifc_list immutability

2022-11-08 Thread Vitaliy Makkoveev
> On 8 Nov 2022, at 21:26, Klemens Nanni  wrote:
> 
> On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
>>> On 8 Nov 2022, at 21:08, Klemens Nanni  wrote:
>>> 
>>> Now properly.  How about a single comment block at the top instead of
>>> repeating it for every struct?
>>> 
>>> 
>> 
>> You forgot to mark [I] `if_cloners’ within net/if.c.
> 
> Like this?  First locking commit in if.c.
> 
> 

Please, do this in consistency with other places:

LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);/* [F] */


> diff --git a/sys/net/if.c b/sys/net/if.c
> index 58a972b802c..b7fc5258bd4 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -228,6 +228,7 @@ void  if_idxmap_remove(struct ifnet *);
> 
> TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
> 
> +/* [I] list of clonable interfaces */
> LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
> int if_cloners_count;
> 
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 28514a0bfcd..6272be882f4 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -73,6 +73,18 @@
>  * interfaces.  These routines live in the files if.c and route.c
>  */
> 
> +/*
> + *  Locks used to protect struct members in this file:
> + *   I   immutable after creation
> + *   d   protection left do the driver
> + *   c   only used in ioctl or routing socket contexts (kernel lock)
> + *   K   kernel lock
> + *   N   net lock
> + *
> + *  For SRP related structures that allow lock-free reads, the write lock
> + *  is indicated below.
> + */
> +
> struct rtentry;
> struct ifnet;
> struct task;
> @@ -82,7 +94,7 @@ struct cpumem;
>  * Structure describing a `cloning' interface.
>  */
> struct if_clone {
> - LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
> + LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
>   const char  *ifc_name;  /* name of device, e.g. `gif' */
>   size_t   ifc_namelen;   /* length of name */
> 
> @@ -99,17 +111,6 @@ struct if_clone {
>   .ifc_destroy= destroy,  
> \
> }
> 
> -/*
> - *  Locks used to protect struct members in this file:
> - *   I   immutable after creation
> - *   d   protection left do the driver
> - *   c   only used in ioctl or routing socket contexts (kernel lock)
> - *   K   kernel lock
> - *   N   net lock
> - *
> - *  For SRP related structures that allow lock-free reads, the write lock
> - *  is indicated below.
> - */
> /*
>  * Structure defining a queue for a network interface.
>  *
> 



Re: Document ifc_list immutability

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
> > On 8 Nov 2022, at 21:08, Klemens Nanni  wrote:
> > 
> > Now properly.  How about a single comment block at the top instead of
> > repeating it for every struct?
> > 
> > 
> 
> You forgot to mark [I] `if_cloners’ within net/if.c.

Like this?  First locking commit in if.c.


diff --git a/sys/net/if.c b/sys/net/if.c
index 58a972b802c..b7fc5258bd4 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -228,6 +228,7 @@ voidif_idxmap_remove(struct ifnet *);
 
 TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
 
+/* [I] list of clonable interfaces */
 LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
 int if_cloners_count;
 
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..6272be882f4 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -73,6 +73,18 @@
  * interfaces.  These routines live in the files if.c and route.c
  */
 
+/*
+ *  Locks used to protect struct members in this file:
+ * I   immutable after creation
+ * d   protection left do the driver
+ * c   only used in ioctl or routing socket contexts (kernel lock)
+ * K   kernel lock
+ * N   net lock
+ *
+ *  For SRP related structures that allow lock-free reads, the write lock
+ *  is indicated below.
+ */
+
 struct rtentry;
 struct ifnet;
 struct task;
@@ -82,7 +94,7 @@ struct cpumem;
  * Structure describing a `cloning' interface.
  */
 struct if_clone {
-   LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
+   LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
const char  *ifc_name;  /* name of device, e.g. `gif' */
size_t   ifc_namelen;   /* length of name */
 
@@ -99,17 +111,6 @@ struct if_clone {
   .ifc_destroy = destroy,  \
 }
 
-/*
- *  Locks used to protect struct members in this file:
- * I   immutable after creation
- * d   protection left do the driver
- * c   only used in ioctl or routing socket contexts (kernel lock)
- * K   kernel lock
- * N   net lock
- *
- *  For SRP related structures that allow lock-free reads, the write lock
- *  is indicated below.
- */
 /*
  * Structure defining a queue for a network interface.
  *



Re: Document ifc_list immutability

2022-11-08 Thread Vitaliy Makkoveev
> On 8 Nov 2022, at 21:08, Klemens Nanni  wrote:
> 
> Now properly.  How about a single comment block at the top instead of
> repeating it for every struct?
> 
> 

You forgot to mark [I] `if_cloners’ within net/if.c.

> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 28514a0bfcd..6272be882f4 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -73,6 +73,18 @@
>  * interfaces.  These routines live in the files if.c and route.c
>  */
> 
> +/*
> + *  Locks used to protect struct members in this file:
> + *   I   immutable after creation
> + *   d   protection left do the driver
> + *   c   only used in ioctl or routing socket contexts (kernel lock)
> + *   K   kernel lock
> + *   N   net lock
> + *
> + *  For SRP related structures that allow lock-free reads, the write lock
> + *  is indicated below.
> + */
> +
> struct rtentry;
> struct ifnet;
> struct task;
> @@ -82,7 +94,7 @@ struct cpumem;
>  * Structure describing a `cloning' interface.
>  */
> struct if_clone {
> - LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
> + LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
>   const char  *ifc_name;  /* name of device, e.g. `gif' */
>   size_t   ifc_namelen;   /* length of name */
> 
> @@ -99,17 +111,6 @@ struct if_clone {
>   .ifc_destroy= destroy,  
> \
> }
> 
> -/*
> - *  Locks used to protect struct members in this file:
> - *   I   immutable after creation
> - *   d   protection left do the driver
> - *   c   only used in ioctl or routing socket contexts (kernel lock)
> - *   K   kernel lock
> - *   N   net lock
> - *
> - *  For SRP related structures that allow lock-free reads, the write lock
> - *  is indicated below.
> - */
> /*
>  * Structure defining a queue for a network interface.
>  *
> 



Re: libcrypto: fix leak in BN_mpi2bn()

2022-11-08 Thread Tobias Heider
On Tue, Nov 08, 2022 at 11:06:43AM -0700, Todd C. Miller wrote:
> On Tue, 08 Nov 2022 18:33:48 +0100, Tobias Heider wrote:
> 
> > If ain == NULL then a points to newly malloced memory which should be
> > freed when BN_bin2bn() fails.
> 
> We don't have an "ain" function argument in LibreSSL so you will
> need a larger diff.  Perhaps something like this (untested).
> 
>  - todd

huh right, mixed them up. Updated diff looks ok to me.

> 
> Index: lib/libcrypto/bn/bn_mpi.c
> ===
> RCS file: /cvs/src/lib/libcrypto/bn/bn_mpi.c,v
> retrieving revision 1.8
> diff -u -p -u -r1.8 bn_mpi.c
> --- lib/libcrypto/bn/bn_mpi.c 29 Jan 2017 17:49:22 -  1.8
> +++ lib/libcrypto/bn/bn_mpi.c 8 Nov 2022 18:03:36 -
> @@ -92,8 +92,9 @@ BN_bn2mpi(const BIGNUM *a, unsigned char
>  }
>  
>  BIGNUM *
> -BN_mpi2bn(const unsigned char *d, int n, BIGNUM *a)
> +BN_mpi2bn(const unsigned char *d, int n, BIGNUM *ain)
>  {
> + BIGNUM *a = ain;
>   long len;
>   int neg = 0;
>  
> @@ -121,8 +122,11 @@ BN_mpi2bn(const unsigned char *d, int n,
>   d += 4;
>   if ((*d) & 0x80)
>   neg = 1;
> - if (BN_bin2bn(d, (int)len, a) == NULL)
> + if (BN_bin2bn(d, (int)len, a) == NULL) {
> + if (ain == NULL)
> + BN_free(a);
>   return (NULL);
> + }
>   a->neg = neg;
>   if (neg) {
>   BN_clear_bit(a, BN_num_bits(a) - 1);
> 



Document ifc_list immutability

2022-11-08 Thread Klemens Nanni
Now properly.  How about a single comment block at the top instead of
repeating it for every struct?


diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..6272be882f4 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -73,6 +73,18 @@
  * interfaces.  These routines live in the files if.c and route.c
  */
 
+/*
+ *  Locks used to protect struct members in this file:
+ * I   immutable after creation
+ * d   protection left do the driver
+ * c   only used in ioctl or routing socket contexts (kernel lock)
+ * K   kernel lock
+ * N   net lock
+ *
+ *  For SRP related structures that allow lock-free reads, the write lock
+ *  is indicated below.
+ */
+
 struct rtentry;
 struct ifnet;
 struct task;
@@ -82,7 +94,7 @@ struct cpumem;
  * Structure describing a `cloning' interface.
  */
 struct if_clone {
-   LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
+   LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
const char  *ifc_name;  /* name of device, e.g. `gif' */
size_t   ifc_namelen;   /* length of name */
 
@@ -99,17 +111,6 @@ struct if_clone {
   .ifc_destroy = destroy,  \
 }
 
-/*
- *  Locks used to protect struct members in this file:
- * I   immutable after creation
- * d   protection left do the driver
- * c   only used in ioctl or routing socket contexts (kernel lock)
- * K   kernel lock
- * N   net lock
- *
- *  For SRP related structures that allow lock-free reads, the write lock
- *  is indicated below.
- */
 /*
  * Structure defining a queue for a network interface.
  *



[no subject]

2022-11-08 Thread Todd C . Miller
On Tue, 08 Nov 2022 18:05:24 +, Klemens Nanni wrote:

> Subject: Document ifc_list immutability

Sure.  OK millert@

 - todd



Re: libcrypto: fix leak in BN_mpi2bn()

2022-11-08 Thread Todd C . Miller
On Tue, 08 Nov 2022 18:33:48 +0100, Tobias Heider wrote:

> If ain == NULL then a points to newly malloced memory which should be
> freed when BN_bin2bn() fails.

We don't have an "ain" function argument in LibreSSL so you will
need a larger diff.  Perhaps something like this (untested).

 - todd

Index: lib/libcrypto/bn/bn_mpi.c
===
RCS file: /cvs/src/lib/libcrypto/bn/bn_mpi.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 bn_mpi.c
--- lib/libcrypto/bn/bn_mpi.c   29 Jan 2017 17:49:22 -  1.8
+++ lib/libcrypto/bn/bn_mpi.c   8 Nov 2022 18:03:36 -
@@ -92,8 +92,9 @@ BN_bn2mpi(const BIGNUM *a, unsigned char
 }
 
 BIGNUM *
-BN_mpi2bn(const unsigned char *d, int n, BIGNUM *a)
+BN_mpi2bn(const unsigned char *d, int n, BIGNUM *ain)
 {
+   BIGNUM *a = ain;
long len;
int neg = 0;
 
@@ -121,8 +122,11 @@ BN_mpi2bn(const unsigned char *d, int n,
d += 4;
if ((*d) & 0x80)
neg = 1;
-   if (BN_bin2bn(d, (int)len, a) == NULL)
+   if (BN_bin2bn(d, (int)len, a) == NULL) {
+   if (ain == NULL)
+   BN_free(a);
return (NULL);
+   }
a->neg = neg;
if (neg) {
BN_clear_bit(a, BN_num_bits(a) - 1);



[no subject]

2022-11-08 Thread Klemens Nanni
Subject: Document ifc_list immutability

OK?

diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..a472c586f3c 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -78,11 +78,15 @@ struct ifnet;
 struct task;
 struct cpumem;
 
+/*
+ *  Locks used to protect struct members in this file:
+ * I   immutable after creation
+ */
 /*
  * Structure describing a `cloning' interface.
  */
 struct if_clone {
-   LIST_ENTRY(if_clone) ifc_list;  /* on list of cloners */
+   LIST_ENTRY(if_clone) ifc_list;  /* [I] on list of cloners */
const char  *ifc_name;  /* name of device, e.g. `gif' */
size_t   ifc_namelen;   /* length of name */
 



Re: riscv64: switch to clockintr(9)

2022-11-08 Thread Jeremie Courreges-Anglas
On Tue, Nov 08 2022, Mark Kettenis  wrote:
>> Date: Tue, 8 Nov 2022 13:06:51 +
>> From: Scott Cheloha 
>> 
>> On Mon, Nov 07, 2022 at 09:23:26PM +0100, Jeremie Courreges-Anglas wrote:
>> > On Sun, Nov 06 2022, Scott Cheloha  wrote:
>> > > This patch switches riscv64 to clockintr(9).
>> > >
>> > > jca@ has been testing it (on a SiFive board?).  It has survived two
>> > > parallel release builds and upgrades from the resulting bsd.rd.
>> > 
>> > I still get the same results on my HiFive Unmatched (produced by
>> > SiFive indeed).  The diff LGTM.
>> > 
>> > Looks like clock and stat evcounts get merged with this diff.  Not a big
>> > problem but probably not intended?
>> 
>> Whoops, good eye, forgot to mention that.
>> 
>> This change is intended.  With this patch, we can't keep a distinct
>> count of hardclock() and statclock() calls anymore because
>> clockintr_dispatch(9) handles the dispatch on behalf of the riscv64
>> code.  So the "stat" evcount goes away.
>> 
>> The "clock" evcount now represents a count of clock interrupts.
>> That's it.  One per riscv64 clock_intr() call.

ack.  Still ok jca@

> And I think that is fine.  Those are the actual interrupts.

Fine with me too!

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



libcrypto: fix leak in x509_name_ex_d2i()

2022-11-08 Thread Tobias Heider
nm.a is initialized to NULL until it gets alloced by x509_name_ex_new().
The following 'goto err' should free nm.a before returning.

ok?

Index: asn1/x_name.c
===
RCS file: /cvs/src/lib/libcrypto/asn1/x_name.c,v
retrieving revision 1.37
diff -u -p -r1.37 x_name.c
--- asn1/x_name.c   25 Dec 2021 13:17:48 -  1.37
+++ asn1/x_name.c   8 Nov 2022 17:45:08 -
@@ -340,6 +340,7 @@ x509_name_ex_d2i(ASN1_VALUE **val, const
  err:
if (nm.x != NULL)
X509_NAME_free(nm.x);
+   free(nm.a);
ASN1error(ERR_R_NESTED_ASN1_ERROR);
return 0;
 }



Re: push kernel lock inside ifioctl_get()

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 08:31:16PM +0300, Vitaliy Makkoveev wrote:
> No reason to keep kernel lock around if_clone_list() call.

Yes, that is why I will remove it in the very next commit.
This way there is one "move existing lock, make things clearer" and one
"actually remove a lock for a specific code path" change.



libcrypto: fix leak in BN_mpi2bn()

2022-11-08 Thread Tobias Heider
If ain == NULL then a points to newly malloced memory which should be
freed when BN_bin2bn() fails.

ok?

Index: bn/bn_mpi.c
===
RCS file: /cvs/src/lib/libcrypto/bn/bn_mpi.c,v
retrieving revision 1.8
diff -u -p -r1.8 bn_mpi.c
--- bn/bn_mpi.c 29 Jan 2017 17:49:22 -  1.8
+++ bn/bn_mpi.c 8 Nov 2022 17:30:33 -
@@ -121,8 +121,11 @@ BN_mpi2bn(const unsigned char *d, int n,
d += 4;
if ((*d) & 0x80)
neg = 1;
-   if (BN_bin2bn(d, (int)len, a) == NULL)
+   if (BN_bin2bn(d, (int)len, a) == NULL) {
+   if (ain == NULL)
+   BN_free(a);
return (NULL);
+   }
a->neg = neg;
if (neg) {
BN_clear_bit(a, BN_num_bits(a) - 1);



Re: vmm(4): remove locking in vm_intr_pending

2022-11-08 Thread Dave Voutila


Mike Larkin  writes:

> This lock/unlock around an atomic operation was causing delays delivering
> interupts into VMs. Pointed out by claudio@ when he ran md5 - in a VM
> and it became very sluggish.
>
> Debugging help from dlg and mpi, thanks.
>
> ok?
>

ok dv@. This is overly defensive and from the time I was chasing issues
on Intel hosts. Now that the VMCS is properly protected this can go.

Removing the lock here speeds up booting my Ubuntu iso 10x, too 8)

> -ml
>
>
> Index: arch/amd64/amd64/vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.326
> diff -u -p -a -u -r1.326 vmm.c
> --- arch/amd64/amd64/vmm.c7 Nov 2022 12:29:12 -   1.326
> +++ arch/amd64/amd64/vmm.c8 Nov 2022 15:29:10 -
> @@ -894,9 +894,7 @@ vm_intr_pending(struct vm_intr_params *v
>   goto out;
>   }
>
> - rw_enter_write(>vc_lock);
>   vcpu->vc_intr = vip->vip_intr;
> - rw_exit_write(>vc_lock);
>
>   refcnt_rele_wake(>vc_refcnt);
>  out:
> @@ -3526,7 +3524,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
>   vmx_setmsrbrw(vcpu, MSR_FSBASE);
>   vmx_setmsrbrw(vcpu, MSR_GSBASE);
>   vmx_setmsrbrw(vcpu, MSR_KERNELGSBASE);
> -
> +
>   vmx_setmsrbr(vcpu, MSR_MISC_ENABLE);
>   vmx_setmsrbr(vcpu, MSR_TSC);
>
> Index: arch/amd64/include/vmmvar.h
> ===
> RCS file: /cvs/src/sys/arch/amd64/include/vmmvar.h,v
> retrieving revision 1.81
> diff -u -p -a -u -r1.81 vmmvar.h
> --- arch/amd64/include/vmmvar.h   1 Sep 2022 22:01:40 -   1.81
> +++ arch/amd64/include/vmmvar.h   8 Nov 2022 15:29:10 -
> @@ -937,7 +937,7 @@ struct vcpu {
>   struct cpu_info *vc_last_pcpu;  /* [v] */
>   struct vm_exit vc_exit; /* [v] */
>
> - uint16_t vc_intr;   /* [v] */
> + uint16_t vc_intr;   /* [a] */
>   uint8_t vc_irqready;/* [v] */
>
>   uint8_t vc_fpuinited;   /* [v] */



Re: push kernel lock inside ifioctl_get()

2022-11-08 Thread Vitaliy Makkoveev
No reason to keep kernel lock around if_clone_list() call.

> On 8 Nov 2022, at 20:27, Klemens Nanni  wrote:
> 
> On Tue, Nov 08, 2022 at 08:04:16PM +0300, Vitaliy Makkoveev wrote:
>> The `if_cloners’ list is immutable. You don't need kernel lock
>> around if_clone_list() call.
>> 
>>> case SIOCIFGCLONERS:
>>> +   KERNEL_LOCK();
>>> error = if_clone_list((struct if_clonereq *)data);
>>> +   KERNEL_UNLOCK();
>>> return (error);
>> 
>> 
>> With this fix diff looks good for me.
> 
> This is going the be first unlocking diff, I just want to clearly
> separate setup churn (like this) from actual lock semantic changes.
> 



Re: push kernel lock inside ifioctl_get()

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 04:47:23PM +, Martin Pieuchot wrote:
> On 08/11/22(Tue) 15:28, Klemens Nanni wrote:
> > After this mechanical move, I can unlock the individual SIOCG* in there.
> 
> I'd suggest grabbing the KERNEL_LOCK() after NET_LOCK_SHARED().
> Otherwise you might spin for the first one then release it when going
> to sleep.

I can do that inside the first switch, but we must grab the net lock
before if_unit() which is called before grabbing the shared net lock.

I can shuffle this around if you really want, or I can simply move the
existing kernel lock further down and keep it in the same order just
like it is now already.

> 
> > OK?
> > 
> > Index: if.c
> > ===
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.667
> > diff -u -p -r1.667 if.c
> > --- if.c8 Nov 2022 15:20:24 -   1.667
> > +++ if.c8 Nov 2022 15:26:07 -
> > @@ -2426,33 +2426,43 @@ ifioctl_get(u_long cmd, caddr_t data)
> > size_t bytesdone;
> > const char *label;
> >  
> > -   KERNEL_LOCK();
> > -
> > switch(cmd) {
> > case SIOCGIFCONF:
> > +   KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = ifconf(data);
> > NET_UNLOCK_SHARED();
> > +   KERNEL_UNLOCK();
> > return (error);
> > case SIOCIFGCLONERS:
> > +   KERNEL_LOCK();
> > error = if_clone_list((struct if_clonereq *)data);
> > +   KERNEL_UNLOCK();
> > return (error);
> > case SIOCGIFGMEMB:
> > +   KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = if_getgroupmembers(data);
> > NET_UNLOCK_SHARED();
> > +   KERNEL_UNLOCK();
> > return (error);
> > case SIOCGIFGATTR:
> > +   KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = if_getgroupattribs(data);
> > NET_UNLOCK_SHARED();
> > +   KERNEL_UNLOCK();
> > return (error);
> > case SIOCGIFGLIST:
> > +   KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = if_getgrouplist(data);
> > NET_UNLOCK_SHARED();
> > +   KERNEL_UNLOCK();
> > return (error);
> > }
> > +
> > +   KERNEL_LOCK();
> >  
> > ifp = if_unit(ifr->ifr_name);
> > if (ifp == NULL) {
> > 
> 



Re: push kernel lock inside ifioctl_get()

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 08:04:16PM +0300, Vitaliy Makkoveev wrote:
> The `if_cloners’ list is immutable. You don't need kernel lock
> around if_clone_list() call.
> 
> > case SIOCIFGCLONERS:
> > +   KERNEL_LOCK();
> > error = if_clone_list((struct if_clonereq *)data);
> > +   KERNEL_UNLOCK();
> > return (error);
> 
> 
> With this fix diff looks good for me.

This is going the be first unlocking diff, I just want to clearly
separate setup churn (like this) from actual lock semantic changes.



libcrypto: leak in DSA_print()

2022-11-08 Thread Tobias Heider
Same diff as for RSA_print(). Old version leaks when EVP_PKEY_set1_DSA()
fails.

ok?

Index: dsa/dsa_prn.c
===
RCS file: /cvs/src/lib/libcrypto/dsa/dsa_prn.c,v
retrieving revision 1.6
diff -u -p -r1.6 dsa_prn.c
--- dsa/dsa_prn.c   29 Jan 2017 17:49:22 -  1.6
+++ dsa/dsa_prn.c   8 Nov 2022 17:22:08 -
@@ -98,12 +98,16 @@ int
 DSA_print(BIO *bp, const DSA *x, int off)
 {
EVP_PKEY *pk;
-   int ret;
+   int ret = 0;
+
+   if ((pk = EVP_PKEY_new()) == NULL)
+   goto out;
+
+   if (!EVP_PKEY_set1_DSA(pk, (DSA *)x))
+   goto out;
 
-   pk = EVP_PKEY_new();
-   if (!pk || !EVP_PKEY_set1_DSA(pk, (DSA *)x))
-   return 0;
ret = EVP_PKEY_print_private(bp, pk, off, NULL);
+ out:
EVP_PKEY_free(pk);
return ret;
 }



Re: push kernel lock inside ifioctl_get()

2022-11-08 Thread Vitaliy Makkoveev
The `if_cloners’ list is immutable. You don't need kernel lock
around if_clone_list() call.

>   case SIOCIFGCLONERS:
> + KERNEL_LOCK();
>   error = if_clone_list((struct if_clonereq *)data);
> + KERNEL_UNLOCK();
>   return (error);


With this fix diff looks good for me.

> On 8 Nov 2022, at 18:28, Klemens Nanni  wrote:
> 
> After this mechanical move, I can unlock the individual SIOCG* in there.
> 
> OK?
> 
> Index: if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.667
> diff -u -p -r1.667 if.c
> --- if.c  8 Nov 2022 15:20:24 -   1.667
> +++ if.c  8 Nov 2022 15:26:07 -
> @@ -2426,33 +2426,43 @@ ifioctl_get(u_long cmd, caddr_t data)
>   size_t bytesdone;
>   const char *label;
> 
> - KERNEL_LOCK();
> -
>   switch(cmd) {
>   case SIOCGIFCONF:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = ifconf(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCIFGCLONERS:
> + KERNEL_LOCK();
>   error = if_clone_list((struct if_clonereq *)data);
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCGIFGMEMB:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = if_getgroupmembers(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCGIFGATTR:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = if_getgroupattribs(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCGIFGLIST:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = if_getgrouplist(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   }
> +
> + KERNEL_LOCK();
> 
>   ifp = if_unit(ifr->ifr_name);
>   if (ifp == NULL) {
> 



Re: push kernel lock inside ifioctl_get()

2022-11-08 Thread Martin Pieuchot
On 08/11/22(Tue) 15:28, Klemens Nanni wrote:
> After this mechanical move, I can unlock the individual SIOCG* in there.

I'd suggest grabbing the KERNEL_LOCK() after NET_LOCK_SHARED().
Otherwise you might spin for the first one then release it when going
to sleep.

> OK?
> 
> Index: if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.667
> diff -u -p -r1.667 if.c
> --- if.c  8 Nov 2022 15:20:24 -   1.667
> +++ if.c  8 Nov 2022 15:26:07 -
> @@ -2426,33 +2426,43 @@ ifioctl_get(u_long cmd, caddr_t data)
>   size_t bytesdone;
>   const char *label;
>  
> - KERNEL_LOCK();
> -
>   switch(cmd) {
>   case SIOCGIFCONF:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = ifconf(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCIFGCLONERS:
> + KERNEL_LOCK();
>   error = if_clone_list((struct if_clonereq *)data);
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCGIFGMEMB:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = if_getgroupmembers(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCGIFGATTR:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = if_getgroupattribs(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   case SIOCGIFGLIST:
> + KERNEL_LOCK();
>   NET_LOCK_SHARED();
>   error = if_getgrouplist(data);
>   NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
>   return (error);
>   }
> +
> + KERNEL_LOCK();
>  
>   ifp = if_unit(ifr->ifr_name);
>   if (ifp == NULL) {
> 



Re: remove eeprom(8) sun4 leftovers

2022-11-08 Thread Mark Kettenis
> Date: Tue, 8 Nov 2022 15:49:19 +
> From: Miod Vallat 
> 
> The following diff removes the last mentions of the sun4 old style
> eeprom behaviour in the eeprom(8) manual page, as well as options
> specific to it.

sure; ok kettenis@

> Index: eeprom.8
> ===
> RCS file: /OpenBSD/src/usr.sbin/eeprom/eeprom.8,v
> retrieving revision 1.22
> diff -u -p -r1.22 eeprom.8
> --- eeprom.8  8 Jan 2020 14:45:36 -   1.22
> +++ eeprom.8  8 Nov 2022 15:42:12 -
> @@ -33,12 +33,11 @@
>  .Os
>  .Sh NAME
>  .Nm eeprom
> -.Nd display or modify contents of the EEPROM or OpenPROM
> +.Nd display or modify contents of the OpenPROM
>  .Sh SYNOPSIS
>  .Nm eeprom
> -.Op Fl cipv
> +.Op Fl pv
>  .Op Fl f Ar device
> -.Op Fl N Ar system
>  .Oo
>  .Ar field Ns Op = Ns Ar value
>  .Ar ...
> @@ -46,7 +45,7 @@
>  .Sh DESCRIPTION
>  .Nm eeprom
>  provides an interface for displaying and changing the contents of the
> -EEPROM or OpenPROM.
> +OpenPROM.
>  Without any arguments,
>  .Nm eeprom
>  will list all of the known fields and their corresponding values.
> @@ -55,145 +54,26 @@ When given the name of a specific field,
>  will display that value or set it if the field name is followed by
>  .Sq =
>  and a value.
> -Only the superuser may modify the contents of the EEPROM or OpenPROM.
> +Only the superuser may modify the contents of the OpenPROM.
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
>  .It Fl
>  Commands are taken from stdin and displayed on stdout.
> -.It Fl c
> -.Nm eeprom
> -will fix incorrect checksum values and exit.
> -This flag is quietly ignored on systems with an OpenPROM.
>  .It Fl f Ar device
> -On systems with an EEPROM, use
> -.Ar device
> -instead of the default
> -.Pa /dev/eeprom .
> -On systems with an OpenPROM, use
> +Use
>  .Ar device
>  instead of the default
>  .Pa /dev/openprom .
> -.It Fl i
> -If checksum values are incorrect,
> -.Nm eeprom
> -will ignore them and continue after displaying a warning.
> -This flag is quietly ignored on systems with an OpenPROM.
> -.It Fl N Ar system
> -Use the system image
> -.Ar system
> -instead of the default
> -.Pa /bsd .
>  .It Fl p
> -On systems with an OpenPROM, display the tree derived from it and exit.
> -This flag is quietly ignored on systems with an EEPROM.
> +Display the tree derived from the OpenPROM and exit.
>  .It Fl v
> -On systems with an OpenPROM, be verbose when setting a value.
> -Systems with an EEPROM are always verbose.
> +Be verbose when setting a value.
>  .El
>  .Sh FIELDS AND VALUES
> -The following fields and values are for systems with an EEPROM:
> -.Bl -tag -width "watchdog_reboot  "
> -.It Ar hwupdate
> -A valid date, such as
> -.Dq 7/12/95 .
> -The strings
> -.Dq today
> -and
> -.Dq now
> -are also acceptable.
> -.It Ar memsize
> -How much memory, in megabytes, is installed in the system.
> -.It Ar memtest
> -How much memory, in megabytes, is to be tested upon power-up.
> -.It Ar scrsize
> -The size of the screen.
> -Acceptable values are
> -.Dq 1024x1024 ,
> -.Dq 1152x900 ,
> -.Dq 1600x1280 ,
> -and
> -.Dq 1440x1440 .
> -.It Ar watchdog_reboot
> -If true, the system will reboot upon reset.
> -Otherwise, the system will fall into the monitor.
> -.It Ar default_boot
> -If true, the system will use the boot device stored in
> -.Ar bootdev .
> -.It Ar bootdev
> -Specifies the default boot device in the form cc(x,x,x), where
> -.Dq cc
> -is a combination of two letters such as
> -.Dq sd
> -or
> -.Dq le
> -and each
> -.Dq x
> -is a hexadecimal number between 0 and ff, less the prepending
> -.Dq 0x .
> -.It Ar kbdtype
> -This value is
> -.Dq 0
> -for all Sun keyboards.
> -.It Ar console
> -Specifies the console type.
> -Valid values are
> -.Dq b ,
> -.Dq ttya ,
> -.Dq ttyb ,
> -.Dq color ,
> -and
> -.Dq p4opt .
> -.It Ar keyclick
> -If true, the keys click annoyingly.
> -.It Ar diagdev
> -This is a string very similar to that used by
> -.Ar bootdev .
> -It specifies the default boot device when the diagnostic switch is
> -turned on.
> -.It Ar diagpath
> -A 40-character, NULL-terminated string specifying the kernel or stand-alone
> -program to load when the diagnostic switch is turned on.
> -.It Ar columns
> -An 8-bit integer specifying the number of columns on the console.
> -.It Ar rows
> -An 8-bit integer specifying the number of rows on the console.
> -.It Ar ttya_use_baud
> -Use the baud rate stored in
> -.Ar ttya_baud
> -instead of the default 9600.
> -.It Ar ttya_baud
> -A 16-bit integer specifying the baud rate to use on ttya.
> -.It Ar ttya_no_rtsdtr
> -If true, disables RTS/DTR.
> -.It Ar ttyb_use_baud
> -Similar to
> -.Ar ttya_use_baud ,
> -but for ttyb.
> -.It Ar ttyb_baud
> -Similar to
> -.Ar ttya_baud ,
> -but for ttyb.
> -.It Ar ttyb_no_rtsdtr
> -Similar to
> -.Ar ttya_no_rtsdtr ,
> -but for ttyb.
> -.It Ar banner
> -An 80-character, NULL-terminated string to use at power-up instead
> -of the default Sun banner.
> -.El
> -.Pp
> -Note that the

remove eeprom(8) sun4 leftovers

2022-11-08 Thread Miod Vallat
The following diff removes the last mentions of the sun4 old style
eeprom behaviour in the eeprom(8) manual page, as well as options
specific to it.

Index: eeprom.8
===
RCS file: /OpenBSD/src/usr.sbin/eeprom/eeprom.8,v
retrieving revision 1.22
diff -u -p -r1.22 eeprom.8
--- eeprom.88 Jan 2020 14:45:36 -   1.22
+++ eeprom.88 Nov 2022 15:42:12 -
@@ -33,12 +33,11 @@
 .Os
 .Sh NAME
 .Nm eeprom
-.Nd display or modify contents of the EEPROM or OpenPROM
+.Nd display or modify contents of the OpenPROM
 .Sh SYNOPSIS
 .Nm eeprom
-.Op Fl cipv
+.Op Fl pv
 .Op Fl f Ar device
-.Op Fl N Ar system
 .Oo
 .Ar field Ns Op = Ns Ar value
 .Ar ...
@@ -46,7 +45,7 @@
 .Sh DESCRIPTION
 .Nm eeprom
 provides an interface for displaying and changing the contents of the
-EEPROM or OpenPROM.
+OpenPROM.
 Without any arguments,
 .Nm eeprom
 will list all of the known fields and their corresponding values.
@@ -55,145 +54,26 @@ When given the name of a specific field,
 will display that value or set it if the field name is followed by
 .Sq =
 and a value.
-Only the superuser may modify the contents of the EEPROM or OpenPROM.
+Only the superuser may modify the contents of the OpenPROM.
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
 .It Fl
 Commands are taken from stdin and displayed on stdout.
-.It Fl c
-.Nm eeprom
-will fix incorrect checksum values and exit.
-This flag is quietly ignored on systems with an OpenPROM.
 .It Fl f Ar device
-On systems with an EEPROM, use
-.Ar device
-instead of the default
-.Pa /dev/eeprom .
-On systems with an OpenPROM, use
+Use
 .Ar device
 instead of the default
 .Pa /dev/openprom .
-.It Fl i
-If checksum values are incorrect,
-.Nm eeprom
-will ignore them and continue after displaying a warning.
-This flag is quietly ignored on systems with an OpenPROM.
-.It Fl N Ar system
-Use the system image
-.Ar system
-instead of the default
-.Pa /bsd .
 .It Fl p
-On systems with an OpenPROM, display the tree derived from it and exit.
-This flag is quietly ignored on systems with an EEPROM.
+Display the tree derived from the OpenPROM and exit.
 .It Fl v
-On systems with an OpenPROM, be verbose when setting a value.
-Systems with an EEPROM are always verbose.
+Be verbose when setting a value.
 .El
 .Sh FIELDS AND VALUES
-The following fields and values are for systems with an EEPROM:
-.Bl -tag -width "watchdog_reboot  "
-.It Ar hwupdate
-A valid date, such as
-.Dq 7/12/95 .
-The strings
-.Dq today
-and
-.Dq now
-are also acceptable.
-.It Ar memsize
-How much memory, in megabytes, is installed in the system.
-.It Ar memtest
-How much memory, in megabytes, is to be tested upon power-up.
-.It Ar scrsize
-The size of the screen.
-Acceptable values are
-.Dq 1024x1024 ,
-.Dq 1152x900 ,
-.Dq 1600x1280 ,
-and
-.Dq 1440x1440 .
-.It Ar watchdog_reboot
-If true, the system will reboot upon reset.
-Otherwise, the system will fall into the monitor.
-.It Ar default_boot
-If true, the system will use the boot device stored in
-.Ar bootdev .
-.It Ar bootdev
-Specifies the default boot device in the form cc(x,x,x), where
-.Dq cc
-is a combination of two letters such as
-.Dq sd
-or
-.Dq le
-and each
-.Dq x
-is a hexadecimal number between 0 and ff, less the prepending
-.Dq 0x .
-.It Ar kbdtype
-This value is
-.Dq 0
-for all Sun keyboards.
-.It Ar console
-Specifies the console type.
-Valid values are
-.Dq b ,
-.Dq ttya ,
-.Dq ttyb ,
-.Dq color ,
-and
-.Dq p4opt .
-.It Ar keyclick
-If true, the keys click annoyingly.
-.It Ar diagdev
-This is a string very similar to that used by
-.Ar bootdev .
-It specifies the default boot device when the diagnostic switch is
-turned on.
-.It Ar diagpath
-A 40-character, NULL-terminated string specifying the kernel or stand-alone
-program to load when the diagnostic switch is turned on.
-.It Ar columns
-An 8-bit integer specifying the number of columns on the console.
-.It Ar rows
-An 8-bit integer specifying the number of rows on the console.
-.It Ar ttya_use_baud
-Use the baud rate stored in
-.Ar ttya_baud
-instead of the default 9600.
-.It Ar ttya_baud
-A 16-bit integer specifying the baud rate to use on ttya.
-.It Ar ttya_no_rtsdtr
-If true, disables RTS/DTR.
-.It Ar ttyb_use_baud
-Similar to
-.Ar ttya_use_baud ,
-but for ttyb.
-.It Ar ttyb_baud
-Similar to
-.Ar ttya_baud ,
-but for ttyb.
-.It Ar ttyb_no_rtsdtr
-Similar to
-.Ar ttya_no_rtsdtr ,
-but for ttyb.
-.It Ar banner
-An 80-character, NULL-terminated string to use at power-up instead
-of the default Sun banner.
-.El
-.Pp
-Note that the
-.Ar secure ,
-.Ar bad_login ,
-and
-.Ar password
-fields are not currently supported.
-.Pp
 Since the OpenPROM is designed such that the field names are arbitrary,
 explaining them here is dubious.
-Below are field names and values that
-one is likely to see on a system with an OpenPROM.
+Below are field names and values that one is likely to see.
 NOTE: this list
 may be incomplete or incorrect due to differences 

vmm(4): remove locking in vm_intr_pending

2022-11-08 Thread Mike Larkin
This lock/unlock around an atomic operation was causing delays delivering
interupts into VMs. Pointed out by claudio@ when he ran md5 - in a VM
and it became very sluggish.

Debugging help from dlg and mpi, thanks.

ok?

-ml


Index: arch/amd64/amd64/vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.326
diff -u -p -a -u -r1.326 vmm.c
--- arch/amd64/amd64/vmm.c  7 Nov 2022 12:29:12 -   1.326
+++ arch/amd64/amd64/vmm.c  8 Nov 2022 15:29:10 -
@@ -894,9 +894,7 @@ vm_intr_pending(struct vm_intr_params *v
goto out;
}

-   rw_enter_write(>vc_lock);
vcpu->vc_intr = vip->vip_intr;
-   rw_exit_write(>vc_lock);

refcnt_rele_wake(>vc_refcnt);
 out:
@@ -3526,7 +3524,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
vmx_setmsrbrw(vcpu, MSR_FSBASE);
vmx_setmsrbrw(vcpu, MSR_GSBASE);
vmx_setmsrbrw(vcpu, MSR_KERNELGSBASE);
-
+
vmx_setmsrbr(vcpu, MSR_MISC_ENABLE);
vmx_setmsrbr(vcpu, MSR_TSC);

Index: arch/amd64/include/vmmvar.h
===
RCS file: /cvs/src/sys/arch/amd64/include/vmmvar.h,v
retrieving revision 1.81
diff -u -p -a -u -r1.81 vmmvar.h
--- arch/amd64/include/vmmvar.h 1 Sep 2022 22:01:40 -   1.81
+++ arch/amd64/include/vmmvar.h 8 Nov 2022 15:29:10 -
@@ -937,7 +937,7 @@ struct vcpu {
struct cpu_info *vc_last_pcpu;  /* [v] */
struct vm_exit vc_exit; /* [v] */

-   uint16_t vc_intr;   /* [v] */
+   uint16_t vc_intr;   /* [a] */
uint8_t vc_irqready;/* [v] */

uint8_t vc_fpuinited;   /* [v] */



push kernel lock inside ifioctl_get()

2022-11-08 Thread Klemens Nanni
After this mechanical move, I can unlock the individual SIOCG* in there.

OK?

Index: if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.667
diff -u -p -r1.667 if.c
--- if.c8 Nov 2022 15:20:24 -   1.667
+++ if.c8 Nov 2022 15:26:07 -
@@ -2426,33 +2426,43 @@ ifioctl_get(u_long cmd, caddr_t data)
size_t bytesdone;
const char *label;
 
-   KERNEL_LOCK();
-
switch(cmd) {
case SIOCGIFCONF:
+   KERNEL_LOCK();
NET_LOCK_SHARED();
error = ifconf(data);
NET_UNLOCK_SHARED();
+   KERNEL_UNLOCK();
return (error);
case SIOCIFGCLONERS:
+   KERNEL_LOCK();
error = if_clone_list((struct if_clonereq *)data);
+   KERNEL_UNLOCK();
return (error);
case SIOCGIFGMEMB:
+   KERNEL_LOCK();
NET_LOCK_SHARED();
error = if_getgroupmembers(data);
NET_UNLOCK_SHARED();
+   KERNEL_UNLOCK();
return (error);
case SIOCGIFGATTR:
+   KERNEL_LOCK();
NET_LOCK_SHARED();
error = if_getgroupattribs(data);
NET_UNLOCK_SHARED();
+   KERNEL_UNLOCK();
return (error);
case SIOCGIFGLIST:
+   KERNEL_LOCK();
NET_LOCK_SHARED();
error = if_getgrouplist(data);
NET_UNLOCK_SHARED();
+   KERNEL_UNLOCK();
return (error);
}
+
+   KERNEL_LOCK();
 
ifp = if_unit(ifr->ifr_name);
if (ifp == NULL) {



Re: tweak pfsync status output in ifconfig

2022-11-08 Thread Alexandr Nedvedicky
On Wed, Nov 09, 2022 at 12:09:50AM +1000, David Gwynne wrote:
> i'm hacking on pfsync(4) at the moment, and i wasted way too much time
> wondering how i broke the pfsync ioctls after i didn't the pfsync_status
> output. turns out if you don't have a sync interface set, it skips
> output.
> 
> i think it's useful to show that the sync interface is not set, so i
> came up with this.
> 
> an unconfigured pfsync interface looks like this with my diff:
> 
> pfsync0: flags=0<> mtu 1500
>   index 5 priority 0 llprio 3
>   pfsync: syncdev none maxupd 128 defer off
>   groups: carp pfsync

looks useful to me. I certainly don't object.

OK sashan



tweak pfsync status output in ifconfig

2022-11-08 Thread David Gwynne
i'm hacking on pfsync(4) at the moment, and i wasted way too much time
wondering how i broke the pfsync ioctls after i didn't the pfsync_status
output. turns out if you don't have a sync interface set, it skips
output.

i think it's useful to show that the sync interface is not set, so i
came up with this.

an unconfigured pfsync interface looks like this with my diff:

pfsync0: flags=0<> mtu 1500
index 5 priority 0 llprio 3
pfsync: syncdev none maxupd 128 defer off
groups: carp pfsync

Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -r1.457 ifconfig.c
--- ifconfig.c  26 Oct 2022 17:06:31 -  1.457
+++ ifconfig.c  8 Nov 2022 14:06:01 -
@@ -5086,22 +5086,23 @@ setpfsync_defer(const char *val, int d)
 void
 pfsync_status(void)
 {
-   struct pfsyncreq preq;
+   struct pfsyncreq pfsyncr;
+   const char *syncif = "none";
 
-   bzero(, sizeof(struct pfsyncreq));
-   ifr.ifr_data = (caddr_t)
+   bzero(, sizeof(pfsyncr));
+   ifr.ifr_data = (caddr_t)
 
if (ioctl(sock, SIOCGETPFSYNC, (caddr_t)) == -1)
return;
 
-   if (preq.pfsyncr_syncdev[0] != '\0') {
-   printf("\tpfsync: syncdev: %s ", preq.pfsyncr_syncdev);
-   if (preq.pfsyncr_syncpeer.s_addr != htonl(INADDR_PFSYNC_GROUP))
-   printf("syncpeer: %s ",
-   inet_ntoa(preq.pfsyncr_syncpeer));
-   printf("maxupd: %d ", preq.pfsyncr_maxupdates);
-   printf("defer: %s\n", preq.pfsyncr_defer ? "on" : "off");
-   }
+   if (pfsyncr.pfsyncr_syncdev[0] != '\0')
+   syncif = pfsyncr.pfsyncr_syncdev;
+
+   printf("\tpfsync: syncdev %s ", syncif);
+   if (pfsyncr.pfsyncr_syncpeer.s_addr != htonl(INADDR_PFSYNC_GROUP))
+   printf("syncpeer %s ", inet_ntoa(pfsyncr.pfsyncr_syncpeer));
+   printf("maxupd %d ", pfsyncr.pfsyncr_maxupdates);
+   printf("defer %s\n", pfsyncr.pfsyncr_defer ? "on" : "off");
 }
 
 void



Re: simplify handling of 'once' rules in pf(4)

2022-11-08 Thread Klemens Nanni
On Mon, Nov 07, 2022 at 06:53:47PM +0100, Alexandr Nedvedicky wrote:
> Hello,
> 
> updated diff. It buys suggestions from Klemens:
> 
> pf.conf(5) section which covers once rules reads as follows:
> 
>  onceCreates a one shot rule. The first matching packet marks rule as
>  expired.  The expired rule is never evaluated then.  pfctl(8)
>  does not report expired rules unless run in verbose mode ('-vv').
>  In verbose mode pfctl(8) appends  '# expired' to note the once
>  rule which got hit by packet other already.

Wording and markup can be improved, but I don't want to block this
change with boring churn, so I'm happy to do that later.

> 
> this how pfctl reports expired once rules now:
> 
>   pf# pfctl -a test/foo/bar -vv -sr 
>   @0 pass out proto tcp from any to any port = 80 flags S/SA once # 
> expired
> [ Evaluations: 25Packets: 5489  Bytes: 4936817 
> States: 0 ]
> [ Inserted: uid 0 pid 2768 State Creations: 1 ]
> 
> updated diff is below.

OK kn

> 
> thanks and
> regards
> sashan
> 
> 8<---8<---8<--8<
> diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
> index 6f39ad72384..fbe19747fc6 100644
> --- a/sbin/pfctl/pfctl_parser.c
> +++ b/sbin/pfctl/pfctl_parser.c
> @@ -1148,6 +1148,9 @@ print_rule(struct pf_rule *r, const char *anchor_call, 
> int opts)
>   printf(" ");
>   print_pool(>route, 0, 0, r->af, PF_POOL_ROUTE, verbose);
>   }
> +
> + if (r->rule_flag & PFRULE_EXPIRED)
> + printf(" # expired");
>  }
>  
>  void
> diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
> index 3e5a17acb95..b98b9f2fd9c 100644
> --- a/share/man/man5/pf.conf.5
> +++ b/share/man/man5/pf.conf.5
> @@ -661,10 +661,14 @@ When the rate is exceeded, all ICMP is blocked until 
> the rate falls below
>  100 per 10 seconds again.
>  .Pp
>  .It Cm once
> -Creates a one shot rule that will remove itself from an active ruleset after
> -the first match.
> -In case this is the only rule in the anchor, the anchor will be destroyed
> -automatically after the rule is matched.
> +Creates a one shot rule. The first matching packet marks rule as expired.
> +The expired rule is never evaluated then.
> +.Xr pfctl 8
> +does not report expired rules unless run in verbose mode ('-vv'). In verbose
> +mode
> +.Xr pfctl 8
> +appends  '# expired' to note the once rule which got hit by packet other
> +already.
>  .Pp
>  .It Cm probability Ar number Ns %
>  A probability attribute can be attached to a rule,
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index 2c6124e74f2..6295b4eb9d7 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -313,9 +313,6 @@ RB_GENERATE(pf_state_tree, pf_state_key, entry, 
> pf_state_compare_key);
>  RB_GENERATE(pf_state_tree_id, pf_state,
>  entry_id, pf_state_compare_id);
>  
> -SLIST_HEAD(pf_rule_gcl, pf_rule) pf_rule_gcl =
> - SLIST_HEAD_INITIALIZER(pf_rule_gcl);
> -
>  __inline int
>  pf_addr_compare(struct pf_addr *a, struct pf_addr *b, sa_family_t af)
>  {
> @@ -1478,23 +1475,6 @@ pf_state_import(const struct pfsync_state *sp, int 
> flags)
>  
>  /* END state table stuff */
>  
> -void
> -pf_purge_expired_rules(void)
> -{
> - struct pf_rule  *r;
> -
> - PF_ASSERT_LOCKED();
> -
> - if (SLIST_EMPTY(_rule_gcl))
> - return;
> -
> - while ((r = SLIST_FIRST(_rule_gcl)) != NULL) {
> - SLIST_REMOVE(_rule_gcl, r, pf_rule, gcle);
> - KASSERT(r->rule_flag & PFRULE_EXPIRED);
> - pf_purge_rule(r);
> - }
> -}
> -
>  void  pf_purge_states(void *);
>  struct task   pf_purge_states_task =
>TASK_INITIALIZER(pf_purge_states, NULL);
> @@ -1588,7 +1568,6 @@ pf_purge(void *null)
>   PF_LOCK();
>  
>   pf_purge_expired_src_nodes();
> - pf_purge_expired_rules();
>  
>   PF_UNLOCK();
>  
> @@ -3916,8 +3895,11 @@ pf_match_rule(struct pf_test_ctx *ctx, struct 
> pf_ruleset *ruleset)
>   struct pf_rule  *save_a;
>   struct pf_ruleset   *save_aruleset;
>  
> +retry:
>   r = TAILQ_FIRST(ruleset->rules.active.ptr);
>   while (r != NULL) {
> + PF_TEST_ATTRIB(r->rule_flag & PFRULE_EXPIRED,
> + TAILQ_NEXT(r, entries));
>   r->evaluations++;
>   PF_TEST_ATTRIB(
>   (pfi_kif_match(r->kif, ctx->pd->kif) == r->ifnot),
> @@ -4042,6 +4024,19 @@ pf_match_rule(struct pf_test_ctx *ctx, struct 
> pf_ruleset *ruleset)
>   if (r->tag)
>   ctx->tag = r->tag;
>   if (r->anchor == NULL) {
> +
> + if (r->rule_flag & PFRULE_ONCE) {
> + u_int32_t   rule_flag;
> +
> + rule_flag = r->rule_flag;
> + if (((rule_flag & PFRULE_EXPIRED) == 0) &&
> + 

push kernel lock into ifioctl_get()

2022-11-08 Thread Klemens Nanni
Another mechanical diff without semantic changes to avoid churn in
actual unlocking diffs.

OK?

Index: if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.666
diff -u -p -r1.666 if.c
--- if.c8 Nov 2022 11:25:01 -   1.666
+++ if.c8 Nov 2022 13:53:27 -
@@ -1979,9 +1979,7 @@ ifioctl(struct socket *so, u_long cmd, c
case SIOCGIFRDOMAIN:
case SIOCGIFGROUP:
case SIOCGIFLLPRIO:
-   KERNEL_LOCK();
error = ifioctl_get(cmd, data);
-   KERNEL_UNLOCK();
return (error);
}
 
@@ -2428,6 +2426,8 @@ ifioctl_get(u_long cmd, caddr_t data)
size_t bytesdone;
const char *label;
 
+   KERNEL_LOCK();
+
switch(cmd) {
case SIOCGIFCONF:
NET_LOCK_SHARED();
@@ -2455,8 +2455,10 @@ ifioctl_get(u_long cmd, caddr_t data)
}
 
ifp = if_unit(ifr->ifr_name);
-   if (ifp == NULL)
+   if (ifp == NULL) {
+   KERNEL_UNLOCK();
return (ENXIO);
+   }
 
NET_LOCK_SHARED();
 
@@ -2527,6 +2529,8 @@ ifioctl_get(u_long cmd, caddr_t data)
}
 
NET_UNLOCK_SHARED();
+
+   KERNEL_UNLOCK();
 
if_put(ifp);
 



Re: riscv64: switch to clockintr(9)

2022-11-08 Thread Mark Kettenis
> Date: Tue, 8 Nov 2022 13:06:51 +
> From: Scott Cheloha 
> 
> On Mon, Nov 07, 2022 at 09:23:26PM +0100, Jeremie Courreges-Anglas wrote:
> > On Sun, Nov 06 2022, Scott Cheloha  wrote:
> > > This patch switches riscv64 to clockintr(9).
> > >
> > > jca@ has been testing it (on a SiFive board?).  It has survived two
> > > parallel release builds and upgrades from the resulting bsd.rd.
> > 
> > I still get the same results on my HiFive Unmatched (produced by
> > SiFive indeed).  The diff LGTM.
> > 
> > Looks like clock and stat evcounts get merged with this diff.  Not a big
> > problem but probably not intended?
> 
> Whoops, good eye, forgot to mention that.
> 
> This change is intended.  With this patch, we can't keep a distinct
> count of hardclock() and statclock() calls anymore because
> clockintr_dispatch(9) handles the dispatch on behalf of the riscv64
> code.  So the "stat" evcount goes away.
> 
> The "clock" evcount now represents a count of clock interrupts.
> That's it.  One per riscv64 clock_intr() call.

And I think that is fine.  Those are the actual interrupts.

> > > It could use testing on another machine.  Are there other practical
> > > machines aside from SiFive?
> > 
> > There aren't that many vendors/machines that we do support:
> > 
> >   https://www.openbsd.org/riscv64.html
> 
> Okay, noted.
> 
> 



Re: Mark sched_yield(2) as NOLOCK

2022-11-08 Thread Mike Larkin
On Tue, Nov 08, 2022 at 01:14:02PM +, Martin Pieuchot wrote:
> Now that mmap/munmap/mprotect(2) are no longer creating contention it is
> possible to see that sched_yield(2) is one of the syscalls waiting for
> the KERNEL_LOCK() to be released.  However this is no longer necessary.
>
> Traversing `ps_threads' require either the KERNEL_LOCK() or the
> SCHED_LOCK() and we are holding both in this case.  So let's drop the
> requirement for the KERNEL_LOCK().
>
> ok?
>
> Index: kern/syscalls.master
> ===
> RCS file: /cvs/src/sys/kern/syscalls.master,v
> retrieving revision 1.235
> diff -u -p -r1.235 syscalls.master
> --- kern/syscalls.master  8 Nov 2022 11:05:57 -   1.235
> +++ kern/syscalls.master  8 Nov 2022 13:09:10 -
> @@ -531,7 +531,7 @@
>  #else
>  297  UNIMPL
>  #endif
> -298  STD { int sys_sched_yield(void); }
> +298  STD NOLOCK  { int sys_sched_yield(void); }
>  299  STD NOLOCK  { pid_t sys_getthrid(void); }
>  300  OBSOL   t32___thrsleep
>  301  STD NOLOCK  { int sys___thrwakeup(const volatile void *ident, \
>

Works here. This doesn't move the needle as much as the mmap unlock diff did
though :)



Mark sched_yield(2) as NOLOCK

2022-11-08 Thread Martin Pieuchot
Now that mmap/munmap/mprotect(2) are no longer creating contention it is
possible to see that sched_yield(2) is one of the syscalls waiting for
the KERNEL_LOCK() to be released.  However this is no longer necessary.

Traversing `ps_threads' require either the KERNEL_LOCK() or the
SCHED_LOCK() and we are holding both in this case.  So let's drop the
requirement for the KERNEL_LOCK().

ok?

Index: kern/syscalls.master
===
RCS file: /cvs/src/sys/kern/syscalls.master,v
retrieving revision 1.235
diff -u -p -r1.235 syscalls.master
--- kern/syscalls.master8 Nov 2022 11:05:57 -   1.235
+++ kern/syscalls.master8 Nov 2022 13:09:10 -
@@ -531,7 +531,7 @@
 #else
 297UNIMPL
 #endif
-298STD { int sys_sched_yield(void); }
+298STD NOLOCK  { int sys_sched_yield(void); }
 299STD NOLOCK  { pid_t sys_getthrid(void); }
 300OBSOL   t32___thrsleep
 301STD NOLOCK  { int sys___thrwakeup(const volatile void *ident, \



Re: riscv64: switch to clockintr(9)

2022-11-08 Thread Scott Cheloha
On Mon, Nov 07, 2022 at 09:23:26PM +0100, Jeremie Courreges-Anglas wrote:
> On Sun, Nov 06 2022, Scott Cheloha  wrote:
> > This patch switches riscv64 to clockintr(9).
> >
> > jca@ has been testing it (on a SiFive board?).  It has survived two
> > parallel release builds and upgrades from the resulting bsd.rd.
> 
> I still get the same results on my HiFive Unmatched (produced by
> SiFive indeed).  The diff LGTM.
> 
> Looks like clock and stat evcounts get merged with this diff.  Not a big
> problem but probably not intended?

Whoops, good eye, forgot to mention that.

This change is intended.  With this patch, we can't keep a distinct
count of hardclock() and statclock() calls anymore because
clockintr_dispatch(9) handles the dispatch on behalf of the riscv64
code.  So the "stat" evcount goes away.

The "clock" evcount now represents a count of clock interrupts.
That's it.  One per riscv64 clock_intr() call.

> > It could use testing on another machine.  Are there other practical
> > machines aside from SiFive?
> 
> There aren't that many vendors/machines that we do support:
> 
>   https://www.openbsd.org/riscv64.html

Okay, noted.



Re: tc_setclock: don't print warning when tc_windup() rejects RTC time

2022-11-08 Thread Mark Kettenis
> Date: Tue, 8 Nov 2022 11:59:17 +
> From: Scott Cheloha 
> 
> On some arm64 machines, the agtimer(4) ticks slowly enough that the
> tc_delta() doesn't overflow across brief suspends.  While working on
> arm64 suspend/resume, kettenis@ has been seeing warnings like this
> during resume:
> 
> tc_setclock: cannot rewind uptime to 307.253324249
> 
> The warning is misleading and should be removed.  The code is behaving
> as intended, but in a way I didn't anticipate when I added the warning
> a few years ago.
> 
> It might be useful print a warning in inittodr(9) during resume if the
> RTC time predates the system UTC suspend timestamp, but that's a
> distinct concern.
> 
> ok?

ok kettenis@

> Index: kern_tc.c
> ===
> RCS file: /cvs/src/sys/kern/kern_tc.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 kern_tc.c
> --- kern_tc.c 18 Sep 2022 20:47:09 -  1.78
> +++ kern_tc.c 8 Nov 2022 11:53:01 -
> @@ -552,7 +552,6 @@ void
>  tc_setclock(const struct timespec *ts)
>  {
>   struct bintime new_naptime, old_naptime, uptime, utc;
> - struct timespec tmp;
>   static int first = 1;
>  #ifndef SMALL_KERNEL
>   struct bintime elapsed;
> @@ -582,12 +581,6 @@ tc_setclock(const struct timespec *ts)
>   new_naptime = timehands->th_naptime;
>  
>   mtx_leave(_mtx);
> -
> - if (bintimecmp(_naptime, _naptime, ==)) {
> - BINTIME_TO_TIMESPEC(, );
> - printf("%s: cannot rewind uptime to %lld.%09ld\n",
> - __func__, (long long)tmp.tv_sec, tmp.tv_nsec);
> - }
>  
>  #ifndef SMALL_KERNEL
>   /* convert the bintime to ticks */
> 
> 



Re: libcrypto potential memory leak in OBJ_NAME_add error path

2022-11-08 Thread Theo Buehler
On Tue, Nov 08, 2022 at 01:33:47PM +0100, Moritz Buhl wrote:
> In case lh_OBJ_NAME_insert returns NULL due to a failed malloc, onp
> is leaked in OBJ_NAME_add.
> 
> Found by CodeChecker.
> 
> OK?
> mbuhl
> 
> Index: lib/libcrypto/objects/o_names.c
> ===
> RCS file: /cvs/src/lib/libcrypto/objects/o_names.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 o_names.c
> --- lib/libcrypto/objects/o_names.c   29 Jan 2017 17:49:23 -  1.22
> +++ lib/libcrypto/objects/o_names.c   8 Nov 2022 12:24:47 -
> @@ -196,6 +196,7 @@ OBJ_NAME_add(const char *name, int type,
>   }
>   free(ret);
>   } else {
> + free(onp);
>   if (lh_OBJ_NAME_error(names_lh)) {

Actually, it needs freeing only here. If lh_OBJ_NAME_error() returns 0,
there was no error and a hash-entry containing NULL was replaced.

>   /* ERROR */
>   return (0);



libcrypto potential memory leak in OBJ_NAME_add error path

2022-11-08 Thread Moritz Buhl
In case lh_OBJ_NAME_insert returns NULL due to a failed malloc, onp
is leaked in OBJ_NAME_add.

Found by CodeChecker.

OK?
mbuhl

Index: lib/libcrypto/objects/o_names.c
===
RCS file: /cvs/src/lib/libcrypto/objects/o_names.c,v
retrieving revision 1.22
diff -u -p -r1.22 o_names.c
--- lib/libcrypto/objects/o_names.c 29 Jan 2017 17:49:23 -  1.22
+++ lib/libcrypto/objects/o_names.c 8 Nov 2022 12:24:47 -
@@ -196,6 +196,7 @@ OBJ_NAME_add(const char *name, int type,
}
free(ret);
} else {
+   free(onp);
if (lh_OBJ_NAME_error(names_lh)) {
/* ERROR */
return (0);



Re: libcrypto: leak in RSA_print()

2022-11-08 Thread Theo Buehler
On Tue, Nov 08, 2022 at 01:01:17PM +0100, Tobias Heider wrote:
> If EVP_PKEY_set1_RSA() returns 0 we seem leak pk here.
> 
> ok?
> 
> Index: rsa/rsa_prn.c
> ===
> RCS file: /cvs/src/lib/libcrypto/rsa/rsa_prn.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 rsa_prn.c
> --- rsa/rsa_prn.c 29 Jan 2017 17:49:23 -  1.7
> +++ rsa/rsa_prn.c 8 Nov 2022 11:59:28 -
> @@ -85,8 +85,10 @@ RSA_print(BIO *bp, const RSA *x, int off
>   int ret;
>  
>   pk = EVP_PKEY_new();
> - if (!pk || !EVP_PKEY_set1_RSA(pk, (RSA *)x))
> + if (!pk || !EVP_PKEY_set1_RSA(pk, (RSA *)x)) {
> + EVP_PKEY_free(pk);
>   return 0;
> + }

I'd rewrite this as:

if ((pk = EVP_PKEY_new()) == NULL)
goto out;

if (!EVP_PKEY_set1_RSA(pk, (RSA *)x))
goto out;

>   ret = EVP_PKEY_print_private(bp, pk, off, NULL);

 out:

>   EVP_PKEY_free(pk);
>   return ret;
> 



Re: tc_setclock: don't print warning when tc_windup() rejects RTC time

2022-11-08 Thread Mike Larkin
On Tue, Nov 08, 2022 at 11:59:17AM +, Scott Cheloha wrote:
> On some arm64 machines, the agtimer(4) ticks slowly enough that the
> tc_delta() doesn't overflow across brief suspends.  While working on
> arm64 suspend/resume, kettenis@ has been seeing warnings like this
> during resume:
>
> tc_setclock: cannot rewind uptime to 307.253324249
>
> The warning is misleading and should be removed.  The code is behaving
> as intended, but in a way I didn't anticipate when I added the warning
> a few years ago.
>
> It might be useful print a warning in inittodr(9) during resume if the
> RTC time predates the system UTC suspend timestamp, but that's a
> distinct concern.
>
> ok?
>
> Index: kern_tc.c
> ===
> RCS file: /cvs/src/sys/kern/kern_tc.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 kern_tc.c
> --- kern_tc.c 18 Sep 2022 20:47:09 -  1.78
> +++ kern_tc.c 8 Nov 2022 11:53:01 -
> @@ -552,7 +552,6 @@ void
>  tc_setclock(const struct timespec *ts)
>  {
>   struct bintime new_naptime, old_naptime, uptime, utc;
> - struct timespec tmp;
>   static int first = 1;
>  #ifndef SMALL_KERNEL
>   struct bintime elapsed;
> @@ -582,12 +581,6 @@ tc_setclock(const struct timespec *ts)
>   new_naptime = timehands->th_naptime;
>
>   mtx_leave(_mtx);
> -
> - if (bintimecmp(_naptime, _naptime, ==)) {
> - BINTIME_TO_TIMESPEC(, );
> - printf("%s: cannot rewind uptime to %lld.%09ld\n",
> - __func__, (long long)tmp.tv_sec, tmp.tv_nsec);
> - }
>
>  #ifndef SMALL_KERNEL
>   /* convert the bintime to ticks */
>

ok mlarkin



libcrypto: leak in RSA_print()

2022-11-08 Thread Tobias Heider
If EVP_PKEY_set1_RSA() returns 0 we seem leak pk here.

ok?

Index: rsa/rsa_prn.c
===
RCS file: /cvs/src/lib/libcrypto/rsa/rsa_prn.c,v
retrieving revision 1.7
diff -u -p -r1.7 rsa_prn.c
--- rsa/rsa_prn.c   29 Jan 2017 17:49:23 -  1.7
+++ rsa/rsa_prn.c   8 Nov 2022 11:59:28 -
@@ -85,8 +85,10 @@ RSA_print(BIO *bp, const RSA *x, int off
int ret;
 
pk = EVP_PKEY_new();
-   if (!pk || !EVP_PKEY_set1_RSA(pk, (RSA *)x))
+   if (!pk || !EVP_PKEY_set1_RSA(pk, (RSA *)x)) {
+   EVP_PKEY_free(pk);
return 0;
+   }
ret = EVP_PKEY_print_private(bp, pk, off, NULL);
EVP_PKEY_free(pk);
return ret;



tc_setclock: don't print warning when tc_windup() rejects RTC time

2022-11-08 Thread Scott Cheloha
On some arm64 machines, the agtimer(4) ticks slowly enough that the
tc_delta() doesn't overflow across brief suspends.  While working on
arm64 suspend/resume, kettenis@ has been seeing warnings like this
during resume:

tc_setclock: cannot rewind uptime to 307.253324249

The warning is misleading and should be removed.  The code is behaving
as intended, but in a way I didn't anticipate when I added the warning
a few years ago.

It might be useful print a warning in inittodr(9) during resume if the
RTC time predates the system UTC suspend timestamp, but that's a
distinct concern.

ok?

Index: kern_tc.c
===
RCS file: /cvs/src/sys/kern/kern_tc.c,v
retrieving revision 1.78
diff -u -p -r1.78 kern_tc.c
--- kern_tc.c   18 Sep 2022 20:47:09 -  1.78
+++ kern_tc.c   8 Nov 2022 11:53:01 -
@@ -552,7 +552,6 @@ void
 tc_setclock(const struct timespec *ts)
 {
struct bintime new_naptime, old_naptime, uptime, utc;
-   struct timespec tmp;
static int first = 1;
 #ifndef SMALL_KERNEL
struct bintime elapsed;
@@ -582,12 +581,6 @@ tc_setclock(const struct timespec *ts)
new_naptime = timehands->th_naptime;
 
mtx_leave(_mtx);
-
-   if (bintimecmp(_naptime, _naptime, ==)) {
-   BINTIME_TO_TIMESPEC(, );
-   printf("%s: cannot rewind uptime to %lld.%09ld\n",
-   __func__, (long long)tmp.tv_sec, tmp.tv_nsec);
-   }
 
 #ifndef SMALL_KERNEL
/* convert the bintime to ticks */



Re: xenstore.c: return error number

2022-11-08 Thread Martin Pieuchot
On 01/11/22(Tue) 15:26, Masato Asou wrote:
> Hi,
> 
> Return error number instead of call panic().

Makes sense to me.  Do you know how this error can occur?  Is is a logic
error or are we trusting values produced by a third party?

> comment, ok?
> --
> ASOU Masato
> 
> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
> index 1e4f15d30eb..dc89ba0fa6d 100644
> --- a/sys/dev/pv/xenstore.c
> +++ b/sys/dev/pv/xenstore.c
> @@ -118,6 +118,7 @@ struct xs_msg {
>   struct xs_msghdr xsm_hdr;
>   uint32_t xsm_read;
>   uint32_t xsm_dlen;
> + int  xsm_error;
>   uint8_t *xsm_data;
>   TAILQ_ENTRY(xs_msg)  xsm_link;
>  };
> @@ -566,9 +567,7 @@ xs_intr(void *arg)
>   }
>  
>   if (xsm->xsm_hdr.xmh_len > xsm->xsm_dlen)
> - panic("message too large: %d vs %d for type %d, rid %u",
> - xsm->xsm_hdr.xmh_len, xsm->xsm_dlen, xsm->xsm_hdr.xmh_type,
> - xsm->xsm_hdr.xmh_rid);
> + xsm->xsm_error = EMSGSIZE;
>  
>   len = MIN(xsm->xsm_hdr.xmh_len - xsm->xsm_read, avail);
>   if (len) {
> @@ -800,7 +799,9 @@ xs_cmd(struct xs_transaction *xst, int cmd, const char 
> *path,
>   error = xs_geterror(xsm);
>   DPRINTF("%s: xenstore request %d \"%s\" error %s\n",
>   xs->xs_sc->sc_dev.dv_xname, cmd, path, xsm->xsm_data);
> - } else if (mode == READ) {
> + } else if (xsm->xsm_error != 0)
> + error = xsm->xsm_error;
> + else if (mode == READ) {
>   KASSERT(iov && iov_cnt);
>   error = xs_parse(xst, xsm, iov, iov_cnt);
>   }
> 



reorder_kernel: suport ro /usr like library_aslr does

2022-11-08 Thread Klemens Nanni
More read-only filesystems mean less fsck during boot after crashes.
Especially on crappy machines like the Pinebook Poop, I keep /usr
read-only and run with this diff so I still get a relinked kernel.

rc's reorder_libs() already does the same remount dance, but for
multiple directories/filesystems.

My diff works as expected with read-write and read-only /usr as well as
interrupting /usr/libexec/reorder_kernel runs with ^C, i.e. a failed run
will correctly mount /usr ro again if it was ro before the run.

Feedback? OK?

Index: reorder_kernel.sh
===
RCS file: /cvs/src/libexec/reorder_kernel/reorder_kernel.sh,v
retrieving revision 1.13
diff -u -p -r1.13 reorder_kernel.sh
--- reorder_kernel.sh   7 Nov 2022 15:55:56 -   1.13
+++ reorder_kernel.sh   8 Nov 2022 11:02:42 -
@@ -27,13 +27,32 @@ LOGFILE=$KERNEL_DIR/$KERNEL/relink.log
 PROGNAME=${0##*/}
 SHA256=/var/db/kernel.SHA256
 
-# Silently skip if on a NFS mounted filesystem.
-df -t nonfs $KERNEL_DIR >/dev/null 2>&1
+# Silently skip if on NFS, otherwise remount the filesystem read-write.
+DEV=$(df -t nonfs $KERNEL_DIR 2>/dev/null | awk 'NR == 2 { print $1 }')
+MP=$(mount -t ffs | grep ^$DEV)
+RO=false
+if [[ $MP == *read-only* ]]; then
+   MP=${MP%% *}
+   mount -u -w $MP
+   RO=true
+fi
+
+restore_mount() {
+   if $RO; then
+   # Close the logfile to unbusy $MP by switching back to console.
+   exec 1>/dev/console
+   exec 2>&1
+   mount -u -r $MP
+   fi
+}
 
 # Install trap handlers to inform about success or failure via syslog.
 ERRMSG='failed'
-trap 'trap - EXIT; logger -st $PROGNAME "$ERRMSG" >/dev/console 2>&1' ERR
-trap 'logger -t $PROGNAME "kernel relinking done"' EXIT
+trap 'trap - EXIT
+   logger -st $PROGNAME "$ERRMSG" 2>/dev/console
+   restore_mount' ERR
+trap 'logger -t $PROGNAME "kernel relinking done"
+   restore_mount' EXIT
 
 # Create kernel compile dir and redirect stdout/stderr to a logfile.
 mkdir -m 700 -p $KERNEL_DIR/$KERNEL



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Jesper Wallin
I concur, my 8-cores laptop (amd64) is happy with this patch.

Mostly using ungoogled-chromium, mutt, ncspot, xterm along with a few
vmd guests as dev machines.  No isuses to report.


Jesper Wallin

On Tue, Nov 08, 2022 at 09:55:36AM +, Stefan Hagen wrote:
> Martin Pieuchot wrote (2022-11-06 10:54 WET):
> > These 3 syscalls should now be ready to run w/o KERNEL_LOCK().  This
> > will reduce contention a lot.  I'd be happy to hear from test reports
> > on many architectures and possible workloads.
> 
> My two amd64 machines are happy with it for two days now. With several
> kernel builds and regular desktop use (browsers and such).
> 
> Best Regards,
> Stefan
> 



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Bob Beck
I have now been running it for two days, I *thought * had one hang a day ago, 
with chrome and local building churning away with me mashing on the editor.. 
but I’ve now been doing the same thing with witness on for a day and had no 
issues.  So I think whatever I might have seen is not reproducible or 
unrelated..

Ok beck@

> On Nov 8, 2022, at 10:30 AM, Martin Pieuchot  wrote:
> 
> On 08/11/22(Tue) 11:12, Mark Kettenis wrote:
>>> Date: Tue, 8 Nov 2022 10:32:14 +0100
>>> From: Christian Weisgerber 
>>> 
>>> Martin Pieuchot:
>>> 
 These 3 syscalls should now be ready to run w/o KERNEL_LOCK().  This
 will reduce contention a lot.  I'd be happy to hear from test reports
 on many architectures and possible workloads.
>>> 
>>> This survived a full amd64 package build.
>> 
>> \8/
>> 
>> I think that means it should be comitted.
> 
> I agree.  This has been tested on i386, riscv64, m88k, arm64, amd64 (of
> course) and sparc64.  I'm pretty confident.
> 



Re: mbufs growing in 7.2

2022-11-08 Thread Moritz Buhl
Hi Greg, Hi Joe,

dlg@ hinted to me that the ring might overwrite it's own starting
position with the current code.

Does this help?
mbuhl

Index: dev/pci/if_igc.c
===
RCS file: /cvs/src/sys/dev/pci/if_igc.c,v
retrieving revision 1.9
diff -u -p -r1.9 if_igc.c
--- dev/pci/if_igc.c2 Jun 2022 07:41:17 -   1.9
+++ dev/pci/if_igc.c8 Nov 2022 10:35:39 -
@@ -978,7 +978,7 @@ igc_start(struct ifqueue *ifq)
mask = sc->num_tx_desc - 1;
 
for (;;) {
-   if (free <= IGC_MAX_SCATTER) {
+   if (free <= IGC_MAX_SCATTER + 1) {
ifq_set_oactive(ifq);
break;
}
@@ -1005,6 +1005,7 @@ igc_start(struct ifqueue *ifq)
/* Consume the first descriptor */
prod++;
prod &= mask;
+   free--;
}
 
for (i = 0; i < map->dm_nsegs; i++) {



Re: rc(8): reorder_libs(): print names of relinked libraries

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 10:23:23AM +, Stuart Henderson wrote:
> On 2022/11/07 23:54, Theo de Raadt wrote:
> > Klemens Nanni  wrote:
> > 
> > > > I know this makes rc(8) a bit noisier but it really does improve my
> > > > (for want of a better term) "user experience" as I wait for my machine
> > > > to boot.
> > > 
> > > I like this and it doesn't add more **lines** to the boot log, but maybe
> > > print library names without versions to reduces noise?
> > 
> > Only if it is the short names.
> 
> No need for the .so really either. ld.so libc libcrypto is enough.
> 
> > But I am not sure people need to see this detail.  It just takes a bit
> > of time.  How does knowing what steps are being taken help...
> 
> Sometimes it's a bit of time, sometimes it's a _lot_ of time until
> people get a new computer or raid battery or something and it's less
> annoying if you can see _some_ progress.

Here's a simple diff that prints "ld.so" and "libc".
Not trying to take over Scott's diff, just helping out on shell bits.

Index: rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.564
diff -u -p -r1.564 rc
--- rc  29 Aug 2022 11:51:05 -  1.564
+++ rc  8 Nov 2022 10:32:32 -
@@ -193,7 +193,7 @@ reorder_libs() {
# Remount the (read-only) filesystems in _ro_list as read-write.
for _mp in $_ro_list; do
if ! mount -u -w $_mp; then
-   echo ' failed.'
+   echo '(failed).'
return
fi
done
@@ -215,6 +215,7 @@ reorder_libs() {
cd $_tmpdir
ar x $_liba
if [[ $_lib == ld.so ]]; then
+   echo -n " $_lib"
args="-g -x -e _dl_start \
--version-script=Symbols.map --shared -Bsymbolic \
--no-undefined"
@@ -225,6 +226,7 @@ reorder_libs() {
$_install /usr/libexec/ld.so /usr/libexec/ld.so.save
$_install ld.so.test $_lib_dir/ld.so
else
+   echo -n " ${_lib%%.*}"
cc -shared -o $_lib $(ls *.so | sort -R) $(<.ldadd)
[[ -s $_lib ]] && file $_lib | fgrep -q 'shared object'
LD_BIND_NOW=1 LD_LIBRARY_PATH=$_tmpdir awk 'BEGIN {exit 
0}'
@@ -243,9 +245,9 @@ reorder_libs() {
done
 
if $_error; then
-   echo ' failed.'
+   echo '(failed).'
else
-   echo ' done.'
+   echo '.'
fi
 }
 



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 10:30:47AM +, Martin Pieuchot wrote:
> On 08/11/22(Tue) 11:12, Mark Kettenis wrote:
> > I think that means it should be comitted.
> 
> I agree.  This has been tested on i386, riscv64, m88k, arm64, amd64 (of
> course) and sparc64.  I'm pretty confident.

OK kn



Re: rc(8): reorder_libs(): print names of relinked libraries

2022-11-08 Thread Theo de Raadt
Stuart Henderson  wrote:

> > But I am not sure people need to see this detail.  It just takes a bit
> > of time.  How does knowing what steps are being taken help...
> 
> Sometimes it's a bit of time, sometimes it's a _lot_ of time until
> people get a new computer or raid battery or something and it's less
> annoying if you can see _some_ progress.

Sigh.

Do we need a spinny thingy?



Re: rc(8): reorder_libs(): print names of relinked libraries

2022-11-08 Thread Theo de Raadt
Klemens Nanni  wrote:

> On Tue, Nov 08, 2022 at 10:23:23AM +, Stuart Henderson wrote:
> > On 2022/11/07 23:54, Theo de Raadt wrote:
> > > Klemens Nanni  wrote:
> > > 
> > > > > I know this makes rc(8) a bit noisier but it really does improve my
> > > > > (for want of a better term) "user experience" as I wait for my machine
> > > > > to boot.
> > > > 
> > > > I like this and it doesn't add more **lines** to the boot log, but maybe
> > > > print library names without versions to reduces noise?
> > > 
> > > Only if it is the short names.
> > 
> > No need for the .so really either. ld.so libc libcrypto is enough.
> 
> Yes, but that needs a little more shell tweaking as ${_lib%%.*} will
> turn "ld.so" into "ld", so I suggested a simple middle ground before
> blowing up the diff...

Sorry, but I insist on the shorters cute names.



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Martin Pieuchot
On 08/11/22(Tue) 11:12, Mark Kettenis wrote:
> > Date: Tue, 8 Nov 2022 10:32:14 +0100
> > From: Christian Weisgerber 
> > 
> > Martin Pieuchot:
> > 
> > > These 3 syscalls should now be ready to run w/o KERNEL_LOCK().  This
> > > will reduce contention a lot.  I'd be happy to hear from test reports
> > > on many architectures and possible workloads.
> > 
> > This survived a full amd64 package build.
> 
> \8/
> 
> I think that means it should be comitted.

I agree.  This has been tested on i386, riscv64, m88k, arm64, amd64 (of
course) and sparc64.  I'm pretty confident.



Re: rc(8): reorder_libs(): print names of relinked libraries

2022-11-08 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 10:23:23AM +, Stuart Henderson wrote:
> On 2022/11/07 23:54, Theo de Raadt wrote:
> > Klemens Nanni  wrote:
> > 
> > > > I know this makes rc(8) a bit noisier but it really does improve my
> > > > (for want of a better term) "user experience" as I wait for my machine
> > > > to boot.
> > > 
> > > I like this and it doesn't add more **lines** to the boot log, but maybe
> > > print library names without versions to reduces noise?
> > 
> > Only if it is the short names.
> 
> No need for the .so really either. ld.so libc libcrypto is enough.

Yes, but that needs a little more shell tweaking as ${_lib%%.*} will
turn "ld.so" into "ld", so I suggested a simple middle ground before
blowing up the diff...

> 
> > But I am not sure people need to see this detail.  It just takes a bit
> > of time.  How does knowing what steps are being taken help...
> 
> Sometimes it's a bit of time, sometimes it's a _lot_ of time until
> people get a new computer or raid battery or something and it's less
> annoying if you can see _some_ progress.

That and it just goes well in line with how rc.d scripts are printed.



Re: rc(8): reorder_libs(): print names of relinked libraries

2022-11-08 Thread Stuart Henderson
On 2022/11/07 23:54, Theo de Raadt wrote:
> Klemens Nanni  wrote:
> 
> > > I know this makes rc(8) a bit noisier but it really does improve my
> > > (for want of a better term) "user experience" as I wait for my machine
> > > to boot.
> > 
> > I like this and it doesn't add more **lines** to the boot log, but maybe
> > print library names without versions to reduces noise?
> 
> Only if it is the short names.

No need for the .so really either. ld.so libc libcrypto is enough.

> But I am not sure people need to see this detail.  It just takes a bit
> of time.  How does knowing what steps are being taken help...

Sometimes it's a bit of time, sometimes it's a _lot_ of time until
people get a new computer or raid battery or something and it's less
annoying if you can see _some_ progress.



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Mark Kettenis
> Date: Tue, 8 Nov 2022 10:32:14 +0100
> From: Christian Weisgerber 
> 
> Martin Pieuchot:
> 
> > These 3 syscalls should now be ready to run w/o KERNEL_LOCK().  This
> > will reduce contention a lot.  I'd be happy to hear from test reports
> > on many architectures and possible workloads.
> 
> This survived a full amd64 package build.

\8/

I think that means it should be comitted.



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Stefan Hagen
Martin Pieuchot wrote (2022-11-06 10:54 WET):
> These 3 syscalls should now be ready to run w/o KERNEL_LOCK().  This
> will reduce contention a lot.  I'd be happy to hear from test reports
> on many architectures and possible workloads.

My two amd64 machines are happy with it for two days now. With several
kernel builds and regular desktop use (browsers and such).

Best Regards,
Stefan



Re: Please test: unlock mprotect/mmap/munmap

2022-11-08 Thread Christian Weisgerber
Martin Pieuchot:

> These 3 syscalls should now be ready to run w/o KERNEL_LOCK().  This
> will reduce contention a lot.  I'd be happy to hear from test reports
> on many architectures and possible workloads.

This survived a full amd64 package build.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de