Re: m_copym2 is unused, let's remove it

[David Gwynne]

Yes, this is just a conservative first step to that.

On 14 Sep 2016 12:00 p.m., "Todd C. Miller" 
wrote:

> Looks OK.  Do you intend to change m_copym0 to m_copym and remove
> the deep copy code?
>
>  - todd
>

Re: random malloc junk

[Theo Buehler]

On Tue, Sep 13, 2016 at 01:29:17PM +0200, Otto Moerbeek wrote:
> As often, real life came in between. Did anybody do measurements? I
> really would like to to see hard data.

It seems that the price is relatively modest. 

I ran several 'make build's:

3rd gen X1, i7 5500U, 2.4GHz (amd64):
no malloc.conf, malloc.c r1.195:
 1525.04 real  2505.98 user  1362.47 sys
no malloc.conf, malloc.c + diff:
 1532.15 real  2540.63 user  1356.98 sys

for comparison:
malloc.conf -> J, malloc.c + diff:
 1554.76 real  2596.40 user  1353.26 sys

Acer Aspire 5633WLMi, Core 2 Duo 1.66 GHz (i386):
no malloc.conf, malloc.c, r1.195:
 5020.77 real  5725.21 user  1609.28 sys
no malloc.conf, malloc.c + diff:
 5088.07 real  5865.80 user  1572.12 sys

Re: random malloc junk

[Theo Buehler]

On Wed, Sep 07, 2016 at 06:29:07PM -0400, Ted Unangst wrote:
> There's some overlap here with canaries, but nothing wrong with that. :)

The diff breaks canaries since random_junk() overwrites them before they
are validated.  The following straightforward modification fixes that:

> Index: malloc.c
> ===

[..]

> @@ -1336,10 +1367,11 @@ ofree(struct dir_info *argpool, void *p)
>   }
>   STATS_SUB(pool->malloc_guarded, mopts.malloc_guard);
>   }
> - if (mopts.malloc_junk && !mopts.malloc_freeunmap) {
> - size_t amt = mopts.malloc_junk == 1 ? MALLOC_MAXCHUNK :
> - PAGEROUND(sz) - mopts.malloc_guard;
> - memset(p, SOME_FREEJUNK, amt);
> + if (mopts.malloc_junk == 2 && !mopts.malloc_freeunmap) {
> + memset(p, SOME_FREEJUNK,
> + PAGEROUND(sz) - mopts.malloc_guard);
> + } else if (mopts.malloc_junk == 1 && !mopts.malloc_freeunmap) {
> + random_junk(p, MALLOC_MAXCHUNK); 

should be:
random_junk(p, MALLOC_MAXCHUNK - mopts.malloc_canaries);

>   }
>   unmap(pool, p, PAGEROUND(sz));
>   delete(pool, r);
> @@ -1347,8 +1379,10 @@ ofree(struct dir_info *argpool, void *p)
>   void *tmp;
>   int i;
>  
> - if (mopts.malloc_junk && sz > 0)
> + if (mopts.malloc_junk == 2 && sz > 0)
>   memset(p, SOME_FREEJUNK, sz - mopts.malloc_canaries);
> + else if (mopts.malloc_junk == 1 && sz > 0)
> + random_junk(p, sz);

should be:
random_junk(p, sz - mopts.malloc_canaries);

>   if (!mopts.malloc_freenow) {
>   if (find_chunknum(pool, r, p) == -1)
>   goto done;
> 

Re: m_copym2 is unused, let's remove it

[Todd C. Miller]

Looks OK.  Do you intend to change m_copym0 to m_copym and remove
the deep copy code?

 - todd

Re: m_copym2 is unused, let's remove it

[David Gwynne]

> On 14 Sep 2016, at 11:08, David Gwynne  wrote:
> 
> ok?
> 
> Index: sys/arch/amd64/conf/GENERIC
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
> retrieving revision 1.433
> diff -u -p -r1.433 GENERIC
> --- sys/arch/amd64/conf/GENERIC   12 Sep 2016 08:28:44 -  1.433
> +++ sys/arch/amd64/conf/GENERIC   13 Sep 2016 23:54:46 -
> @@ -27,6 +27,8 @@ option  HIBERNATE   # Hibernate support
> 
> configbsd swap generic
> 
> +makeoption DEBUG="-g"
> +
> mainbus0 at root
> 
> bios0 at mainbus?

ignore this chunk

dlg

m_copym2 is unused, let's remove it

[David Gwynne]

ok?

Index: share/man/man9/mbuf.9
===
RCS file: /cvs/src/share/man/man9/mbuf.9,v
retrieving revision 1.103
diff -u -p -r1.103 mbuf.9
--- share/man/man9/mbuf.9   13 Sep 2016 19:56:55 -  1.103
+++ share/man/man9/mbuf.9   13 Sep 2016 23:54:46 -
@@ -29,7 +29,6 @@
 .Dt MGET 9
 .Os
 .Sh NAME
-.Nm m_copym2 ,
 .Nm m_copym ,
 .Nm m_free ,
 .Nm m_get ,
@@ -70,8 +69,6 @@
 .Sh SYNOPSIS
 .In sys/mbuf.h
 .Ft struct mbuf *
-.Fn m_copym2 "struct mbuf *m" "int off" "int len" "int wait"
-.Ft struct mbuf *
 .Fn m_copym "struct mbuf *m" "int off" "int len" "int wait"
 .Ft struct mbuf *
 .Fn m_free "struct mbuf *m"
@@ -437,12 +434,6 @@ The
 parameter can be M_WAIT or
 M_DONTWAIT.
 It does not copy clusters, it just increases their reference count.
-.It Fn m_copym2 "struct mbuf *m" "int off" "int len" "int wait"
-The same as
-.Fn m_copym
-except that it copies cluster mbufs, whereas
-.Fn m_copym
-just increases the reference count of the clusters.
 .It Fn m_free "struct mbuf *m"
 Free the mbuf pointed to by
 .Fa m .
Index: sys/arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.433
diff -u -p -r1.433 GENERIC
--- sys/arch/amd64/conf/GENERIC 12 Sep 2016 08:28:44 -  1.433
+++ sys/arch/amd64/conf/GENERIC 13 Sep 2016 23:54:46 -
@@ -27,6 +27,8 @@ optionHIBERNATE   # Hibernate support
 
 config bsd swap generic
 
+makeoption DEBUG="-g"
+
 mainbus0 at root
 
 bios0  at mainbus?
Index: sys/kern/uipc_mbuf.c
===
RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.228
diff -u -p -r1.228 uipc_mbuf.c
--- sys/kern/uipc_mbuf.c13 Sep 2016 19:56:55 -  1.228
+++ sys/kern/uipc_mbuf.c13 Sep 2016 23:54:47 -
@@ -576,16 +576,6 @@ m_copym(struct mbuf *m, int off, int len
return m_copym0(m, off, len, wait, 0);  /* shallow copy on M_EXT */
 }
 
-/*
- * m_copym2() is like m_copym(), except it COPIES cluster mbufs, instead
- * of merely bumping the reference count.
- */
-struct mbuf *
-m_copym2(struct mbuf *m, int off, int len, int wait)
-{
-   return m_copym0(m, off, len, wait, 1);  /* deep copy */
-}
-
 struct mbuf *
 m_copym0(struct mbuf *m0, int off, int len, int wait, int deep)
 {
Index: sys/sys/mbuf.h
===
RCS file: /cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.218
diff -u -p -r1.218 mbuf.h
--- sys/sys/mbuf.h  13 Sep 2016 19:56:55 -  1.218
+++ sys/sys/mbuf.h  13 Sep 2016 23:54:47 -
@@ -423,7 +423,6 @@ extern  int max_protohdr;   /* largest pro
 extern int max_hdr;/* largest link+protocol header */
 
 void   mbinit(void);
-struct mbuf *m_copym2(struct mbuf *, int, int, int);
 struct mbuf *m_copym(struct mbuf *, int, int, int);
 struct mbuf *m_free(struct mbuf *);
 struct mbuf *m_get(int, int);

Re: axen(4) lladdr fix

[David Gwynne]

> On 13 Sep 2016, at 19:37, Martin Pieuchot  wrote:
> 
> Doing...
>   
>   # ifconfig axen0 lladdr bla
>   
> is currently broken because we don't update the MAC address in the
> driver, which makes the chip drop all the packets unless in promisc
> mode.
> 
> Diff below fixes that, ok?

ok

> 
> Index: if_axen.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_axen.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 if_axen.c
> --- if_axen.c 13 Apr 2016 11:03:37 -  1.22
> +++ if_axen.c 13 Sep 2016 09:35:14 -
> @@ -499,6 +499,10 @@ axen_ax88179_init(struct axen_softc *sc)
>   }
>   axen_cmd(sc, AXEN_CMD_MAC_SET_RXSR, 5, AXEN_RX_BULKIN_QCTRL, );
> 
> + /* Set MAC address. */
> + axen_cmd(sc, AXEN_CMD_MAC_WRITE_ETHER, ETHER_ADDR_LEN,
> + AXEN_CMD_MAC_NODE_ID, >arpcom.ac_enaddr);
> +
>   /*
>* set buffer high/low watermark to pause/resume.
>* write 2byte will set high/log simultaneous with AXEN_PAUSE_HIGH.
> @@ -662,11 +666,10 @@ axen_attach(struct device *parent, struc
>*/
>   /* use MAC command */
>   axen_lock_mii(sc);
> - axen_cmd(sc, AXEN_CMD_MAC_READ_ETHER, 6, AXEN_CMD_MAC_NODE_ID, );
> + axen_cmd(sc, AXEN_CMD_MAC_READ_ETHER, ETHER_ADDR_LEN,
> + AXEN_CMD_MAC_NODE_ID, );
>   axen_unlock_mii(sc);
> 
> - axen_ax88179_init(sc);
> -
>   /*
>* An ASIX chip was detected. Inform the world.
>*/
> @@ -678,6 +681,8 @@ axen_attach(struct device *parent, struc
>   printf(", address %s\n", ether_sprintf(eaddr));
> 
>   bcopy(eaddr, (char *)>arpcom.ac_enaddr, ETHER_ADDR_LEN);
> +
> + axen_ax88179_init(sc);
> 
>   /* Initialize interface info. */
>   ifp = >arpcom.ac_if;
> Index: if_axenreg.h
> ===
> RCS file: /cvs/src/sys/dev/usb/if_axenreg.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 if_axenreg.h
> --- if_axenreg.h  16 Jul 2015 00:17:40 -  1.5
> +++ if_axenreg.h  13 Sep 2016 09:28:17 -
> @@ -182,6 +182,7 @@
> 
> /*   6byte cmd   */ 
> #define AXEN_CMD_MAC_READ_ETHER   0x6001
> +#define AXEN_CMD_MAC_WRITE_ETHER 0x6101
> #define   AXEN_CMD_MAC_NODE_ID  0x10
> 
> /*   8byte cmd   */ 
> 

innetgr(3): fix matching when user, host and domain are specified

[Todd C. Miller]

Currently, innetgr(3) will return false if all of user, host and
domain are specified, even if the tuple would otherwise match.

The commented assumption "If a domainname is given, we would have
found a match" is simply not true.

I also moved the strdup after the "Too bad need the slow recursive
way" comment for readability.

 - todd

Index: lib/libc/gen/getnetgrent.c
===
RCS file: /cvs/src/lib/libc/gen/getnetgrent.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 getnetgrent.c
--- lib/libc/gen/getnetgrent.c  14 Sep 2015 16:09:13 -  1.27
+++ lib/libc/gen/getnetgrent.c  13 Sep 2016 19:54:38 -
@@ -704,20 +704,16 @@ innetgr(const char *grp, const char *hos
if (in_lookup(ypdom, grp, user, domain, _NG_KEYBYUSER))
return 1;
}
-   /* If a domainname is given, we would have found a match */
-   if (domain != NULL)
+
+   /* Too bad need the slow recursive way */
+   sl = _ng_sl_init();
+   if (sl == NULL)
return 0;
 
grpdup = strdup(grp);
if (grpdup == NULL)
return 0;
 
-   /* Too bad need the slow recursive way */
-   sl = _ng_sl_init();
-   if (sl == NULL) {
-   free(grpdup);
-   return 0;
-   }
found = in_find(ypdom, sl, grpdup, host, user, domain);
_ng_sl_free(sl, 1);
 

Re: generate pkg-config files at build time

[Jasper Lievisse Adriaanse]

On Tue, Sep 13, 2016 at 06:35:08PM +0200, Martin Natano wrote:
> Currently pkg-config files are generated at install time, while they
> should be generated at build time like everything else. One reason why
> generating files during install is bad is that the two steps might be
> run by two differnt users, resulting in permission problems.
> 
> While there I removed the dependency from the install target on the
> pkg-config files as this is also not done for libraries and programs.
> If you didn't build before install you are fucked anyway.
> 
> Ok?
> 
> natano
Makes sense; OK.

> Index: lib/libcrypto/Makefile
> ===
> RCS file: /cvs/src/lib/libcrypto/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- lib/libcrypto/Makefile11 Sep 2016 14:31:02 -  1.5
> +++ lib/libcrypto/Makefile12 Sep 2016 18:19:53 -
> @@ -431,10 +431,11 @@ distribution:
>   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
>  ${.CURDIR}/x509v3.cnf ${DESTDIR}/etc/ssl/x509v3.cnf
>  
> +all: ${PC_FILES}
>  ${PC_FILES}: opensslv.h
>   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
>  
> -beforeinstall: ${PC_FILES}
> +beforeinstall:
>   ${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
>   -m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
>  
> Index: lib/libexpat/Makefile
> ===
> RCS file: /cvs/src/lib/libexpat/Makefile,v
> retrieving revision 1.10
> diff -u -p -r1.10 Makefile
> --- lib/libexpat/Makefile 4 Sep 2016 09:54:25 -   1.10
> +++ lib/libexpat/Makefile 12 Sep 2016 18:20:08 -
> @@ -17,10 +17,11 @@ includes:
> ${INSTALL} ${INSTALL_COPY} -m 444 -o $(BINOWN) -g $(BINGRP) \
> ${.CURDIR}/lib/expat_external.h 
> ${DESTDIR}/usr/include/expat_external.h
>  
> +all: ${PC_FILES}
>  ${PC_FILES}: lib/expat.h
>   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
>  
> -beforeinstall: ${PC_FILES}
> +beforeinstall:
>   ${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
>   -m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
>  
> Index: lib/libfuse/Makefile
> ===
> RCS file: /cvs/src/lib/libfuse/Makefile,v
> retrieving revision 1.9
> diff -u -p -r1.9 Makefile
> --- lib/libfuse/Makefile  4 Sep 2016 09:54:25 -   1.9
> +++ lib/libfuse/Makefile  12 Sep 2016 18:20:16 -
> @@ -29,10 +29,11 @@ includes:
>   eval "$$j"; \
>   done
>  
> +all: ${PC_FILES}
>  ${PC_FILES}: fuse_private.h
>   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
>  
> -beforeinstall: ${PC_FILES}
> +beforeinstall:
>   ${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
>   -m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
>  
> Index: lib/libssl/Makefile
> ===
> RCS file: /cvs/src/lib/libssl/Makefile,v
> retrieving revision 1.21
> diff -u -p -r1.21 Makefile
> --- lib/libssl/Makefile   4 Sep 2016 09:54:25 -   1.21
> +++ lib/libssl/Makefile   12 Sep 2016 18:20:34 -
> @@ -48,10 +48,11 @@ includes:
>  
>  .include 
>  
> +all: ${PC_FILES}
>  ${PC_FILES}: ${.CURDIR}/../libcrypto/opensslv.h
>   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
>  
> -beforeinstall: ${PC_FILES}
> +beforeinstall:
>   nm -o lib${LIB}.a | egrep -w 'printf|fprintf' && \
>   (echo please fix stdio usage in this library; false) || true
>  .for p in ${PC_FILES}
> Index: lib/libz/Makefile
> ===
> RCS file: /cvs/src/lib/libz/Makefile,v
> retrieving revision 1.19
> diff -u -p -r1.19 Makefile
> --- lib/libz/Makefile 4 Sep 2016 09:54:25 -   1.19
> +++ lib/libz/Makefile 12 Sep 2016 18:20:44 -
> @@ -19,10 +19,11 @@ includes:
>   eval "$$j"; \
>   done
>  
> +all: ${PC_FILES}
>  ${PC_FILES}: zlib.h
>   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
>  
> -beforeinstall: ${PC_FILES}
> +beforeinstall:
>   ${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
>   -m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
>  
> 

-- 
jasper

generate pkg-config files at build time

[Martin Natano]

Currently pkg-config files are generated at install time, while they
should be generated at build time like everything else. One reason why
generating files during install is bad is that the two steps might be
run by two differnt users, resulting in permission problems.

While there I removed the dependency from the install target on the
pkg-config files as this is also not done for libraries and programs.
If you didn't build before install you are fucked anyway.

Ok?

natano


Index: lib/libcrypto/Makefile
===
RCS file: /cvs/src/lib/libcrypto/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- lib/libcrypto/Makefile  11 Sep 2016 14:31:02 -  1.5
+++ lib/libcrypto/Makefile  12 Sep 2016 18:19:53 -
@@ -431,10 +431,11 @@ distribution:
${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
   ${.CURDIR}/x509v3.cnf ${DESTDIR}/etc/ssl/x509v3.cnf
 
+all: ${PC_FILES}
 ${PC_FILES}: opensslv.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libexpat/Makefile
===
RCS file: /cvs/src/lib/libexpat/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- lib/libexpat/Makefile   4 Sep 2016 09:54:25 -   1.10
+++ lib/libexpat/Makefile   12 Sep 2016 18:20:08 -
@@ -17,10 +17,11 @@ includes:
  ${INSTALL} ${INSTALL_COPY} -m 444 -o $(BINOWN) -g $(BINGRP) \
  ${.CURDIR}/lib/expat_external.h 
${DESTDIR}/usr/include/expat_external.h
 
+all: ${PC_FILES}
 ${PC_FILES}: lib/expat.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libfuse/Makefile
===
RCS file: /cvs/src/lib/libfuse/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- lib/libfuse/Makefile4 Sep 2016 09:54:25 -   1.9
+++ lib/libfuse/Makefile12 Sep 2016 18:20:16 -
@@ -29,10 +29,11 @@ includes:
eval "$$j"; \
done
 
+all: ${PC_FILES}
 ${PC_FILES}: fuse_private.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libssl/Makefile
===
RCS file: /cvs/src/lib/libssl/Makefile,v
retrieving revision 1.21
diff -u -p -r1.21 Makefile
--- lib/libssl/Makefile 4 Sep 2016 09:54:25 -   1.21
+++ lib/libssl/Makefile 12 Sep 2016 18:20:34 -
@@ -48,10 +48,11 @@ includes:
 
 .include 
 
+all: ${PC_FILES}
 ${PC_FILES}: ${.CURDIR}/../libcrypto/opensslv.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
nm -o lib${LIB}.a | egrep -w 'printf|fprintf' && \
(echo please fix stdio usage in this library; false) || true
 .for p in ${PC_FILES}
Index: lib/libz/Makefile
===
RCS file: /cvs/src/lib/libz/Makefile,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile
--- lib/libz/Makefile   4 Sep 2016 09:54:25 -   1.19
+++ lib/libz/Makefile   12 Sep 2016 18:20:44 -
@@ -19,10 +19,11 @@ includes:
eval "$$j"; \
done
 
+all: ${PC_FILES}
 ${PC_FILES}: zlib.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 

Re: acme switch

[Todd C. Miller]

On Tue, 13 Sep 2016 12:25:58 -0400, "Ted Unangst" wrote:

> convert two if else if chains to switches.

OK millert@

 - todd

acme switch

[Ted Unangst]

convert two if else if chains to switches.

Index: base64.c
===
RCS file: /cvs/src/usr.sbin/acme-client/base64.c,v
retrieving revision 1.5
diff -u -p -r1.5 base64.c
--- base64.c1 Sep 2016 13:49:32 -   1.5
+++ base64.c13 Sep 2016 16:24:52 -
@@ -51,12 +51,17 @@ base64buf_url(const char *data, size_t l
b64_ntop(data, len, buf, sz);
 
for (i = 0; i < sz; i++)
-   if ('+' == buf[i])
+   switch (buf[i]) {
+   case '+':
buf[i] = '-';
-   else if ('/' == buf[i])
+   break;
+   case '/':
buf[i] = '_';
-   else if ('=' == buf[i])
+   break;
+   case '=':
buf[i] = '\0';
+   break;
+   }
 
return (buf);
 }
Index: json.c
===
RCS file: /cvs/src/usr.sbin/acme-client/json.c,v
retrieving revision 1.4
diff -u -p -r1.4 json.c
--- json.c  13 Sep 2016 16:04:51 -  1.4
+++ json.c  13 Sep 2016 16:24:52 -
@@ -152,15 +152,23 @@ jsmnparse_free(struct parse *p)
 
if (NULL == p)
return;
-   for (i = 0; i < p->max; i++)
-   if (JSMN_ARRAY == p->nodes[i].type)
-   free(p->nodes[i].d.array);
-   else if (JSMN_OBJECT == p->nodes[i].type)
-   free(p->nodes[i].d.obj);
-   else if (JSMN_PRIMITIVE == p->nodes[i].type)
-   free(p->nodes[i].d.str);
-   else if (JSMN_STRING == p->nodes[i].type)
-   free(p->nodes[i].d.str);
+   for (i = 0; i < p->max; i++) {
+   struct jsmnn*n = >nodes[i];
+   switch (p->nodes[i].type) {
+   case JSMN_ARRAY:
+   free(n->d.array);
+   break;
+   case JSMN_OBJECT:
+   free(n->d.obj);
+   break;
+   case JSMN_PRIMITIVE:
+   free(n->d.str);
+   break;
+   case JSMN_STRING:
+   free(n->d.str);
+   break;
+   }
+   }
free(p->nodes);
free(p);
 }

Invalid references in ncurses(3) man page

[Anthony Coulter]

The ncurses(3) and curs_util(3) man pages reference some manual pages
that do not exist.

$ man 3 curs_memleaks
man: No entry for curs_memleak in the manual
$ man 3 curs_trace
man: No entry for curs_trace in the manual
$ man 3 legacy_coding
man: No entry for legacy_coding in the manual.

This proposed diff simply excises the faulty references. This might not
be the correct thing, particularly for the line about use_legacy_coding
(which is at least described in curs_util(3) even if it is not listed
at the top of that page). The other functions listed in curses.3tbl do
not appear to be described in any extant man pages, however.


Index: lib/libcurses/curs_util.3
===
RCS file: /open/anoncvs/cvs/src/lib/libcurses/curs_util.3,v
retrieving revision 1.6
diff -u -p -r1.6 curs_util.3
--- lib/libcurses/curs_util.3   12 Jan 2010 23:21:59 -  1.6
+++ lib/libcurses/curs_util.3   13 Sep 2016 15:18:22 -
@@ -246,12 +246,10 @@ It was not supported on Version 7, BSD o
 It is recommended that any code depending on ncurses extensions
 be conditioned using NCURSES_VERSION.
 .SH SEE ALSO
-\fBlegacy_coding\fR(3),
 \fBcurses\fR(3),
 \fBcurs_initscr\fR(3),
 \fBcurs_kernel\fR(3),
 \fBcurs_scr_dump\fR(3),
-\fBlegacy_coding\fR(3).
 .\"#
 .\"# The following sets edit modes for GNU EMACS
 .\"# Local Variables:
Index: lib/libcurses/curses.3tbl
===
RCS file: /open/anoncvs/cvs/src/lib/libcurses/curses.3tbl,v
retrieving revision 1.25
diff -u -p -r1.25 curses.3tbl
--- lib/libcurses/curses.3tbl   3 Dec 2015 11:29:55 -   1.25
+++ lib/libcurses/curses.3tbl   13 Sep 2016 15:32:11 -
@@ -288,17 +288,6 @@ l l .
 =
 COLOR_PAIR/\fBcurs_color\fR(3)
 PAIR_NUMBER/\fBcurs_attr\fR(3)
-_nc_free_and_exit/\fBcurs_memleaks\fR(3)*
-_nc_freeall/\fBcurs_memleaks\fR(3)*
-_nc_tracebits/\fBcurs_trace\fR(3)*
-_traceattr/\fBcurs_trace\fR(3)*
-_traceattr2/\fBcurs_trace\fR(3)*
-_tracechar/\fBcurs_trace\fR(3)*
-_tracechtype/\fBcurs_trace\fR(3)*
-_tracechtype2/\fBcurs_trace\fR(3)*
-_tracedump/\fBcurs_trace\fR(3)*
-_tracef/\fBcurs_trace\fR(3)*
-_tracemouse/\fBcurs_trace\fR(3)*
 add_wch/\fBcurs_add_wch\fR(3)
 add_wchnstr/\fBcurs_add_wchstr\fR(3)
 add_wchstr/\fBcurs_add_wchstr\fR(3)
@@ -628,7 +617,6 @@ untouchwin/\fBcurs_touch\fR(3)
 use_default_colors/\fBdefault_colors\fR(3)*
 use_env/\fBcurs_util\fR(3)
 use_extended_names/\fBcurs_extend\fR(3)*
-use_legacy_coding/\fBlegacy_coding\fR(3)*
 vid_attr/\fBterminfo\fR(3)
 vid_puts/\fBterminfo\fR(3)
 vidattr/\fBterminfo\fR(3)

Re: ip6_setpktopt: dead code & param

[Vincent Gross]

On Tue, 13 Sep 2016 14:19:24 +0200
j...@wxcvbn.org (Jeremie Courreges-Anglas) wrote:

> Since it has been introduced, ip6_setpktopt has only been called with
> (sticky=1, cmsg=0) or (sticky=0, cmsg=1).  Let's simplify this code.

Ok vgross@

> 
> 
> Index: ip6_output.c
> ===
> RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
> retrieving revision 1.213
> diff -u -p -p -u -r1.213 ip6_output.c
> --- ip6_output.c  25 Aug 2016 12:30:16 -  1.213
> +++ ip6_output.c  13 Sep 2016 11:56:19 -
> @@ -119,8 +119,7 @@ struct ip6_exthdrs {
>  int ip6_pcbopt(int, u_char *, int, struct ip6_pktopts **, int, int);
>  int ip6_pcbopts(struct ip6_pktopts **, struct mbuf *, struct socket
> *); int ip6_getpcbopt(struct ip6_pktopts *, int, struct mbuf **);
> -int ip6_setpktopt(int, u_char *, int, struct ip6_pktopts *, int, int,
> - int, int);
> +int ip6_setpktopt(int, u_char *, int, struct ip6_pktopts *, int,
> int, int); int ip6_setmoptions(int, struct ip6_moptions **, struct
> mbuf *); int ip6_getmoptions(int, struct ip6_moptions *, struct mbuf
> **); int ip6_copyexthdr(struct mbuf **, caddr_t, int);
> @@ -1770,7 +1769,7 @@ ip6_pcbopt(int optname, u_char *buf, int
>   }
>   opt = *pktopt;
>  
> - return (ip6_setpktopt(optname, buf, len, opt, priv, 1, 0,
> uproto));
> + return (ip6_setpktopt(optname, buf, len, opt, priv, 1,
> uproto)); }
>  
>  int
> @@ -2352,7 +2351,7 @@ ip6_setpktopts(struct mbuf *control, str
>   return (EINVAL);
>   if (cm->cmsg_level == IPPROTO_IPV6) {
>   error = ip6_setpktopt(cm->cmsg_type,
> CMSG_DATA(cm),
> - cm->cmsg_len - CMSG_LEN(0), opt, priv,
> 0, 1, uproto);
> + cm->cmsg_len - CMSG_LEN(0), opt, priv,
> 0, uproto); if (error)
>   return (error);
>   }
> @@ -2367,39 +2366,12 @@ ip6_setpktopts(struct mbuf *control, str
>  /*
>   * Set a particular packet option, as a sticky option or an
> ancillary data
>   * item.  "len" can be 0 only when it's a sticky option.
> - * We have 4 cases of combination of "sticky" and "cmsg":
> - * "sticky=0, cmsg=0": impossible
> - * "sticky=0, cmsg=1": RFC2292 or RFC3542 ancillary data
> - * "sticky=1, cmsg=0": RFC3542 socket option
> - * "sticky=1, cmsg=1": RFC2292 socket option
>   */
>  int
>  ip6_setpktopt(int optname, u_char *buf, int len, struct ip6_pktopts
> *opt,
> -int priv, int sticky, int cmsg, int uproto)
> +int priv, int sticky, int uproto)
>  {
>   int minmtupolicy;
> -
> - if (!sticky && !cmsg) {
> -#ifdef DIAGNOSTIC
> - printf("ip6_setpktopt: impossible case\n");
> -#endif
> - return (EINVAL);
> - }
> -
> - if (sticky && cmsg) {
> - switch (optname) {
> - case IPV6_PKTINFO:
> - case IPV6_HOPLIMIT:
> - case IPV6_HOPOPTS:
> - case IPV6_DSTOPTS:
> - case IPV6_RTHDRDSTOPTS:
> - case IPV6_RTHDR:
> - case IPV6_USE_MIN_MTU:
> - case IPV6_DONTFRAG:
> - case IPV6_TCLASS:
> - return (ENOPROTOOPT);
> - }
> - }
>  
>   switch (optname) {
>   case IPV6_PKTINFO:
> 

Re: netinet6 bcopy->memcpy

[Jeremie Courreges-Anglas]

David Hill  writes:

> Hello -
>
> Here is a diff to switch some bcopy's to memcpy's.
>
> Most bcopy's are on freshly alloc'd memory.

Looks fine to me, ok jca@

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

ip6_setpktopt: dead code & param

[Jeremie Courreges-Anglas]

Since it has been introduced, ip6_setpktopt has only been called with
(sticky=1, cmsg=0) or (sticky=0, cmsg=1).  Let's simplify this code.


Index: ip6_output.c
===
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.213
diff -u -p -p -u -r1.213 ip6_output.c
--- ip6_output.c25 Aug 2016 12:30:16 -  1.213
+++ ip6_output.c13 Sep 2016 11:56:19 -
@@ -119,8 +119,7 @@ struct ip6_exthdrs {
 int ip6_pcbopt(int, u_char *, int, struct ip6_pktopts **, int, int);
 int ip6_pcbopts(struct ip6_pktopts **, struct mbuf *, struct socket *);
 int ip6_getpcbopt(struct ip6_pktopts *, int, struct mbuf **);
-int ip6_setpktopt(int, u_char *, int, struct ip6_pktopts *, int, int,
-   int, int);
+int ip6_setpktopt(int, u_char *, int, struct ip6_pktopts *, int, int, int);
 int ip6_setmoptions(int, struct ip6_moptions **, struct mbuf *);
 int ip6_getmoptions(int, struct ip6_moptions *, struct mbuf **);
 int ip6_copyexthdr(struct mbuf **, caddr_t, int);
@@ -1770,7 +1769,7 @@ ip6_pcbopt(int optname, u_char *buf, int
}
opt = *pktopt;
 
-   return (ip6_setpktopt(optname, buf, len, opt, priv, 1, 0, uproto));
+   return (ip6_setpktopt(optname, buf, len, opt, priv, 1, uproto));
 }
 
 int
@@ -2352,7 +2351,7 @@ ip6_setpktopts(struct mbuf *control, str
return (EINVAL);
if (cm->cmsg_level == IPPROTO_IPV6) {
error = ip6_setpktopt(cm->cmsg_type, CMSG_DATA(cm),
-   cm->cmsg_len - CMSG_LEN(0), opt, priv, 0, 1, 
uproto);
+   cm->cmsg_len - CMSG_LEN(0), opt, priv, 0, uproto);
if (error)
return (error);
}
@@ -2367,39 +2366,12 @@ ip6_setpktopts(struct mbuf *control, str
 /*
  * Set a particular packet option, as a sticky option or an ancillary data
  * item.  "len" can be 0 only when it's a sticky option.
- * We have 4 cases of combination of "sticky" and "cmsg":
- * "sticky=0, cmsg=0": impossible
- * "sticky=0, cmsg=1": RFC2292 or RFC3542 ancillary data
- * "sticky=1, cmsg=0": RFC3542 socket option
- * "sticky=1, cmsg=1": RFC2292 socket option
  */
 int
 ip6_setpktopt(int optname, u_char *buf, int len, struct ip6_pktopts *opt,
-int priv, int sticky, int cmsg, int uproto)
+int priv, int sticky, int uproto)
 {
int minmtupolicy;
-
-   if (!sticky && !cmsg) {
-#ifdef DIAGNOSTIC
-   printf("ip6_setpktopt: impossible case\n");
-#endif
-   return (EINVAL);
-   }
-
-   if (sticky && cmsg) {
-   switch (optname) {
-   case IPV6_PKTINFO:
-   case IPV6_HOPLIMIT:
-   case IPV6_HOPOPTS:
-   case IPV6_DSTOPTS:
-   case IPV6_RTHDRDSTOPTS:
-   case IPV6_RTHDR:
-   case IPV6_USE_MIN_MTU:
-   case IPV6_DONTFRAG:
-   case IPV6_TCLASS:
-   return (ENOPROTOOPT);
-   }
-   }
 
switch (optname) {
case IPV6_PKTINFO:

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

Re: look(1): eliminate FOLD and DICT macros

[Jeremie Courreges-Anglas]

"Todd C. Miller"  writes:

> There's no need to check for isascii() with ANSI ctype macros/functions.
> Eliminating the macros makes the code clearer.

ok jca@

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

Re: random malloc junk

[Otto Moerbeek]

On Thu, Sep 08, 2016 at 08:15:42AM +0200, Otto Moerbeek wrote:

> On Wed, Sep 07, 2016 at 06:29:07PM -0400, Ted Unangst wrote:
> 
> > Instead of always using a fixed byte pattern, I think malloc should use a
> > random pattern. Now, this sometimes means it's harder to identify exactly
> > what's used after free, so we should provide a means to get the old 0xdf
> > pattern back.
> > 
> > Since we already have two junk modes, I thought I'd carry on along those
> > lines. The default junk behavior, for free chunks only, is more of a 
> > security
> > measure. I think this means we want random junk. The second level 'J' junk 
> > is
> > more of a debugging tool, so that retains 0xdf.
> > 
> > There's some overlap here with canaries, but nothing wrong with that. :)
> 
> Like it, though I am a bit worried about the costs. Any measurements?
> 
> Should be able to look more closely the coming days.

As often, real life came in between. Did anybody do measurements? I
really would like to to see hard data.

-Otto

> 
> BTW, we should revisit canaries and work further on moving them
> closer to requested size. There's a chance this diff wil complicate
> that.
> 
>   -Otto
> 
> > 
> > Index: malloc.c
> > ===
> > RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> > retrieving revision 1.195
> > diff -u -p -r1.195 malloc.c
> > --- malloc.c1 Sep 2016 10:41:02 -   1.195
> > +++ malloc.c7 Sep 2016 22:21:37 -
> > @@ -186,6 +186,7 @@ struct malloc_readonly {
> >  #endif
> > u_int32_t malloc_canary;/* Matched against ones in malloc_pool 
> > */
> > uintptr_t malloc_chunk_canary;
> > +   u_char  malloc_junkbytes[256];
> >  };
> >  
> >  /* This object is mapped PROT_READ after initialisation to prevent 
> > tampering */
> > @@ -597,6 +598,8 @@ omalloc_init(void)
> > mopts.malloc_move = 1;
> > mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
> >  
> > +   arc4random_buf(mopts.malloc_junkbytes, sizeof(mopts.malloc_junkbytes));
> > +
> > for (i = 0; i < 3; i++) {
> > switch (i) {
> > case 0:
> > @@ -1260,7 +1263,29 @@ malloc(size_t size)
> >  /*DEF_STRONG(malloc);*/
> >  
> >  static void
> > -validate_junk(struct dir_info *pool, void *p) {
> > +random_junk(void *p, size_t amt)
> > +{
> > +   u_long offset = ((u_long)p) & (sizeof(mopts.malloc_junkbytes) - 1);
> > +
> > +   if (amt < sizeof(mopts.malloc_junkbytes) - offset) {
> > +   memcpy(p, mopts.malloc_junkbytes + offset, amt);
> > +   } else {
> > +   memcpy(p, mopts.malloc_junkbytes + offset,
> > +   sizeof(mopts.malloc_junkbytes) - offset);
> > +   amt -= sizeof(mopts.malloc_junkbytes) - offset;
> > +   while (amt > 0) {
> > +   size_t x = amt > sizeof(mopts.malloc_junkbytes) ?
> > +   sizeof(mopts.malloc_junkbytes) : amt;
> > +   memcpy(p, mopts.malloc_junkbytes, x);
> > +   amt -= x;
> > +   }
> > +   }
> > +}
> > +
> > +
> > +static void
> > +validate_junk(struct dir_info *pool, void *p)
> > +{
> > struct region_info *r;
> > size_t byte, sz;
> >  
> > @@ -1274,9 +1299,15 @@ validate_junk(struct dir_info *pool, voi
> > sz -= mopts.malloc_canaries;
> > if (sz > 32)
> > sz = 32;
> > -   for (byte = 0; byte < sz; byte++) {
> > -   if (((unsigned char *)p)[byte] != SOME_FREEJUNK)
> > +   if (mopts.malloc_junk == 1) {
> > +   u_long offset = ((u_long)p) & (sizeof(mopts.malloc_junkbytes) - 
> > 1);
> > +   if (memcmp(p, mopts.malloc_junkbytes + offset, sz) != 0)
> > wrterror(pool, "use after free", p);
> > +   } else {
> > +   for (byte = 0; byte < sz; byte++) {
> > +   if (((unsigned char *)p)[byte] != SOME_FREEJUNK)
> > +   wrterror(pool, "use after free", p);
> > +   }
> > }
> >  }
> >  
> > @@ -1336,10 +1367,11 @@ ofree(struct dir_info *argpool, void *p)
> > }
> > STATS_SUB(pool->malloc_guarded, mopts.malloc_guard);
> > }
> > -   if (mopts.malloc_junk && !mopts.malloc_freeunmap) {
> > -   size_t amt = mopts.malloc_junk == 1 ? MALLOC_MAXCHUNK :
> > -   PAGEROUND(sz) - mopts.malloc_guard;
> > -   memset(p, SOME_FREEJUNK, amt);
> > +   if (mopts.malloc_junk == 2 && !mopts.malloc_freeunmap) {
> > +   memset(p, SOME_FREEJUNK,
> > +   PAGEROUND(sz) - mopts.malloc_guard);
> > +   } else if (mopts.malloc_junk == 1 && !mopts.malloc_freeunmap) {
> > +   random_junk(p, MALLOC_MAXCHUNK); 
> > }
> > unmap(pool, p, PAGEROUND(sz));
> > delete(pool, r);
> > @@ -1347,8 +1379,10 @@ ofree(struct dir_info *argpool, void *p)
> > void *tmp;
> > int i;
> >  

Re: random malloc junk

[Otto Moerbeek]

On Thu, Sep 08, 2016 at 06:42:33PM -0400, Daniel Micay wrote:

> On Wed, 2016-09-07 at 18:29 -0400, Ted Unangst wrote:
> > Instead of always using a fixed byte pattern, I think malloc should
> > use a
> > random pattern. Now, this sometimes means it's harder to identify
> > exactly
> > what's used after free, so we should provide a means to get the old
> > 0xdf
> > pattern back.
> > 
> > Since we already have two junk modes, I thought I'd carry on along
> > those
> > lines. The default junk behavior, for free chunks only, is more of a
> > security
> > measure. I think this means we want random junk. The second level 'J'
> > junk is
> > more of a debugging tool, so that retains 0xdf.
> 
> A bit off-topic: 'J' enables junk-on-init which is for debugging, but it
> also currently has security improvements for large allocations. There's
> only partial junk-on-free by default (half a page), and 'U' disables
> large allocation junk-on-free without 'J'. I think it would make sense
> to remove those optimizations since it's fine if the cost scales up with
> larger allocations and losing the guarantee of not leaking data via
> uninitialized memory with 'U' is not great. Using 'U' is quite expensive
> regardless, and adds some pathological performance cases for small size
> allocations which is more important. I ended up removing both of those
> optimizations for the CopperheadOS port.

I would prefer to see a diff with this. For me, that should be easier
to understand than you description.

-Otto

Re: Fix an infinite loop in iked

[Mike Belopuhov]

On 10 September 2016 at 08:42, Nikolay Edigaryev  wrote:
> This fixes a bug introduced in revision 1.8 of timer.c that causes
> evtimer_set() to be called on an already active event, which is an error
> according to event_add(3):
>
>>The event in the ev argument must be already initialized by event_set()
>>and may not be used in calls to event_set() until it has timed out or
>>been removed with event_del(). If the event in the ev argument already
>>has a scheduled timeout, the old timeout will be replaced by the new
>>one.
>
> The simplest way to trigger the loop is by toggling the active/passive
> knob via ikectl:
>
> $ doas ikectl active # Calls event_set() and starts 2 second timer
> $ doas ikectl active # Calls event_set() on an active timer
>
> It can also be triggered by connecting to a peer and receiving
> INVALID_KE_PAYLOAD notification from it, in which case event_set() will
> be called in ikev2_pld_notify().
>

Indeed.  Thanks for your report and fix; it has been just committed.

pledge(2), *: fix [referering] typos

[Frank Schoep]

Came across two 'refer[e]ring' typos in the pledge(2) man page. The first
diff fixes those. After grepping through the source tree, there's a few
more, similar typos in unrelated files. Attached a second diff for those.

Left HTTP-related stuff alone since refe[r]er is the preferred term in that
context. There's also some gcc/binutils abi_refe[r]er mentions, but I'm not
sure if those should be renamed. Didn't include these either.


Index: pledge.2
===
RCS file: /cvs/src/lib/libc/sys/pledge.2,v
retrieving revision 1.35
diff -u -p -r1.35 pledge.2
--- pledge.25 Sep 2016 11:17:24 -   1.35
+++ pledge.213 Sep 2016 09:14:40 -
@@ -380,11 +380,11 @@ operations.
 .It Va "sendfd"
 Allows sending of file descriptors using
 .Xr sendmsg 2 .
-File descriptors referering to directories may not be passed.
+File descriptors referring to directories may not be passed.
 .It Va "recvfd"
 Allows receiving of file descriptors using
 .Xr recvmsg 2 .
-File descriptors referering to directories may not be passed.
+File descriptors referring to directories may not be passed.
 .It Va "ioctl"
 Allows a subset of
 .Xr ioctl 2


Index: gnu/llvm/include/llvm/IR/TrackingMDRef.h
===
RCS file: /cvs/src/gnu/llvm/include/llvm/IR/TrackingMDRef.h,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 TrackingMDRef.h
--- gnu/llvm/include/llvm/IR/TrackingMDRef.h3 Sep 2016 22:46:58 -
1.1.1.1
+++ gnu/llvm/include/llvm/IR/TrackingMDRef.h13 Sep 2016 09:26:12 -
@@ -96,7 +96,7 @@ private:

 /// \brief Typed tracking ref.
 ///
-/// Track refererences of a particular type.  It's useful to use this for \a
+/// Track references of a particular type.  It's useful to use this for \a
 /// MDNode and \a ValueAsMetadata.
 template  class TypedTrackingMDRef {
   TrackingMDRef Ref;
Index: gnu/usr.bin/binutils/gdb/doc/gdbint.texinfo
===
RCS file: /cvs/src/gnu/usr.bin/binutils/gdb/doc/gdbint.texinfo,v
retrieving revision 1.6
diff -u -p -r1.6 gdbint.texinfo
--- gnu/usr.bin/binutils/gdb/doc/gdbint.texinfo 27 Dec 2004 14:00:52 -
1.6
+++ gnu/usr.bin/binutils/gdb/doc/gdbint.texinfo 13 Sep 2016 09:26:20 -
@@ -2590,7 +2590,7 @@ Given the type flags representing an add
 its name.
 @end deftypefn
 @deftypefn {Target Macro} int ADDRESS_CLASS_NAME_to_TYPE_FLAGS (int @var{name},
 int *var{type_flags_ptr})
-Given an address qualifier name, set the @code{int} refererenced by @var{type_f
lags_ptr} to the type flags
+Given an address qualifier name, set the @code{int} referenced by @var{type_fla
gs_ptr} to the type flags
 for that address class qualifier.
 @end deftypefn

Index: sbin/isakmpd/transport.c
===
RCS file: /cvs/src/sbin/isakmpd/transport.c,v
retrieving revision 1.37
diff -u -p -r1.37 transport.c
--- sbin/isakmpd/transport.c10 Mar 2016 07:32:16 -  1.37
+++ sbin/isakmpd/transport.c13 Sep 2016 09:26:43 -
@@ -89,7 +89,7 @@ transport_setup(struct transport *t, int
t->flags = 0;
 }

-/* Add a referer to transport T.  */
+/* Add a referrer to transport T.  */
 void
 transport_reference(struct transport *t)
 {
@@ -100,7 +100,7 @@ transport_reference(struct transport *t)
 }

 /*
- * Remove a referer from transport T, removing all of T when no referers left.
+ * Remove a referrer from transport T, removing all of T when no referers left.
  */
 void
 transport_release(struct transport *t)
Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.88
diff -u -p -r1.88 pipex.c
--- sys/net/pipex.c 30 Aug 2016 23:29:04 -  1.88
+++ sys/net/pipex.c 13 Sep 2016 09:26:48 -
@@ -838,7 +838,7 @@ pipex_timer(void *ignored_arg)
case PIPEX_STATE_CLOSED:
/*
 * mbuf queued in pipexinq or pipexoutq may have a
-* refererce to this session.
+* reference to this session.
 */
if (!mq_empty() || !mq_empty())
continue;

bpf_mtap(9) w/o KERNEL_LOCK

[Martin Pieuchot]

Here's the big scary diff I've been using for some months now to stop
grabbing the KERNEL_LOCK() in bpf_mtap(9).  This has been originally
written to prevent lock ordering inside pf_test().  Now that we're
heading toward using a rwlock, we won't have this problem, but fewer
usages of KERNEL_LOCK() is still interesting.

I'm going to split this diff in small chunks to ease the review.  But
I'd appreciate if people could try to break it, test & report back.

Some notes:

  - Now that selwakeup() is called in a thread context (task) we only
rely on the KERNEL_LOCK() to serialize access to kqueue(9) data.

  - The reference counting is here to make sure a descriptor is not
freed during a sleep.  That's why the KERNEL_LOCK() is still
necessary in the slow path.  On the other hand bpf_catchpacket()
relies on the reference guaranteed by the SRP list.

  - A mutex now protects the rotating buffers and their associated
fields.  It is dropped before calling ifpromisc() because USB
devices sleep.

  - The dance around uiomove(9) is here to check that buffers aren't
rotated while data is copied to userland.  Setting ``b->bd_fbuf''
to NULL should be enough to let bpf_catchpacket() to drop the
patcket.  But I added ``__in_uiomove'' to be able to have usable
panic if something weird happen.

Comments?

Index: net/bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.149
diff -u -p -r1.149 bpf.c
--- net/bpf.c   12 Sep 2016 16:24:37 -  1.149
+++ net/bpf.c   13 Sep 2016 09:56:18 -
@@ -92,15 +92,13 @@ int bpf_maxbufsize = BPF_MAXBUFSIZE;
 struct bpf_if  *bpf_iflist;
 LIST_HEAD(, bpf_d) bpf_d_list;
 
-void   bpf_allocbufs(struct bpf_d *);
+intbpf_allocbufs(struct bpf_d *);
 void   bpf_ifname(struct ifnet *, struct ifreq *);
 int_bpf_mtap(caddr_t, const struct mbuf *, u_int,
void (*)(const void *, void *, size_t));
 void   bpf_mcopy(const void *, void *, size_t);
 intbpf_movein(struct uio *, u_int, struct mbuf **,
struct sockaddr *, struct bpf_insn *);
-void   bpf_attachd(struct bpf_d *, struct bpf_if *);
-void   bpf_detachd(struct bpf_d *);
 intbpf_setif(struct bpf_d *, struct ifreq *);
 intbpfpoll(dev_t, int, struct proc *);
 intbpfkqfilter(dev_t, struct knote *);
@@ -108,7 +106,6 @@ voidbpf_wakeup(struct bpf_d *);
 void   bpf_wakeup_cb(void *);
 void   bpf_catchpacket(struct bpf_d *, u_char *, size_t, size_t,
void (*)(const void *, void *, size_t), struct timeval *);
-void   bpf_reset_d(struct bpf_d *);
 intbpf_getdltlist(struct bpf_d *, struct bpf_dltlist *);
 intbpf_setdlt(struct bpf_d *, u_int);
 
@@ -120,6 +117,13 @@ intbpf_sysctl_locked(int *, u_int, void
 struct bpf_d *bpfilter_lookup(int);
 
 /*
+ * Called holding ``bd_mtx''.
+ */
+void   bpf_attachd(struct bpf_d *, struct bpf_if *);
+void   bpf_detachd(struct bpf_d *);
+void   bpf_resetd(struct bpf_d *);
+
+/*
  * Reference count access to descriptor buffers
  */
 void   bpf_get(struct bpf_d *);
@@ -259,11 +263,12 @@ bpf_movein(struct uio *uio, u_int linkty
 
 /*
  * Attach file to the bpf interface, i.e. make d listen on bp.
- * Must be called at splnet.
  */
 void
 bpf_attachd(struct bpf_d *d, struct bpf_if *bp)
 {
+   MUTEX_ASSERT_LOCKED(>bd_mtx);
+
/*
 * Point d at bp, and add d to the interface's list of listeners.
 * Finally, point the driver's bpf cookie at the interface so
@@ -286,7 +291,23 @@ bpf_detachd(struct bpf_d *d)
 {
struct bpf_if *bp;
 
+   MUTEX_ASSERT_LOCKED(>bd_mtx);
+
bp = d->bd_bif;
+
+   /* Remove d from the interface's descriptor list. */
+   KERNEL_ASSERT_LOCKED();
+   SRPL_REMOVE_LOCKED(_d_rc, >bif_dlist, d, bpf_d, bd_next);
+
+   if (SRPL_EMPTY_LOCKED(>bif_dlist)) {
+   /*
+* Let the driver know that there are no more listeners.
+*/
+   *bp->bif_driverp = NULL;
+   }
+
+   d->bd_bif = NULL;
+
/*
 * Check if this descriptor had requested promiscuous mode.
 * If so, turn it off.
@@ -295,7 +316,13 @@ bpf_detachd(struct bpf_d *d)
int error;
 
d->bd_promisc = 0;
+
+   bpf_get(d);
+   mtx_leave(>bd_mtx);
error = ifpromisc(bp->bif_ifp, 0);
+   mtx_enter(>bd_mtx);
+   bpf_put(d);
+
if (error && !(error == EINVAL || error == ENODEV))
/*
 * Something is really wrong if we were able to put
@@ -304,19 +331,6 @@ bpf_detachd(struct bpf_d *d)
 */
panic("bpf: ifpromisc failed");
}
-
-   /* Remove d from the interface's descriptor list. */
-   KERNEL_ASSERT_LOCKED();
-   SRPL_REMOVE_LOCKED(_d_rc, >bif_dlist, d, bpf_d, bd_next);
-
-   if (SRPL_EMPTY_LOCKED(>bif_dlist)) {

axen(4) lladdr fix

[Martin Pieuchot]

Doing...

# ifconfig axen0 lladdr bla

is currently broken because we don't update the MAC address in the
driver, which makes the chip drop all the packets unless in promisc
mode.

Diff below fixes that, ok?

Index: if_axen.c
===
RCS file: /cvs/src/sys/dev/usb/if_axen.c,v
retrieving revision 1.22
diff -u -p -r1.22 if_axen.c
--- if_axen.c   13 Apr 2016 11:03:37 -  1.22
+++ if_axen.c   13 Sep 2016 09:35:14 -
@@ -499,6 +499,10 @@ axen_ax88179_init(struct axen_softc *sc)
}
axen_cmd(sc, AXEN_CMD_MAC_SET_RXSR, 5, AXEN_RX_BULKIN_QCTRL, );
 
+   /* Set MAC address. */
+   axen_cmd(sc, AXEN_CMD_MAC_WRITE_ETHER, ETHER_ADDR_LEN,
+   AXEN_CMD_MAC_NODE_ID, >arpcom.ac_enaddr);
+
/*
 * set buffer high/low watermark to pause/resume.
 * write 2byte will set high/log simultaneous with AXEN_PAUSE_HIGH.
@@ -662,11 +666,10 @@ axen_attach(struct device *parent, struc
 */
/* use MAC command */
axen_lock_mii(sc);
-   axen_cmd(sc, AXEN_CMD_MAC_READ_ETHER, 6, AXEN_CMD_MAC_NODE_ID, );
+   axen_cmd(sc, AXEN_CMD_MAC_READ_ETHER, ETHER_ADDR_LEN,
+   AXEN_CMD_MAC_NODE_ID, );
axen_unlock_mii(sc);
 
-   axen_ax88179_init(sc);
-
/*
 * An ASIX chip was detected. Inform the world.
 */
@@ -678,6 +681,8 @@ axen_attach(struct device *parent, struc
printf(", address %s\n", ether_sprintf(eaddr));
 
bcopy(eaddr, (char *)>arpcom.ac_enaddr, ETHER_ADDR_LEN);
+
+   axen_ax88179_init(sc);
 
/* Initialize interface info. */
ifp = >arpcom.ac_if;
Index: if_axenreg.h
===
RCS file: /cvs/src/sys/dev/usb/if_axenreg.h,v
retrieving revision 1.5
diff -u -p -r1.5 if_axenreg.h
--- if_axenreg.h16 Jul 2015 00:17:40 -  1.5
+++ if_axenreg.h13 Sep 2016 09:28:17 -
@@ -182,6 +182,7 @@
 
 /*   6byte cmd   */ 
 #define AXEN_CMD_MAC_READ_ETHER0x6001
+#define AXEN_CMD_MAC_WRITE_ETHER   0x6101
 #define   AXEN_CMD_MAC_NODE_ID   0x10
 
 /*   8byte cmd   */ 

Re: TCP timers & tasks

[Mark Kettenis]

> Date: Tue, 13 Sep 2016 10:30:03 +0200
> From: Martin Pieuchot 
> 
> On 12/09/16(Mon) 13:54, Martin Pieuchot wrote:
> > TCP timers end up calling ip_output(), which will soon require that the
> > caller holds a rwlock.
> > 
> > In order to do that, I diff below changes 'struct tcptimer' to become a
> > tuple of (timeout, task).  When a timeout fires, the corresponding task
> > gets scheduled.
> > 
> > I'm currently using the ``softnettq'' to schedule everything related to
> > the network.  This is the safest approach, because concurrent tasks are
> > still serialized.
> > 
> > I extended the G/C timeout to 1sec, because there's no reference count
> > for 'struct tcpcb'.  And the only race possible is described in the
> > comment below:
> > 
> > /*
> >  * If one of the timer tasks is already running, it must
> >  * be spinning on the KERNEL_LOCK().  So schedule a G/C
> >  * in one second, when we known the task will have released
> >  * its reference.
> >  */
> > 
> > Since the first thing task are doing is checking for the ``TF_DEAD''
> > flag this should be enough.
> > 
> > Comments, oks?
> 
> So I heard a lot of comments about using a specific API for timed task.
> I agreed with all of them but think it's a second step.
> 
> Apart from that, do you have any other concern?  Ok?

I'm a bit concerned about the KERNEL_LOCK here.  Mixing locked tasks
and unlocked tasks in the same thread should be avoided if possible.
But if tasks need to be properly serialized, you have no cjoice.

> > Index: netinet/tcp_timer.c
> > ===
> > RCS file: /cvs/src/sys/netinet/tcp_timer.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 tcp_timer.c
> > --- netinet/tcp_timer.c 7 Mar 2016 18:44:00 -   1.49
> > +++ netinet/tcp_timer.c 12 Sep 2016 11:42:16 -
> > @@ -71,6 +71,11 @@ void tcp_timer_persist(void *);
> >  void   tcp_timer_keep(void *);
> >  void   tcp_timer_2msl(void *);
> >  
> > +void   tcp_timer_rexmt_task(void *);
> > +void   tcp_timer_persist_task(void *);
> > +void   tcp_timer_keep_task(void *);
> > +void   tcp_timer_2msl_task(void *);
> > +
> >  const tcp_timer_func_t tcp_timer_funcs[TCPT_NTIMERS] = {
> > tcp_timer_rexmt,
> > tcp_timer_persist,
> > @@ -78,6 +83,13 @@ const tcp_timer_func_t tcp_timer_funcs[T
> > tcp_timer_2msl,
> >  };
> >  
> > +const tcp_timer_func_t tcp_timer_tasks[TCPT_NTIMERS] = {
> > +   tcp_timer_rexmt_task,
> > +   tcp_timer_persist_task,
> > +   tcp_timer_keep_task,
> > +   tcp_timer_2msl_task,
> > +};
> > +
> >  /*
> >   * Timer state initialization, called from tcp_init().
> >   */
> > @@ -105,6 +117,14 @@ void
> >  tcp_delack(void *arg)
> >  {
> > struct tcpcb *tp = arg;
> > +
> > +   task_add(softnettq, >t_delack_to_task);
> > +}
> > +
> > +void
> > +tcp_delack_task(void *arg)
> > +{
> > +   struct tcpcb *tp = arg;
> > int s;
> >  
> > /*
> > @@ -112,15 +132,15 @@ tcp_delack(void *arg)
> >  * for whatever reason, it will restart the delayed
> >  * ACK callout.
> >  */
> > -
> > +   KERNEL_LOCK();
> > s = splsoftnet();
> > -   if (tp->t_flags & TF_DEAD) {
> > -   splx(s);
> > -   return;
> > -   }
> > +   if (tp->t_flags & TF_DEAD)
> > +   goto out;
> > tp->t_flags |= TF_ACKNOW;
> > (void) tcp_output(tp);
> > + out:
> > splx(s);
> > +   KERNEL_UNLOCK();
> >  }
> >  
> >  /*
> > @@ -144,8 +164,7 @@ tcp_slowtimo(void)
> >   * Cancel all timers for TCP tp.
> >   */
> >  void
> > -tcp_canceltimers(tp)
> > -   struct tcpcb *tp;
> > +tcp_canceltimers(struct tcpcb *tp)
> >  {
> > int i;
> >  
> > @@ -153,6 +172,15 @@ tcp_canceltimers(tp)
> > TCP_TIMER_DISARM(tp, i);
> >  }
> >  
> > +void
> > +tcp_destroytimers(struct tcpcb *tp)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < TCPT_NTIMERS; i++)
> > +   TCP_TIMER_DESTROY(tp, i);
> > +}
> > +
> >  inttcp_backoff[TCP_MAXRXTSHIFT + 1] =
> >  { 1, 2, 4, 8, 16, 32, 64, 64, 64, 64, 64, 64, 64 };
> >  
> > @@ -191,14 +219,21 @@ void
> >  tcp_timer_rexmt(void *arg)
> >  {
> > struct tcpcb *tp = arg;
> > +
> > +   task_add(softnettq, >t_timer[TCPT_REXMT].tcpt_task);
> > +}
> > +
> > +void
> > +tcp_timer_rexmt_task(void *arg)
> > +{
> > +   struct tcpcb *tp = arg;
> > uint32_t rto;
> > int s;
> >  
> > +   KERNEL_LOCK();
> > s = splsoftnet();
> > -   if (tp->t_flags & TF_DEAD) {
> > -   splx(s);
> > -   return;
> > -   }
> > +   if (tp->t_flags & TF_DEAD)
> > +   goto out;
> >  
> > if ((tp->t_flags & TF_PMTUD_PEND) && tp->t_inpcb &&
> > SEQ_GEQ(tp->t_pmtud_th_seq, tp->snd_una) &&
> > @@ -225,8 +260,7 @@ tcp_timer_rexmt(void *arg)
> > sin.sin_addr = tp->t_inpcb->inp_faddr;
> > in_pcbnotifyall(, sintosa(),
> > tp->t_inpcb->inp_rtableid, EMSGSIZE, tcp_mtudisc);
> > -   splx(s);
> > -   

Re: TCP timers & tasks

[Martin Pieuchot]

On 12/09/16(Mon) 13:54, Martin Pieuchot wrote:
> TCP timers end up calling ip_output(), which will soon require that the
> caller holds a rwlock.
> 
> In order to do that, I diff below changes 'struct tcptimer' to become a
> tuple of (timeout, task).  When a timeout fires, the corresponding task
> gets scheduled.
> 
> I'm currently using the ``softnettq'' to schedule everything related to
> the network.  This is the safest approach, because concurrent tasks are
> still serialized.
> 
> I extended the G/C timeout to 1sec, because there's no reference count
> for 'struct tcpcb'.  And the only race possible is described in the
> comment below:
> 
>   /*
>* If one of the timer tasks is already running, it must
>* be spinning on the KERNEL_LOCK().  So schedule a G/C
>* in one second, when we known the task will have released
>* its reference.
>*/
> 
> Since the first thing task are doing is checking for the ``TF_DEAD''
> flag this should be enough.
> 
> Comments, oks?

So I heard a lot of comments about using a specific API for timed task.
I agreed with all of them but think it's a second step.

Apart from that, do you have any other concern?  Ok?

> Index: netinet/tcp_timer.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_timer.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 tcp_timer.c
> --- netinet/tcp_timer.c   7 Mar 2016 18:44:00 -   1.49
> +++ netinet/tcp_timer.c   12 Sep 2016 11:42:16 -
> @@ -71,6 +71,11 @@ void   tcp_timer_persist(void *);
>  void tcp_timer_keep(void *);
>  void tcp_timer_2msl(void *);
>  
> +void tcp_timer_rexmt_task(void *);
> +void tcp_timer_persist_task(void *);
> +void tcp_timer_keep_task(void *);
> +void tcp_timer_2msl_task(void *);
> +
>  const tcp_timer_func_t tcp_timer_funcs[TCPT_NTIMERS] = {
>   tcp_timer_rexmt,
>   tcp_timer_persist,
> @@ -78,6 +83,13 @@ const tcp_timer_func_t tcp_timer_funcs[T
>   tcp_timer_2msl,
>  };
>  
> +const tcp_timer_func_t tcp_timer_tasks[TCPT_NTIMERS] = {
> + tcp_timer_rexmt_task,
> + tcp_timer_persist_task,
> + tcp_timer_keep_task,
> + tcp_timer_2msl_task,
> +};
> +
>  /*
>   * Timer state initialization, called from tcp_init().
>   */
> @@ -105,6 +117,14 @@ void
>  tcp_delack(void *arg)
>  {
>   struct tcpcb *tp = arg;
> +
> + task_add(softnettq, >t_delack_to_task);
> +}
> +
> +void
> +tcp_delack_task(void *arg)
> +{
> + struct tcpcb *tp = arg;
>   int s;
>  
>   /*
> @@ -112,15 +132,15 @@ tcp_delack(void *arg)
>* for whatever reason, it will restart the delayed
>* ACK callout.
>*/
> -
> + KERNEL_LOCK();
>   s = splsoftnet();
> - if (tp->t_flags & TF_DEAD) {
> - splx(s);
> - return;
> - }
> + if (tp->t_flags & TF_DEAD)
> + goto out;
>   tp->t_flags |= TF_ACKNOW;
>   (void) tcp_output(tp);
> + out:
>   splx(s);
> + KERNEL_UNLOCK();
>  }
>  
>  /*
> @@ -144,8 +164,7 @@ tcp_slowtimo(void)
>   * Cancel all timers for TCP tp.
>   */
>  void
> -tcp_canceltimers(tp)
> - struct tcpcb *tp;
> +tcp_canceltimers(struct tcpcb *tp)
>  {
>   int i;
>  
> @@ -153,6 +172,15 @@ tcp_canceltimers(tp)
>   TCP_TIMER_DISARM(tp, i);
>  }
>  
> +void
> +tcp_destroytimers(struct tcpcb *tp)
> +{
> + int i;
> +
> + for (i = 0; i < TCPT_NTIMERS; i++)
> + TCP_TIMER_DESTROY(tp, i);
> +}
> +
>  int  tcp_backoff[TCP_MAXRXTSHIFT + 1] =
>  { 1, 2, 4, 8, 16, 32, 64, 64, 64, 64, 64, 64, 64 };
>  
> @@ -191,14 +219,21 @@ void
>  tcp_timer_rexmt(void *arg)
>  {
>   struct tcpcb *tp = arg;
> +
> + task_add(softnettq, >t_timer[TCPT_REXMT].tcpt_task);
> +}
> +
> +void
> +tcp_timer_rexmt_task(void *arg)
> +{
> + struct tcpcb *tp = arg;
>   uint32_t rto;
>   int s;
>  
> + KERNEL_LOCK();
>   s = splsoftnet();
> - if (tp->t_flags & TF_DEAD) {
> - splx(s);
> - return;
> - }
> + if (tp->t_flags & TF_DEAD)
> + goto out;
>  
>   if ((tp->t_flags & TF_PMTUD_PEND) && tp->t_inpcb &&
>   SEQ_GEQ(tp->t_pmtud_th_seq, tp->snd_una) &&
> @@ -225,8 +260,7 @@ tcp_timer_rexmt(void *arg)
>   sin.sin_addr = tp->t_inpcb->inp_faddr;
>   in_pcbnotifyall(, sintosa(),
>   tp->t_inpcb->inp_rtableid, EMSGSIZE, tcp_mtudisc);
> - splx(s);
> - return;
> + goto out;
>   }
>  
>  #ifdef TCP_SACK
> @@ -378,20 +412,29 @@ tcp_timer_rexmt(void *arg)
>  
>   out:
>   splx(s);
> + KERNEL_UNLOCK();
>  }
>  
>  void
>  tcp_timer_persist(void *arg)
>  {
>   struct tcpcb *tp = arg;
> +
> + task_add(softnettq, >t_timer[TCPT_PERSIST].tcpt_task);
> +}
> +
> +void
> +tcp_timer_persist_task(void *arg)
> +{
> + struct tcpcb *tp = arg;
>   uint32_t rto;
>   int s;
>  
> + KERNEL_LOCK();
>   s = splsoftnet();
>   if 

Re: rwsleep(9)

[Vincent Gross]

On Tue, 13 Sep 2016 10:08:13 +0200
Martin Pieuchot  wrote:

> On 12/09/16(Mon) 12:12, Vincent Gross wrote:
> > On Mon, 12 Sep 2016 10:49:03 +0200
> > Martin Pieuchot  wrote:
> >   
> > > I'd like to use a write lock to serialize accesses to ip_output().
> > > This will be used to guarantee that atomic code sections in the
> > > socket layer stay atomic when the input/forwarding path won't run
> > > under KERNEL_LOCK().
> > > 
> > > For such purpose I'll have to convert some tsleep(9) to an
> > > msleep(9)-like function operating on a write lock.  That's why I'd
> > > like to introduce rwsleep(9).  I did not bother exporting a read
> > > variant of this function since I don't need it for the moment.
> > > 
> > > ok?  
> > 
> > MP noob here :
> > 
> > tsleep() and msleep() check if they are called during
> > autoconfiguration or after a panic to let interrupts run. There is
> > no such check here. I get that rwsleep() during autoconf makes
> > little sense, but to err on the safe side maybe add some kind of
> > assert (if it is not too much of a pain) ? and what about panic,
> > shouldn't this be handled ?  
> 
> This is not a MP problem but an old BSD heritage.  I don't mind adding
> it.  But that's not a real solution to panic being broken with sleep
> or locks.
> 

"old BSD heritage" -> 'nuff said. No need to spread the rot then.

ok vgross@

Re: rwsleep(9)

[Martin Pieuchot]

On 12/09/16(Mon) 12:12, Vincent Gross wrote:
> On Mon, 12 Sep 2016 10:49:03 +0200
> Martin Pieuchot  wrote:
> 
> > I'd like to use a write lock to serialize accesses to ip_output().
> > This will be used to guarantee that atomic code sections in the
> > socket layer stay atomic when the input/forwarding path won't run
> > under KERNEL_LOCK().
> > 
> > For such purpose I'll have to convert some tsleep(9) to an
> > msleep(9)-like function operating on a write lock.  That's why I'd
> > like to introduce rwsleep(9).  I did not bother exporting a read
> > variant of this function since I don't need it for the moment.
> > 
> > ok?
> 
> MP noob here :
> 
> tsleep() and msleep() check if they are called during autoconfiguration
> or after a panic to let interrupts run. There is no such check here. I
> get that rwsleep() during autoconf makes little sense, but to err on
> the safe side maybe add some kind of assert (if it is not too much of
> a pain) ? and what about panic, shouldn't this be handled ?

This is not a MP problem but an old BSD heritage.  I don't mind adding
it.  But that's not a real solution to panic being broken with sleep or
locks.

Re: Fix Wacom Intuos S 2 descriptor and make wsmouse work

[Martin Pieuchot]

On 12/09/16(Mon) 21:14, Frank Groeneveld wrote:
> On Mon, Sep 12, 2016 at 10:18:14AM +0200, Martin Pieuchot wrote:
> > 
> > I just committed your driver, with some tweaks in the manpage.
> 
> Great, thank you!
> 
> > > It is quite likely related to this discussion:
> > > 
> > > http://marc.info/?l=openbsd-misc=140529029513846=2
> > > http://marc.info/?l=openbsd-misc=140589215905202=2
> > > http://marc.info/?l=openbsd-misc=141676273919602=2
> > 
> > Indeed.  This is a common issue to all WSMOUSE_TYPE_TPANEL pointers.
> > 
> > If you're interested in fixing this problem you should look at X11
> > ws(4) driver.  If you can modify the driver and the wscons(4) layer
> > such that it isn't necessary to open a different node to calibrate
> > your device then plugging a device after starting a Xserver wouldn't
> > be a problem.
> 
> Thanks for the pointers, that is most probably the cause indeed. I'll
> try to dig into this. Is the solution you propose the best one, or would
> it also be possible to let the ws driver reopen the device on a failed
> read and fix it like that?

Reopening the device is the problem.  Because the question is when?  If
you can have a working solution for your tablet and a standard mouse
both using the mux, without sending any even to say "hey a new input
device is there, please do something", that would be perfect.