Re: sync libfido2 with upstream

2020-08-20 Thread Theo Buehler
On Fri, Aug 21, 2020 at 11:42:53AM +1000, Damien Miller wrote:
> On Mon, 17 Aug 2020, Damien Miller wrote:
> 
> > On Mon, 10 Aug 2020, Damien Miller wrote:
> > 
> > > Hi,
> > > 
> > > This syncs libfido2 with the current state of upstream. It includes
> > > a few new APIs that I want to use in OpenSSH to improve FIDO token
> > > support (require-PIN and fixing some corner-case bugs around multiple
> > > inserted tokens).
> > > 
> > > ok?
> > > 
> > > (major crank for ABI change)
> > 
> > So I pounced on the new API a bit too soon and before it stabilised.
> > There have been a couple more changes upstream that I need.
> > 
> > Sorry for the unneccessary churn.
> > 
> > ok?
> 
> I'd like to commit this. Ok?

ok tb



Re: sync libfido2 with upstream

2020-08-20 Thread Damien Miller
On Mon, 17 Aug 2020, Damien Miller wrote:

> On Mon, 10 Aug 2020, Damien Miller wrote:
> 
> > Hi,
> > 
> > This syncs libfido2 with the current state of upstream. It includes
> > a few new APIs that I want to use in OpenSSH to improve FIDO token
> > support (require-PIN and fixing some corner-case bugs around multiple
> > inserted tokens).
> > 
> > ok?
> > 
> > (major crank for ABI change)
> 
> So I pounced on the new API a bit too soon and before it stabilised.
> There have been a couple more changes upstream that I need.
> 
> Sorry for the unneccessary churn.
> 
> ok?

I'd like to commit this. Ok?

(to be clear - nothing in src/ yet uses the APIs affected by this change)

Index: README.openbsd
===
RCS file: /cvs/src/lib/libfido2/README.openbsd,v
retrieving revision 1.3
diff -u -p -r1.3 README.openbsd
--- README.openbsd  11 Aug 2020 08:44:53 -  1.3
+++ README.openbsd  21 Aug 2020 01:42:03 -
@@ -1,4 +1,4 @@
-This is an import of https://github.com/Yubico/libfido2 2fa20b889 (20200810)
+This is an import of https://github.com/Yubico/libfido2 46710ac06 (20200815)
 
 Local changes:
 
Index: shlib_version
===
RCS file: /cvs/src/lib/libfido2/shlib_version,v
retrieving revision 1.4
diff -u -p -r1.4 shlib_version
--- shlib_version   11 Aug 2020 08:44:53 -  1.4
+++ shlib_version   21 Aug 2020 01:42:03 -
@@ -1,2 +1,2 @@
-major=3
+major=4
 minor=0
Index: man/fido_dev_get_touch_begin.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_dev_get_touch_begin.3,v
retrieving revision 1.1
diff -u -p -r1.1 fido_dev_get_touch_begin.3
--- man/fido_dev_get_touch_begin.3  11 Aug 2020 08:44:53 -  1.1
+++ man/fido_dev_get_touch_begin.3  21 Aug 2020 01:42:03 -
@@ -14,7 +14,7 @@
 .Ft int
 .Fn fido_dev_get_touch_begin "fido_dev_t *dev"
 .Ft int
-.Fn fido_dev_get_touch_status "fido_dev_t *dev" "int *touched" "int *pin_set" 
"int ms"
+.Fn fido_dev_get_touch_status "fido_dev_t *dev" "int *touched" "int ms"
 .Sh DESCRIPTION
 The functions described in this page allow an application to
 asynchronously wait for touch on a FIDO authenticator.
Index: man/fido_dev_open.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_dev_open.3,v
retrieving revision 1.4
diff -u -p -r1.4 fido_dev_open.3
--- man/fido_dev_open.3 11 Aug 2020 08:44:53 -  1.4
+++ man/fido_dev_open.3 21 Aug 2020 01:42:03 -
@@ -16,6 +16,7 @@
 .Nm fido_dev_is_fido2 ,
 .Nm fido_dev_supports_cred_prot ,
 .Nm fido_dev_supports_pin ,
+.Nm fido_dev_has_pin ,
 .Nm fido_dev_protocol ,
 .Nm fido_dev_build ,
 .Nm fido_dev_flags ,
@@ -44,6 +45,8 @@
 .Fn fido_dev_supports_cred_prot "const fido_dev_t *dev"
 .Ft bool
 .Fn fido_dev_supports_pin "const fido_dev_t *dev"
+.Ft bool
+.Fn fido_dev_has_pin "const fido_dev_t *dev"
 .Ft uint8_t
 .Fn fido_dev_protocol "const fido_dev_t *dev"
 .Ft uint8_t
@@ -137,6 +140,14 @@ function returns
 if
 .Fa dev
 supports FIDO 2.0 Client PINs.
+.Pp
+The
+.Fn fido_dev_has_pin
+function returns
+.Dv true
+if
+.Fa dev
+has a FIDO 2.0 Client PIN set.
 .Pp
 The
 .Fn fido_dev_protocol
Index: src/dev.c
===
RCS file: /cvs/src/lib/libfido2/src/dev.c,v
retrieving revision 1.3
diff -u -p -r1.3 dev.c
--- src/dev.c   11 Aug 2020 08:44:53 -  1.3
+++ src/dev.c   21 Aug 2020 01:42:03 -
@@ -123,30 +123,27 @@ static void
 fido_dev_set_flags(fido_dev_t *dev, const fido_cbor_info_t *info)
 {
char * const*ptr;
+   const bool  *val;
size_t   len;
 
ptr = fido_cbor_info_extensions_ptr(info);
len = fido_cbor_info_extensions_len(info);
 
-   for (size_t i = 0; i < len; i++) {
-   if (strcmp(ptr[i], "credProtect") == 0) {
-   dev->flags |= FIDO_DEV_SUPPORTS_CRED_PROT;
-   }
-   }
+   for (size_t i = 0; i < len; i++)
+   if (strcmp(ptr[i], "credProtect") == 0)
+   dev->flags |= FIDO_DEV_CRED_PROT;
 
ptr = fido_cbor_info_options_name_ptr(info);
+   val = fido_cbor_info_options_value_ptr(info);
len = fido_cbor_info_options_len(info);
 
-   for (size_t i = 0; i < len; i++) {
-   /*
-* clientPin: PIN supported and set;
-* noclientPin: PIN supported but not set.
-*/
-   if (strcmp(ptr[i], "clientPin") == 0 ||
-   strcmp(ptr[i], "noclientPin") == 0) {
-   dev->flags |= FIDO_DEV_SUPPORTS_PIN;
+   for (size_t i = 0; i < len; i++)
+   if (strcmp(ptr[i], "clientPin") == 0) {
+   if (val[i] == true)
+   dev->flags |= FIDO_DEV_PIN_SET;
+   else
+ 

ubcmtp* at uhidev? ?

2020-08-20 Thread Marcus Glocker
Is ubcmtp(4) really attaching to uhidev(4)?  I only can see it
attaching to uhub(4).  Do I miss something?


Index: sys/dev/usb/ubcmtp.c
===
RCS file: /cvs/src/sys/dev/usb/ubcmtp.c,v
retrieving revision 1.21
diff -u -p -u -p -r1.21 ubcmtp.c
--- sys/dev/usb/ubcmtp.c31 Jul 2020 10:49:33 -  1.21
+++ sys/dev/usb/ubcmtp.c20 Aug 2020 21:14:31 -
@@ -47,7 +47,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -328,7 +327,6 @@ struct ubcmtp_softc {
 
struct ubcmtp_dev   *dev_type;
 
-   struct uhidev   sc_hdev;
struct usbd_device  *sc_udev;
struct device   *sc_wsmousedev;
 
Index: share/man/man4/ubcmtp.4
===
RCS file: /cvs/src/share/man/man4/ubcmtp.4,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 ubcmtp.4
--- share/man/man4/ubcmtp.4 25 May 2019 15:47:46 -  1.2
+++ share/man/man4/ubcmtp.4 20 Aug 2020 21:14:31 -
@@ -34,7 +34,7 @@
 .Nm ubcmtp
 .Nd Broadcom trackpad mouse
 .Sh SYNOPSIS
-.Cd "ubcmtp*  at uhidev?"
+.Cd "ubcmtp*  at uhub?"
 .Cd "wsmouse* at ubcmtp? mux 0"
 .Sh DESCRIPTION
 The
@@ -42,7 +42,7 @@ The
 driver provides support for Broadcom multi-touch trackpads found on
 newer Apple MacBook, MacBook Pro, and MacBook Air laptops.
 .Sh SEE ALSO
-.Xr uhidev 4 ,
+.Xr uhub 4 ,
 .Xr usb 4 ,
 .Xr wsmouse 4
 .Sh HISTORY
Index: share/man/man4/uhidev.4
===
RCS file: /cvs/src/share/man/man4/uhidev.4,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 uhidev.4
--- share/man/man4/uhidev.4 25 Aug 2018 20:31:31 -  1.10
+++ share/man/man4/uhidev.4 20 Aug 2020 21:14:31 -
@@ -36,7 +36,6 @@
 .Nd USB Human Interface Device support
 .Sh SYNOPSIS
 .Cd "uhidev*  at uhub?"
-.Cd "ubcmtp*  at uhidev?"
 .Cd "ucycom*  at uhidev?"
 .Cd "ugold*   at uhidev?"
 .Cd "uhid*at uhidev?"
@@ -68,7 +67,6 @@ kinds of devices and
 only dispatches data to them based on the report id.
 .Sh SEE ALSO
 .Xr intro 4 ,
-.Xr ubcmtp 4 ,
 .Xr ucycom 4 ,
 .Xr ugold 4 ,
 .Xr uhid 4 ,



make(1): anchors in :S/old_string/new_string/

2020-08-20 Thread Theo Buehler
gnezdo noticed that :S/old_string/new_string/ modifiers such as
:S/^sth/&/ and :S/sth$/&/ with an anchor in the old_string and an &
in the new_string don't work as documented (and expected) since they
replace & with old_string including the anchors.

This is because get_spatternarg() deals with skipping the anchors in
pattern->lhs only after having replaced any '&' in new_string with
pattern->lhs.  This happens in VarGetPattern if the last argument is
non-NULL when pattern->rhs is determined, i.e., before
common_get_patternarg() is done.

I think the fix should be as simple as the diff below.  I added a small
extension of a regress test for this.  Unpatched make fails, patched
make passes.

Simple test case that illustrates the problem:

$ cat > makefile
A= foo bar barr
B= ${A:S/^b/s&/}
C= ${A:S/r$/&/}
D= ${A:S/^bar$/&/}

all:
@echo "B= $B"
@echo "C= $C"
@echo "D= $D"
EOF
$ make
B= foo s^bar s^barr
C= foo bar$ barr$
D= foo ^bar$bar barr

I'm not entirely clear on why I get that result for D, but with the
patch below, I get the expected " foo barbarian barr"

Index: usr.bin/make/varmodifiers.c
===
RCS file: /var/cvs/src/usr.bin/make/varmodifiers.c,v
retrieving revision 1.47
diff -u -p -r1.47 varmodifiers.c
--- usr.bin/make/varmodifiers.c 10 Jul 2017 07:10:29 -  1.47
+++ usr.bin/make/varmodifiers.c 20 Aug 2020 16:03:54 -
@@ -1215,21 +1215,7 @@ get_patternarg(const char **p, SymTable 
 static void *
 get_spatternarg(const char **p, SymTable *ctxt, bool err, int endc)
 {
-   VarPattern *pattern;
-
-   pattern = common_get_patternarg(p, ctxt, err, endc, true);
-   if (pattern != NULL && pattern->leftLen > 0) {
-   if (pattern->lhs[pattern->leftLen-1] == '$') {
-   pattern->leftLen--;
-   pattern->flags |= VAR_MATCH_END;
-   }
-   if (pattern->lhs[0] == '^') {
-   pattern->lhs++;
-   pattern->leftLen--;
-   pattern->flags |= VAR_MATCH_START;
-   }
-   }
-   return pattern;
+   return common_get_patternarg(p, ctxt, err, endc, true);
 }
 
 static void
@@ -1304,6 +1290,17 @@ common_get_patternarg(const char **p, Sy
>leftLen, NULL);
pattern->lbuffer = pattern->lhs;
if (pattern->lhs != NULL) {
+   if (dosubst && pattern->leftLen > 0) {
+   if (pattern->lhs[pattern->leftLen-1] == '$') {
+   pattern->leftLen--;
+   pattern->flags |= VAR_MATCH_END;
+   }
+   if (pattern->lhs[0] == '^') {
+   pattern->lhs++;
+   pattern->leftLen--;
+   pattern->flags |= VAR_MATCH_START;
+   }
+   }
pattern->rhs = VarGetPattern(ctxt, err, , delim, delim,
>rightLen, dosubst ? pattern: NULL);
if (pattern->rhs != NULL) {
Index: regress/usr.bin/make/mk21
===
RCS file: /var/cvs/src/regress/usr.bin/make/mk21,v
retrieving revision 1.2
diff -u -p -r1.2 mk21
--- regress/usr.bin/make/mk21   7 Jul 2017 16:31:37 -   1.2
+++ regress/usr.bin/make/mk21   20 Aug 2020 16:20:31 -
@@ -12,7 +12,13 @@ X?=${TRUC:C@([^:/])/.+$@\1/@}
 Y?=${TRUC:S/^${X}//:S/^://}
 Z?=${TRUC:S/^${TRUC:C@([^:/])/.+$@\1/@}//:S/^://}
 
+A?=machin truc
+B?=${A:S/^/mot: &/}
+C?=${A:S/$/&: mot/}
+
 all:
+   @echo "B= $B"
+   @echo "C= $C"
@echo "S= $S"
@echo "T= $T"
@echo "X= $X"
Index: regress/usr.bin/make/t21.out
===
RCS file: /var/cvs/src/regress/usr.bin/make/t21.out,v
retrieving revision 1.2
diff -u -p -r1.2 t21.out
--- regress/usr.bin/make/t21.out7 Jul 2017 16:31:37 -   1.2
+++ regress/usr.bin/make/t21.out20 Aug 2020 16:20:31 -
@@ -1,3 +1,5 @@
+B= mot: machin mot: truc
+C= machin: mot truc: mot
 S= sourceforge/%SUBDIR%/
 T= sourceforge/%SUBDIR%/
 X= http://heanet.dl.sourceforge.net/



Re: pppoe: add sizes to free() calls

2020-08-20 Thread Vitaliy Makkoveev
ok mvs@

> On 20 Aug 2020, at 17:12, Klemens Nanni  wrote:
> 
> On Thu, Aug 20, 2020 at 03:33:17PM +0200, Klemens Nanni wrote:
>> These are straight forward as we either maintain a size variable all the
>> way or can reuse strlen() for free() just like it's done during malloc().
>> 
>> One exception is freeing the softc structure, which is fixed in size;
>> `ifconfig pppoe1 create; ifconfig pppoe1 destroy' exercises this code
>> path and does not blow up - as expected.
>> 
>> Running fine on a octeon vdsl2 router.
>> 
>> Feedback? OK?
> Sorry for the noise, Peter J. Philipp pointed out how I missed the +1
> byte to account for the NUL character which strlen(9) does not count.
> All corresponding malloc calls do strlen() + 1, so without it there's an
> off-by-one in the size.
> 
> In my setup I do not exercise the service and concentrator name paths,
> but playing with `ifconfig pppoe0 [[-]pppoeac foo] [[-]pppoesvc bar]'
> doesn't blow up on any of those diff versions, either.
> 
> Fixed diff.
> 
> Index: if_pppoe.c
> ===
> RCS file: /cvs/src/sys/net/if_pppoe.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 if_pppoe.c
> --- if_pppoe.c28 Jul 2020 09:52:32 -  1.70
> +++ if_pppoe.c20 Aug 2020 13:50:07 -
> @@ -257,15 +267,17 @@ pppoe_clone_destroy(struct ifnet *ifp)
>   if_detach(ifp);
> 
>   if (sc->sc_concentrator_name)
> - free(sc->sc_concentrator_name, M_DEVBUF, 0);
> + free(sc->sc_concentrator_name, M_DEVBUF,
> + strlen(sc->sc_concentrator_name) + 1);
>   if (sc->sc_service_name)
> - free(sc->sc_service_name, M_DEVBUF, 0);
> + free(sc->sc_service_name, M_DEVBUF,
> + strlen(sc->sc_service_name) + 1);
>   if (sc->sc_ac_cookie)
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
>   if (sc->sc_relay_sid)
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
> 
> - free(sc, M_DEVBUF, 0);
> + free(sc, M_DEVBUF, sizeof(*sc));
> 
>   return (0);
> }
> @@ -547,7 +559,8 @@ breakbreak:
>   }
>   if (ac_cookie) {
>   if (sc->sc_ac_cookie)
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF,
> + sc->sc_ac_cookie_len);
>   sc->sc_ac_cookie = malloc(ac_cookie_len, M_DEVBUF,
>   M_DONTWAIT);
>   if (sc->sc_ac_cookie == NULL)
> @@ -557,7 +570,8 @@ breakbreak:
>   }
>   if (relay_sid) {
>   if (sc->sc_relay_sid)
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF,
> + sc->sc_relay_sid_len);
>   sc->sc_relay_sid = malloc(relay_sid_len, M_DEVBUF,
>   M_DONTWAIT);
>   if (sc->sc_relay_sid == NULL)
> @@ -610,11 +624,12 @@ breakbreak:
>   sc->sc_state = PPPOE_STATE_INITIAL;
>   memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
>   if (sc->sc_ac_cookie) {
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF,
> + sc->sc_ac_cookie_len);
>   sc->sc_ac_cookie = NULL;
>   }
>   if (sc->sc_relay_sid) {
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
>   sc->sc_relay_sid = NULL;
>   }
>   sc->sc_ac_cookie_len = 0;
> @@ -817,7 +847,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
>   }
> 
>   if (sc->sc_concentrator_name)
> - free(sc->sc_concentrator_name, M_DEVBUF, 0);
> + free(sc->sc_concentrator_name, M_DEVBUF,
> + strlen(sc->sc_concentrator_name) + 1);
>   sc->sc_concentrator_name = NULL;
> 
>   len = strlen(parms->ac_name);
> @@ -830,7 +861,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
>   }
> 
>   if (sc->sc_service_name)
> - free(sc->sc_service_name, M_DEVBUF, 0);
> + free(sc->sc_service_name, M_DEVBUF,
> + strlen(sc->sc_service_name) + 1);
>   sc->sc_service_name = NULL;
> 
>   len = strlen(parms->service_name);
> @@ -1175,12 +1207,12 @@ pppoe_disconnect(struct pppoe_softc *sc)
>   sc->sc_state = PPPOE_STATE_INITIAL;
>   memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
>   if (sc->sc_ac_cookie) {
> -  

Re: Log mutex for msgbuf concurrency control

2020-08-20 Thread Visa Hankala
On Tue, Aug 18, 2020 at 03:06:58PM +, Visa Hankala wrote:
> This diff introduces a mutex that serializes access to the kernel
> message buffers. At the moment, the buffers do not have clear
> concurrency control.
> 
> The mutex controls access to all the modifiable fields of struct msgbuf.
> It also protects logsoftc.sc_state for managing thread wakeups.
> 
> A tricky thing with the diff is that the code is indirectly used in many
> different contexts and in varied machine-specific functions. Luckily,
> the code already utilizes splhigh() in critical parts, so the transition
> to using a mutex possibly is not such a big step any longer.
> 
> To avoid creating problems with lock order, the code does not take
> new locks when the log mutex is held. This is visible in logwakeup()
> and logread(). It looks that selrecord() does not acquire new locks,
> so no extra steps are needed in logpoll().
> 
> In logread(), the code assumes that there is only one reader. This keeps
> the data extraction logic simple.
> 
> An additional benefit (?) of the diff is that now uiomove(9) is called
> without splhigh().
> 
> I have tested the diff on amd64 and octeon. However, additional tests
> on these and other platforms would be very helpful.

Here is an updated diff that has sleep_setup()'s priority parameter
fixed. The mistake was noticed by mpi@.

Index: kern/subr_log.c
===
RCS file: src/sys/kern/subr_log.c,v
retrieving revision 1.68
diff -u -p -r1.68 subr_log.c
--- kern/subr_log.c 18 Aug 2020 13:41:49 -  1.68
+++ kern/subr_log.c 20 Aug 2020 13:52:05 -
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifdef KTRACE
@@ -69,8 +70,12 @@
 #define LOG_ASYNC  0x04
 #define LOG_RDWAIT 0x08
 
+/*
+ * Locking:
+ * L   log_mtx
+ */
 struct logsoftc {
-   int sc_state;   /* see above for possibilities */
+   int sc_state;   /* [L] see above for possibilities */
struct  selinfo sc_selp;/* process waiting on select call */
struct  sigio_ref sc_sigio; /* async I/O registration */
int sc_need_wakeup; /* if set, wake up waiters */
@@ -83,6 +88,14 @@ struct   msgbuf *msgbufp;/* the mapped b
 struct msgbuf *consbufp;   /* console message buffer. */
 struct file *syslogf;
 
+/*
+ * Lock that serializes access to log message buffers.
+ * This should be kept as a leaf lock in order not to constrain where
+ * printf(9) can be used.
+ */
+struct mutex log_mtx =
+MUTEX_INITIALIZER_FLAGS(IPL_HIGH, "logmtx", MTX_NOWITNESS);
+
 void filt_logrdetach(struct knote *kn);
 int filt_logread(struct knote *kn, long hint);
 
@@ -144,13 +157,11 @@ initconsbuf(void)
 void
 msgbuf_putchar(struct msgbuf *mbp, const char c)
 {
-   int s;
-
if (mbp->msg_magic != MSG_MAGIC)
/* Nothing we can do */
return;
 
-   s = splhigh();
+   mtx_enter(_mtx);
mbp->msg_bufc[mbp->msg_bufx++] = c;
if (mbp->msg_bufx < 0 || mbp->msg_bufx >= mbp->msg_bufs)
mbp->msg_bufx = 0;
@@ -160,20 +171,19 @@ msgbuf_putchar(struct msgbuf *mbp, const
mbp->msg_bufr = 0;
mbp->msg_bufd++;
}
-   splx(s);
+   mtx_leave(_mtx);
 }
 
 size_t
 msgbuf_getlen(struct msgbuf *mbp)
 {
long len;
-   int s;
 
-   s = splhigh();
+   mtx_enter(_mtx);
len = mbp->msg_bufx - mbp->msg_bufr;
if (len < 0)
len += mbp->msg_bufs;
-   splx(s);
+   mtx_leave(_mtx);
return (len);
 }
 
@@ -208,34 +218,47 @@ logclose(dev_t dev, int flag, int mode, 
 int
 logread(dev_t dev, struct uio *uio, int flag)
 {
+   struct sleep_state sls;
struct msgbuf *mbp = msgbufp;
-   size_t l;
-   int s, error = 0;
+   size_t l, rpos;
+   int error = 0;
 
-   s = splhigh();
+   mtx_enter(_mtx);
while (mbp->msg_bufr == mbp->msg_bufx) {
if (flag & IO_NDELAY) {
error = EWOULDBLOCK;
goto out;
}
logsoftc.sc_state |= LOG_RDWAIT;
-   error = tsleep_nsec(mbp, LOG_RDPRI | PCATCH, "klog", INFSLP);
+   mtx_leave(_mtx);
+   /*
+* Set up and enter sleep manually instead of using msleep()
+* to keep log_mtx as a leaf lock.
+*/
+   sleep_setup(, mbp, LOG_RDPRI | PCATCH, "klog");
+   sleep_setup_signal();
+   sleep_finish(, logsoftc.sc_state & LOG_RDWAIT);
+   error = sleep_finish_signal();
+   mtx_enter(_mtx);
if (error)
goto out;
}
-   logsoftc.sc_state &= ~LOG_RDWAIT;
 
if (mbp->msg_bufd > 0) {
char buf[64];
+   long ndropped;
 
+ 

Re: pppoe: add sizes to free() calls

2020-08-20 Thread Vitaliy Makkoveev
> On 20 Aug 2020, at 16:33, Klemens Nanni  wrote:
> 
> These are straight forward as we either maintain a size variable all the
> way or can reuse strlen() for free() just like it's done during malloc().
> 
> One exception is freeing the softc structure, which is fixed in size;
> `ifconfig pppoe1 create; ifconfig pppoe1 destroy' exercises this code
> path and does not blow up - as expected.
> 
> Running fine on a octeon vdsl2 router.
> 
> Feedback? OK?

Hi.

You forgot about ‘\0’ for `sc_concentrator_name’ and `sc_service_name'.

> 
> 
> Index: if_pppoe.c
> ===
> RCS file: /cvs/src/sys/net/if_pppoe.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 if_pppoe.c
> --- if_pppoe.c28 Jul 2020 09:52:32 -  1.70
> +++ if_pppoe.c19 Aug 2020 23:33:23 -
> @@ -257,15 +264,17 @@ pppoe_clone_destroy(struct ifnet *ifp)
>   if_detach(ifp);
> 
>   if (sc->sc_concentrator_name)
> - free(sc->sc_concentrator_name, M_DEVBUF, 0);
> + free(sc->sc_concentrator_name, M_DEVBUF,
> + strlen(sc->sc_concentrator_name));
>   if (sc->sc_service_name)
> - free(sc->sc_service_name, M_DEVBUF, 0);
> + free(sc->sc_service_name, M_DEVBUF,
> + strlen(sc->sc_service_name));
>   if (sc->sc_ac_cookie)
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
>   if (sc->sc_relay_sid)
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
> 
> - free(sc, M_DEVBUF, 0);
> + free(sc, M_DEVBUF, sizeof(*sc));
> 
>   return (0);
> }
> @@ -547,7 +556,8 @@ breakbreak:
>   }
>   if (ac_cookie) {
>   if (sc->sc_ac_cookie)
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF,
> + sc->sc_ac_cookie_len);
>   sc->sc_ac_cookie = malloc(ac_cookie_len, M_DEVBUF,
>   M_DONTWAIT);
>   if (sc->sc_ac_cookie == NULL)
> @@ -557,7 +567,8 @@ breakbreak:
>   }
>   if (relay_sid) {
>   if (sc->sc_relay_sid)
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF,
> + sc->sc_relay_sid_len);
>   sc->sc_relay_sid = malloc(relay_sid_len, M_DEVBUF,
>   M_DONTWAIT);
>   if (sc->sc_relay_sid == NULL)
> @@ -610,11 +621,12 @@ breakbreak:
>   sc->sc_state = PPPOE_STATE_INITIAL;
>   memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
>   if (sc->sc_ac_cookie) {
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF,
> + sc->sc_ac_cookie_len);
>   sc->sc_ac_cookie = NULL;
>   }
>   if (sc->sc_relay_sid) {
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
>   sc->sc_relay_sid = NULL;
>   }
>   sc->sc_ac_cookie_len = 0;
> @@ -817,7 +835,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
>   }
> 
>   if (sc->sc_concentrator_name)
> - free(sc->sc_concentrator_name, M_DEVBUF, 0);
> + free(sc->sc_concentrator_name, M_DEVBUF,
> + strlen(sc->sc_concentrator_name));
>   sc->sc_concentrator_name = NULL;
> 
>   len = strlen(parms->ac_name);
> @@ -830,7 +849,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
>   }
> 
>   if (sc->sc_service_name)
> - free(sc->sc_service_name, M_DEVBUF, 0);
> + free(sc->sc_service_name, M_DEVBUF,
> + strlen(sc->sc_service_name));
>   sc->sc_service_name = NULL;
> 
>   len = strlen(parms->service_name);
> @@ -1175,12 +1195,12 @@ pppoe_disconnect(struct pppoe_softc *sc)
>   sc->sc_state = PPPOE_STATE_INITIAL;
>   memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
>   if (sc->sc_ac_cookie) {
> - free(sc->sc_ac_cookie, M_DEVBUF, 0);
> + free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
>   sc->sc_ac_cookie = NULL;
>   }
>   sc->sc_ac_cookie_len = 0;
>   if (sc->sc_relay_sid) {
> - free(sc->sc_relay_sid, M_DEVBUF, 0);
> + free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
>   sc->sc_relay_sid = NULL;
>   }
>   sc->sc_relay_sid_len = 0;
> 



Re: pppoe: add sizes to free() calls

2020-08-20 Thread Klemens Nanni
On Thu, Aug 20, 2020 at 03:33:17PM +0200, Klemens Nanni wrote:
> These are straight forward as we either maintain a size variable all the
> way or can reuse strlen() for free() just like it's done during malloc().
> 
> One exception is freeing the softc structure, which is fixed in size;
> `ifconfig pppoe1 create; ifconfig pppoe1 destroy' exercises this code
> path and does not blow up - as expected.
> 
> Running fine on a octeon vdsl2 router.
> 
> Feedback? OK?
Sorry for the noise, Peter J. Philipp pointed out how I missed the +1
byte to account for the NUL character which strlen(9) does not count.
All corresponding malloc calls do strlen() + 1, so without it there's an
off-by-one in the size.

In my setup I do not exercise the service and concentrator name paths,
but playing with `ifconfig pppoe0 [[-]pppoeac foo] [[-]pppoesvc bar]'
doesn't blow up on any of those diff versions, either.

Fixed diff.

Index: if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_pppoe.c
--- if_pppoe.c  28 Jul 2020 09:52:32 -  1.70
+++ if_pppoe.c  20 Aug 2020 13:50:07 -
@@ -257,15 +267,17 @@ pppoe_clone_destroy(struct ifnet *ifp)
if_detach(ifp);
 
if (sc->sc_concentrator_name)
-   free(sc->sc_concentrator_name, M_DEVBUF, 0);
+   free(sc->sc_concentrator_name, M_DEVBUF,
+   strlen(sc->sc_concentrator_name) + 1);
if (sc->sc_service_name)
-   free(sc->sc_service_name, M_DEVBUF, 0);
+   free(sc->sc_service_name, M_DEVBUF,
+   strlen(sc->sc_service_name) + 1);
if (sc->sc_ac_cookie)
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
if (sc->sc_relay_sid)
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
 
-   free(sc, M_DEVBUF, 0);
+   free(sc, M_DEVBUF, sizeof(*sc));
 
return (0);
 }
@@ -547,7 +559,8 @@ breakbreak:
}
if (ac_cookie) {
if (sc->sc_ac_cookie)
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF,
+   sc->sc_ac_cookie_len);
sc->sc_ac_cookie = malloc(ac_cookie_len, M_DEVBUF,
M_DONTWAIT);
if (sc->sc_ac_cookie == NULL)
@@ -557,7 +570,8 @@ breakbreak:
}
if (relay_sid) {
if (sc->sc_relay_sid)
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF,
+   sc->sc_relay_sid_len);
sc->sc_relay_sid = malloc(relay_sid_len, M_DEVBUF,
M_DONTWAIT);
if (sc->sc_relay_sid == NULL)
@@ -610,11 +624,12 @@ breakbreak:
sc->sc_state = PPPOE_STATE_INITIAL;
memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
if (sc->sc_ac_cookie) {
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF,
+   sc->sc_ac_cookie_len);
sc->sc_ac_cookie = NULL;
}
if (sc->sc_relay_sid) {
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
sc->sc_relay_sid = NULL;
}
sc->sc_ac_cookie_len = 0;
@@ -817,7 +847,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
}
 
if (sc->sc_concentrator_name)
-   free(sc->sc_concentrator_name, M_DEVBUF, 0);
+   free(sc->sc_concentrator_name, M_DEVBUF,
+   strlen(sc->sc_concentrator_name) + 1);
sc->sc_concentrator_name = NULL;
 
len = strlen(parms->ac_name);
@@ -830,7 +861,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
}
 
if (sc->sc_service_name)
-   free(sc->sc_service_name, M_DEVBUF, 0);
+   free(sc->sc_service_name, M_DEVBUF,
+   strlen(sc->sc_service_name) + 1);
sc->sc_service_name = NULL;
 
len = strlen(parms->service_name);
@@ -1175,12 +1207,12 @@ pppoe_disconnect(struct pppoe_softc *sc)
sc->sc_state = PPPOE_STATE_INITIAL;
memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
if (sc->sc_ac_cookie) {
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);

pppoe: add sizes to free() calls

2020-08-20 Thread Klemens Nanni
These are straight forward as we either maintain a size variable all the
way or can reuse strlen() for free() just like it's done during malloc().

One exception is freeing the softc structure, which is fixed in size;
`ifconfig pppoe1 create; ifconfig pppoe1 destroy' exercises this code
path and does not blow up - as expected.

Running fine on a octeon vdsl2 router.

Feedback? OK?


Index: if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_pppoe.c
--- if_pppoe.c  28 Jul 2020 09:52:32 -  1.70
+++ if_pppoe.c  19 Aug 2020 23:33:23 -
@@ -257,15 +264,17 @@ pppoe_clone_destroy(struct ifnet *ifp)
if_detach(ifp);
 
if (sc->sc_concentrator_name)
-   free(sc->sc_concentrator_name, M_DEVBUF, 0);
+   free(sc->sc_concentrator_name, M_DEVBUF,
+   strlen(sc->sc_concentrator_name));
if (sc->sc_service_name)
-   free(sc->sc_service_name, M_DEVBUF, 0);
+   free(sc->sc_service_name, M_DEVBUF,
+   strlen(sc->sc_service_name));
if (sc->sc_ac_cookie)
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
if (sc->sc_relay_sid)
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
 
-   free(sc, M_DEVBUF, 0);
+   free(sc, M_DEVBUF, sizeof(*sc));
 
return (0);
 }
@@ -547,7 +556,8 @@ breakbreak:
}
if (ac_cookie) {
if (sc->sc_ac_cookie)
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF,
+   sc->sc_ac_cookie_len);
sc->sc_ac_cookie = malloc(ac_cookie_len, M_DEVBUF,
M_DONTWAIT);
if (sc->sc_ac_cookie == NULL)
@@ -557,7 +567,8 @@ breakbreak:
}
if (relay_sid) {
if (sc->sc_relay_sid)
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF,
+   sc->sc_relay_sid_len);
sc->sc_relay_sid = malloc(relay_sid_len, M_DEVBUF,
M_DONTWAIT);
if (sc->sc_relay_sid == NULL)
@@ -610,11 +621,12 @@ breakbreak:
sc->sc_state = PPPOE_STATE_INITIAL;
memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
if (sc->sc_ac_cookie) {
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF,
+   sc->sc_ac_cookie_len);
sc->sc_ac_cookie = NULL;
}
if (sc->sc_relay_sid) {
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
sc->sc_relay_sid = NULL;
}
sc->sc_ac_cookie_len = 0;
@@ -817,7 +835,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
}
 
if (sc->sc_concentrator_name)
-   free(sc->sc_concentrator_name, M_DEVBUF, 0);
+   free(sc->sc_concentrator_name, M_DEVBUF,
+   strlen(sc->sc_concentrator_name));
sc->sc_concentrator_name = NULL;
 
len = strlen(parms->ac_name);
@@ -830,7 +849,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
}
 
if (sc->sc_service_name)
-   free(sc->sc_service_name, M_DEVBUF, 0);
+   free(sc->sc_service_name, M_DEVBUF,
+   strlen(sc->sc_service_name));
sc->sc_service_name = NULL;
 
len = strlen(parms->service_name);
@@ -1175,12 +1195,12 @@ pppoe_disconnect(struct pppoe_softc *sc)
sc->sc_state = PPPOE_STATE_INITIAL;
memcpy(>sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
if (sc->sc_ac_cookie) {
-   free(sc->sc_ac_cookie, M_DEVBUF, 0);
+   free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
sc->sc_ac_cookie = NULL;
}
sc->sc_ac_cookie_len = 0;
if (sc->sc_relay_sid) {
-   free(sc->sc_relay_sid, M_DEVBUF, 0);
+   free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
sc->sc_relay_sid = NULL;
}
sc->sc_relay_sid_len = 0;



Re: release.8 - .fs -> .img

2020-08-20 Thread Theo Buehler
On Wed, Aug 19, 2020 at 11:42:39PM -0700, na...@airpost.net wrote:
> Update the name of the install image.

Fixed, thanks.

> 
> Index: release.8
> ===
> RCS file: /cvs/src/share/man/man8/release.8,v
> retrieving revision 1.95
> diff -u -p -r1.95 release.8
> --- release.8 29 Apr 2020 15:02:51 -  1.95
> +++ release.8 20 Aug 2020 06:38:16 -
> @@ -258,7 +258,7 @@ This is described in
>  .Xr ports 7 .
>  .Ss 8. Create boot and installation disk images
>  The disk images
> -.No install${ Ns Va VERSION Ns }.fs
> +.No install${ Ns Va VERSION Ns }.img
>  and
>  .No install${ Ns Va VERSION Ns }.iso
>  are suitable for installs without network connectivity.
> 



release.8 - .fs -> .img

2020-08-20 Thread navan
Update the name of the install image.

Index: release.8
===
RCS file: /cvs/src/share/man/man8/release.8,v
retrieving revision 1.95
diff -u -p -r1.95 release.8
--- release.8   29 Apr 2020 15:02:51 -  1.95
+++ release.8   20 Aug 2020 06:38:16 -
@@ -258,7 +258,7 @@ This is described in
 .Xr ports 7 .
 .Ss 8. Create boot and installation disk images
 The disk images
-.No install${ Ns Va VERSION Ns }.fs
+.No install${ Ns Va VERSION Ns }.img
 and
 .No install${ Ns Va VERSION Ns }.iso
 are suitable for installs without network connectivity.