relayd crash using DNS-sanitizing protocol

[Michael W. Lucas]

Hi,

Running 6.0 snapshot from 5 Feb on amd64, and experimenting with
relayd.

I set up a DNS cluster using redirects, as per relayd.conf(5). Worked
fine, so I'm pretty sure the DNS servers behind my relayd box work.

The man page says that relayd has a relay protocol for DNS, that
randomizes query IDs. Cool idea, let's try it. My relayd.conf now
looks like so:

--
table  { 192.0.2.101 192.0.2.102 }
dns protocol dnsfix
relay dns {
listen on 203.0.113.213 port 53
forward to  port 53 check tcp
protocol dnsfix
}
--


With "protocol dnsfix" present, relayd listens on UDP only. I'm
guessing using relayd's DNS protocol makes this happen. Which would
make sense, you don't need it for TCP queries.

So let's try to run this critter.

# relayd -d
startup
socket_rlimit: max open files 1024
socket_rlimit: max open files 1024
pfe: filter init done
socket_rlimit: max open files 1024
socket_rlimit: max open files 1024
relayd_tls_ticket_rekey: rekeying tickets
relay_privinit: adding relay dns
protocol 1: name dnsfix
flags: used, relay flags:
tls session tickets: enabled
type: dns
hce_notify_done: 192.0.2.101 (tcp connect ok)
host 192.0.2.101, check tcp (4ms,tcp connect ok), state unknown -> up, 
availability 100.00%
hce_notify_done: 192.0.2.102 (tcp connect ok)
host 192.0.2.102, check tcp (6ms,tcp connect ok), state unknown -> up, 
availability 100.00%
pfe_dispatch_hce: state 1 for host 1 192.0.2.101
pfe_dispatch_hce: state 1 for host 2 192.0.2.102
adding 2 hosts from table dns:53
adding 2 hosts from table dns:53
relay_launch: running relay dns
relay_launch: running relay dns
adding 2 hosts from table dns:53
relay_launch: running relay dns

I make a DNS query from a client, say to google.com or my site or
whatever, and get:

lost child: pid 779 terminated; signal 11
hce exiting, pid 61465
pfe exiting, pid 93428
ca exiting, pid 1166
ca exiting, pid 11360
ca exiting, pid 57827
lost child: pid 38872 terminated; signal 11
lost child: pid 57998 terminated; signal 11
parent terminating, pid 76339

Am I abusing this program? Or is this a real crash?

Thanks,
==ml


-- 
Michael W. LucasTwitter @mwlauthor 
nonfiction: https://www.michaelwlucas.com/
fiction: https://www.michaelwarrenlucas.com/
blog: http://blather.michaelwlucas.com/

Re: set sc_vendor in bcm2835_dwctwo

[Mark Kettenis]

Jonathan Gray schreef op 2017-02-21 07:08:

Maybe one day these drivers will attach to a non-Broadcom dwc2
but for now they only match the Broadcom compat strings.

-uhub0 at usb0 configuration 1 interface 0 "vendor 0x DWC2 root
hub" rev 2.00/1.00 addr 1
+uhub0 at usb0 configuration 1 interface 0 "Broadcom DWC2 root hub"
rev 2.00/1.00 addr 1


ok kettenis@


Index: armv7/broadcom/bcm2835_dwctwo.c
===
RCS file: /cvs/src/sys/arch/armv7/broadcom/bcm2835_dwctwo.c,v
retrieving revision 1.1
diff -u -p -r1.1 bcm2835_dwctwo.c
--- armv7/broadcom/bcm2835_dwctwo.c 7 Aug 2016 17:46:36 -   1.1
+++ armv7/broadcom/bcm2835_dwctwo.c 20 Feb 2017 11:04:28 -
@@ -135,6 +135,9 @@ bcm_dwctwo_deferred(void *self)
struct bcm_dwctwo_softc *sc = (struct bcm_dwctwo_softc *)self;
int rc;

+   strlcpy(sc->sc_dwc2.sc_vendor, "Broadcom",
+   sizeof(sc->sc_dwc2.sc_vendor));
+
rc = dwc2_init(>sc_dwc2);
if (rc != 0)
return;
Index: arm64/dev/bcm2835_dwctwo.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/bcm2835_dwctwo.c,v
retrieving revision 1.1
diff -u -p -r1.1 bcm2835_dwctwo.c
--- arm64/dev/bcm2835_dwctwo.c  23 Jan 2017 08:05:47 -  1.1
+++ arm64/dev/bcm2835_dwctwo.c  20 Feb 2017 11:04:33 -
@@ -124,6 +124,9 @@ bcm_dwctwo_deferred(void *self)
struct bcm_dwctwo_softc *sc = (struct bcm_dwctwo_softc *)self;
int rc;

+   strlcpy(sc->sc_dwc2.sc_vendor, "Broadcom",
+   sizeof(sc->sc_dwc2.sc_vendor));
+
rc = dwc2_init(>sc_dwc2);
if (rc != 0)
return;

regress/pledge: test for sendfd/recvfd

[Sebastien Marie]

Hi,

The following diff adds regress tests for sendfd/recvfd promises.

The regress will test 5 types of operations for all 7 types of vnodes.

test types:
  - nopledge : no pledge involved - just testing send/recv just work as
expected
  - sendfd : pledge the sender with "stdio sendfd"
  - recvfd : pledge the receiver with "stdio recvfd"
  - nosendfd : pledge the sender with "stdio" (it is expected to fail)
  - norecvfd : pledge the receiver with "stdio" (it is expected to fail)

type of vnodes:
  - VREG : regular file
  - VDIR : directory (should fail with pledge)
  - VBLK : block device
  - VCHR : char device
  - VLNK : link
  - VSOCK : socket
  - VFIFO : fifo

The test program is simple enough to be runned by hand:

$ ./sendrecvfd
usage: sendrecvfd testtype vnodetype
  testtype  = nopledge sendfd recvfd nosendfd norecvfd
  vnodetype = VREG VDIR VBLK VCHAR VLNK VSOCK VFIFO

$ ./sendrecvfd sendfd VDIR
Abort trap (core dumped)

And Makefile has some loop to testing all cases.

-- 
Sebastien Marie

 
Index: Makefile
===
RCS file: Makefile
diff -N Makefile
--- /dev/null   1 Jan 1970 00:00:00 -
+++ Makefile21 Feb 2017 19:24:58 -
@@ -0,0 +1,72 @@
+#  $OpenBSD$
+CFLAGS+=   -Wall -Werror
+
+testtype=  nopledge sendfd recvfd nosendfd norecvfd
+vnodetype= VREG VDIR VBLK VCHR VLNK VSOCK VFIFO
+
+PASS_TARGETS=  test-nopledge-VREG \
+   test-nopledge-VDIR \
+   test-nopledge-VBLK \
+   test-nopledge-VCHR \
+   test-nopledge-VLNK \
+   test-nopledge-VSOCK \
+   test-nopledge-VFIFO \
+   \
+   test-sendfd-VREG \
+   test-sendfd-VBLK \
+   test-sendfd-VCHR \
+   test-sendfd-VLNK \
+   test-sendfd-VSOCK \
+   test-sendfd-VFIFO \
+   \
+   test-recvfd-VREG \
+   test-recvfd-VBLK \
+   test-recvfd-VCHR \
+   test-recvfd-VLNK \
+   test-recvfd-VSOCK \
+   test-recvfd-VFIFO
+
+FAIL_TARGETS=  test-sendfd-VDIR \
+   test-recvfd-VDIR \
+   \
+   test-nosendfd-VREG \
+   test-nosendfd-VDIR \
+   test-nosendfd-VBLK \
+   test-nosendfd-VCHR \
+   test-nosendfd-VLNK \
+   test-nosendfd-VSOCK \
+   test-nosendfd-VFIFO \
+   \
+   test-norecvfd-VREG \
+   test-norecvfd-VDIR \
+   test-norecvfd-VBLK \
+   test-norecvfd-VCHR \
+   test-norecvfd-VLNK \
+   test-norecvfd-VSOCK \
+   test-norecvfd-VFIFO
+
+CLEANFILES+=   sendrecvfd
+
+.for _test in ${testtype}
+. for _vnode in ${vnodetype}
+REGRESS_TARGETS+=  test-${_test}-${_vnode}
+
+.  if ${PASS_TARGETS:Mtest-${_test}-${_vnode}} 
+test-${_test}-${_vnode}: sendrecvfd
+   @echo test-${_test}-${_vnode}: expected PASS
+   @./sendrecvfd ${_test} ${_vnode}
+
+.  elif ${FAIL_TARGETS:Mtest-${_test}-${_vnode}}
+test-${_test}-${_vnode}: sendrecvfd
+   @echo test-${_test}-${_vnode}: expected FAIL
+   @if ./sendrecvfd ${_test} ${_vnode}; then false; else true; fi
+
+.  else
+test-${_test}-${_vnode}:
+   @echo "ERROR: test-${_test}-${_vnode} is missing"
+   @false
+.  endif
+. endfor
+.endfor
+
+.include 
Index: sendrecvfd.c
===
RCS file: sendrecvfd.c
diff -N sendrecvfd.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ sendrecvfd.c21 Feb 2017 19:24:58 -
@@ -0,0 +1,276 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2017 Sebastien Marie 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+enum testtype {
+   nopledge,
+   sendfd,
+   recvfd,
+   nosendfd,
+   norecvfd,
+};
+
+static void do_receiver(enum testtype type, int sock);
+static void do_sender(enum testtype type, int sock, int fd);
+__dead static void usage();
+
+
+static void
+do_receiver(enum testtype type, int sock)
+{
+   struct msghdr msg;
+   struct 

Re: c99 initialize struct protosw

[Alexander Bluhm]

On Tue, Feb 21, 2017 at 11:21:15AM -0500, David Hill wrote:
> Here is an updated diff without explicitly setting 0/NULL.
> 
> --- kern/uipc_proto.c 5 Feb 2017 07:57:08 -   1.11
> +++ kern/uipc_proto.c 21 Feb 2017 00:42:46 -
> +{
> +  .pr_type   = SOCK_STREAM,
> +  .pr_domain = ,
> +  .pr_protocol  = PF_LOCAL,
> +  .pr_flags  = PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
> +  .pr_usrreq = uipc_usrreq,
>  },

There must be a tab instead of space after pr_protocol.

OK bluhm@

Re: poll(2) vs NET_LOCK()

[Alexander Bluhm]

On Tue, Feb 21, 2017 at 12:07:59PM +0100, Martin Pieuchot wrote:
> Do not grab the NET_LOCK() when polling on unix domain sockets.  This
> was the reason for the "X freeze" reported by pirofti@.
> 
> ok?

OK bluhm@

> 
> Index: kern/sys_socket.c
> ===
> RCS file: /cvs/src/sys/kern/sys_socket.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 sys_socket.c
> --- kern/sys_socket.c 14 Feb 2017 09:46:21 -  1.29
> +++ kern/sys_socket.c 21 Feb 2017 11:00:16 -
> @@ -142,7 +142,7 @@ soo_poll(struct file *fp, int events, st
>   int revents = 0;
>   int s;
>  
> - NET_LOCK(s);
> + s = solock(so);
>   if (events & (POLLIN | POLLRDNORM)) {
>   if (soreadable(so))
>   revents |= events & (POLLIN | POLLRDNORM);
> @@ -168,7 +168,7 @@ soo_poll(struct file *fp, int events, st
>   so->so_snd.sb_flagsintr |= SB_SEL;
>   }
>   }
> - NET_UNLOCK(s);
> + sounlock(s);
>   return (revents);
>  }
>  

Re: make sosetopt responsible for m_free

[David Hill]

On Mon, Feb 06, 2017 at 01:16:45PM +0100, Martin Pieuchot wrote:
> On 03/02/17(Fri) 11:02, David Hill wrote:
> > On Fri, Feb 03, 2017 at 09:50:40AM +0100, Martin Pieuchot wrote:
> > > On 02/02/17(Thu) 12:12, David Hill wrote:
> > > > On Thu, Feb 02, 2017 at 09:34:07AM +0100, Martin Pieuchot wrote:
> > > > > On 01/02/17(Wed) 19:27, David Hill wrote:
> > > > > > Hello -
> > > > > > 
> > > > > > This diff makes sosetopt responsible for m_free which is much 
> > > > > > simpler.
> > > > > > Requested by bluhm@ 
> > > > > 
> > > > > I'd suggest to move the m_free(9) calls to sys_setsockopt().  This
> > > > > simplifies the existing code even more and will make it easier to use
> > > > > the stack for this temporary storage.
> > > > > 
> > > > 
> > > > New diff with mpi@'s suggestion. 
> > > 
> > > You forgot NFS and BFD that should now call m_free(9) after sosetopt(9). 
> > >
> > 
> > Indeed!  Now with BFD and NFS...
> 
> You're introducing a use after-free in ip_pcbopts().  You need to
> allocate/copy the mbuf there.
> 
> I must say I'm a bit afraid of this change because the amount of code it
> touches.  There might be another use after free somewhere that I missed.
> 
> Maybe we should first split our huge *ctloutput functions.  
> 
> One easy move is to split setopt/getopt.
> 
> Then introduce more per-protocol functions instead of having everything
> in ip{,6}_ctloutput().
> 
> For example move all the IPSEC craziness out of these functions.  Same
> thing with ICMP6...  This might sound superfluous but it will help
> if/when we decide to have a fine graining for different subsystems.
> 
> I'd also suggest to change the 'struct protosw' declaration to use C99
> initializer.  So we can have:
> 
>   .pr_ctloutput = ipsec_ctloutput
> 
> This would allow us to grep for "pr_ctloutput" (or pr_setopt) and know
> directly which functions to review.
>

If you are OK with first splitting each *ctloutput into *getopt/*setopt,
I will send each diff individually to make review easier.

Here is the first split, route_ctloutput.

Index: net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.222
diff -u -p -r1.222 rtsock.c
--- net/rtsock.c1 Feb 2017 20:59:47 -   1.222
+++ net/rtsock.c21 Feb 2017 15:52:07 -
@@ -98,6 +98,8 @@ struct walkarg {
caddr_t w_where, w_tmem;
 };
 
+introute_getopt(struct socket *, int, int, struct mbuf *);
+introute_setopt(struct socket *, int, int, struct mbuf *);
 introute_ctloutput(int, struct socket *, int, int, struct mbuf *);
 void   route_input(struct mbuf *m0, sa_family_t);
 introute_arp_conflict(struct rtentry *, struct rt_addrinfo *);
@@ -233,62 +235,86 @@ route_usrreq(struct socket *so, int req,
 }
 
 int
-route_ctloutput(int op, struct socket *so, int level, int optname,
-struct mbuf *m)
+route_getopt(struct socket *so, int level, int optname, struct mbuf *m)
+{
+   struct routecb *rop = sotoroutecb(so);
+   int error = 0;
+
+   if (level != AF_ROUTE)
+   return EINVAL;
+
+   switch (optname) {
+   case ROUTE_MSGFILTER:
+   m->m_len = sizeof(unsigned int);
+   *mtod(m, unsigned int *) = rop->msgfilter;
+   break;
+   case ROUTE_TABLEFILTER:
+   m->m_len = sizeof(unsigned int);
+   *mtod(m, unsigned int *) = rop->rtableid;
+   break;
+   default:
+   error = ENOPROTOOPT;
+   break;
+   }
+   return error;
+}
+
+int
+route_setopt(struct socket *so, int level, int optname, struct mbuf *m)
 {
struct routecb *rop = sotoroutecb(so);
int error = 0;
unsigned int tid;
 
if (level != AF_ROUTE) {
-   error = EINVAL;
-   if (op == PRCO_SETOPT && m)
-   m_free(m);
-   return (error);
+   m_free(m);
+   return EINVAL;
}
 
-   switch (op) {
-   case PRCO_SETOPT:
-   switch (optname) {
-   case ROUTE_MSGFILTER:
-   if (m == NULL || m->m_len != sizeof(unsigned int))
-   error = EINVAL;
-   else
-   rop->msgfilter = *mtod(m, unsigned int *);
-   break;
-   case ROUTE_TABLEFILTER:
-   if (m == NULL || m->m_len != sizeof(unsigned int)) {
-   error = EINVAL;
-   break;
-   }
-   tid = *mtod(m, unsigned int *);
-   if (tid != RTABLE_ANY && !rtable_exists(tid))
-   error = ENOENT;
-   else
-   rop->rtableid = tid;
-   break;
-   default:
-   error = ENOPROTOOPT;
+   switch (optname) {
+  

Re: c99 initialize struct protosw

[David Hill]

Here is an updated diff without explicitly setting 0/NULL.

Index: kern/uipc_proto.c
===
RCS file: /cvs/src/sys/kern/uipc_proto.c,v
retrieving revision 1.11
diff -u -p -r1.11 uipc_proto.c
--- kern/uipc_proto.c   5 Feb 2017 07:57:08 -   1.11
+++ kern/uipc_proto.c   21 Feb 2017 00:42:46 -
@@ -49,20 +49,26 @@
 extern struct domain unixdomain;   /* or at least forward */
 
 struct protosw unixsw[] = {
-{ SOCK_STREAM, ,PF_LOCAL,   
PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
-  0,   0,  0,  0,
-  uipc_usrreq,
-  0,   0,  0,  0,
+{
+  .pr_type = SOCK_STREAM,
+  .pr_domain   = ,
+  .pr_protocol  = PF_LOCAL,
+  .pr_flags= PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
+  .pr_usrreq   = uipc_usrreq,
 },
-{ SOCK_SEQPACKET,,  PF_LOCAL,   
PR_ATOMIC|PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
-  0,   0,  0,  0,
-  uipc_usrreq,
-  0,   0,  0,  0,
+{
+  .pr_type = SOCK_SEQPACKET,
+  .pr_domain   = ,
+  .pr_protocol = PF_LOCAL,
+  .pr_flags= PR_ATOMIC|PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
+  .pr_usrreq   = uipc_usrreq,
 },
-{ SOCK_DGRAM,  ,PF_LOCAL,   PR_ATOMIC|PR_ADDR|PR_RIGHTS,
-  0,   0,  0,  0,
-  uipc_usrreq,
-  0,   0,  0,  0,
+{
+  .pr_type = SOCK_DGRAM,
+  .pr_domain   = ,
+  .pr_protocol = PF_LOCAL,
+  .pr_flags= PR_ATOMIC|PR_ADDR|PR_RIGHTS,
+  .pr_usrreq   = uipc_usrreq,
 }
 };
 
Index: net/pfkey.c
===
RCS file: /cvs/src/sys/net/pfkey.c,v
retrieving revision 1.36
diff -u -p -r1.36 pfkey.c
--- net/pfkey.c 24 Jan 2017 10:08:30 -  1.36
+++ net/pfkey.c 21 Feb 2017 00:42:46 -
@@ -266,20 +266,12 @@ struct domain pfkeydomain = {
 };
 
 static struct protosw pfkey_protosw_template = {
-   SOCK_RAW,
-   ,
-   -1, /* protocol */
-   PR_ATOMIC | PR_ADDR,
-   NULL, /* input */
-   (void *) pfkey_output,
-   NULL, /* ctlinput */
-   NULL, /* ctloutput */
-   pfkey_usrreq,
-   NULL, /* init */
-   NULL, /* fasttimo */
-   NULL, /* slowtimo */
-   NULL, /* drain */
-   NULL/* sysctl */
+  .pr_type = SOCK_RAW,
+  .pr_domain   = ,
+  .pr_protocol = -1,
+  .pr_flags= PR_ATOMIC | PR_ADDR,
+  .pr_output   = (void *) pfkey_output,
+  .pr_usrreq   = pfkey_usrreq
 };
 
 int
Index: net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.222
diff -u -p -r1.222 rtsock.c
--- net/rtsock.c1 Feb 2017 20:59:47 -   1.222
+++ net/rtsock.c21 Feb 2017 00:42:47 -
@@ -1654,11 +1654,15 @@ sysctl_rtable_rtstat(void *oldp, size_t 
 extern struct domain routedomain;  /* or at least forward */
 
 struct protosw routesw[] = {
-{ SOCK_RAW,,   0,  PR_ATOMIC|PR_ADDR|PR_WANTRCVD,
-  0,   route_output,   0,  route_ctloutput,
-  route_usrreq,
-  raw_init,0,  0,  0,
-  sysctl_rtable,
+{
+  .pr_type = SOCK_RAW,
+  .pr_domain   = ,
+  .pr_flags= PR_ATOMIC|PR_ADDR|PR_WANTRCVD,
+  .pr_output   = route_output,
+  .pr_ctloutput= route_ctloutput,
+  .pr_usrreq   = route_usrreq,
+  .pr_init = raw_init,
+  .pr_sysctl   = sysctl_rtable
 }
 };
 
Index: netinet/in_proto.c
===
RCS file: /cvs/src/sys/netinet/in_proto.c,v
retrieving revision 1.72
diff -u -p -r1.72 in_proto.c
--- netinet/in_proto.c  29 Jan 2017 19:58:47 -  1.72
+++ netinet/in_proto.c  21 Feb 2017 00:42:47 -
@@ -175,138 +175,267 @@
 u_char ip_protox[IPPROTO_MAX];
 
 struct protosw inetsw[] = {
-{ 0,   ,0,  0,
-  0,   0,  0,  0,
-  0,
-  ip_init, 0,  ip_slowtimo,ip_drain,   ip_sysctl
-},
-{ SOCK_DGRAM,  ,IPPROTO_UDP,PR_ATOMIC|PR_ADDR|PR_SPLICE,
-  udp_input,   0,  udp_ctlinput,   ip_ctloutput,
-  udp_usrreq,
-  udp_init,0,  0,  0,  udp_sysctl
-},
-{ SOCK_STREAM, ,IPPROTO_TCP,
PR_CONNREQUIRED|PR_WANTRCVD|PR_ABRTACPTDIS|PR_SPLICE,
-  tcp_input,   0,  tcp_ctlinput,   tcp_ctloutput,
-  tcp_usrreq,
-  tcp_init,0,  tcp_slowtimo,   0,  tcp_sysctl
-},
-{ SOCK_RAW,,IPPROTO_RAW,PR_ATOMIC|PR_ADDR,
-  rip_input,   rip_output, 0,  rip_ctloutput,
-  rip_usrreq,
-  0,   0,  0,  0,
-},
-{ SOCK_RAW,,IPPROTO_ICMP,   PR_ATOMIC|PR_ADDR,
-  icmp_input,  rip_output, 0,  rip_ctloutput,
-  rip_usrreq,
-  icmp_init,   0,  0,  0,  icmp_sysctl
+{
+  .pr_domain   = ,
+  .pr_init = ip_init,
+  .pr_slowtimo = 

Re: NFS splsoftnet()

[Martin Pieuchot]

On 13/02/17(Mon) 12:21, Martin Pieuchot wrote:
> Network processing is not longer done in soft-interrupt context.  That
> means that processes doing syscalls no longer need to raise the IPL
> level to guarantee consistency, the KERNEL_LOCK() is enough.
> 
> Diff below kills two unnecessary splsoftnet()/splx(), ok? 

Anyone?

> Index: nfs/nfs_syscalls.c
> ===
> RCS file: /cvs/src/sys/nfs/nfs_syscalls.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 nfs_syscalls.c
> --- nfs/nfs_syscalls.c15 Sep 2016 02:00:18 -  1.106
> +++ nfs/nfs_syscalls.c13 Feb 2017 11:15:49 -
> @@ -221,7 +221,7 @@ nfssvc_addsock(struct file *fp, struct m
>   struct nfssvc_sock *slp;
>   struct socket *so;
>   struct nfssvc_sock *tslp;
> - int error, s;
> + int error;
>  
>   so = (struct socket *)fp->f_data;
>   tslp = NULL;
> @@ -278,12 +278,10 @@ nfssvc_addsock(struct file *fp, struct m
>   slp->ns_nam = mynam;
>   fp->f_count++;
>   slp->ns_fp = fp;
> - s = splsoftnet();
>   so->so_upcallarg = (caddr_t)slp;
>   so->so_upcall = nfsrv_rcv;
>   slp->ns_flag = (SLP_VALID | SLP_NEEDQ);
>   nfsrv_wakenfsd(slp);
> - splx(s);
>   return (0);
>  }
>  
> Index: nfs/nfs_vfsops.c
> ===
> RCS file: /cvs/src/sys/nfs/nfs_vfsops.c,v
> retrieving revision 1.112
> diff -u -p -r1.112 nfs_vfsops.c
> --- nfs/nfs_vfsops.c  15 Nov 2016 13:46:54 -  1.112
> +++ nfs/nfs_vfsops.c  13 Feb 2017 11:17:52 -
> @@ -74,6 +74,8 @@ extern u_int32_t nfs_procids[NFS_NPROCS]
>  int  nfs_sysctl(int *, u_int, void *, size_t *, void *, size_t, 
> struct proc *);
>  int  nfs_checkexp(struct mount *, struct mbuf *, int *, struct ucred 
> **);
>  struct mount *nfs_mount_diskless(struct nfs_dlmount *, char *, int);
> +void nfs_decode_args(struct nfsmount *, struct nfs_args *,
> + struct nfs_args *);
>  
>  /*
>   * nfs vfs operations.
> @@ -389,12 +391,9 @@ void
>  nfs_decode_args(struct nfsmount *nmp, struct nfs_args *argp,
>  struct nfs_args *nargp)
>  {
> - int s;
>   int adjsock = 0;
>   int maxio;
>  
> - s = splsoftnet();
> -
>  #if 0
>   /* Re-bind if rsrvd port requested and wasn't on one */
>   adjsock = !(nmp->nm_flag & NFSMNT_RESVPORT)
> @@ -407,7 +406,6 @@ nfs_decode_args(struct nfsmount *nmp, st
>   /* Update flags atomically.  Don't change the lock bits. */
>   nmp->nm_flag =
>   (argp->flags & ~NFSMNT_INTERNAL) | (nmp->nm_flag & NFSMNT_INTERNAL);
> - splx(s);
>  
>   if ((argp->flags & NFSMNT_TIMEO) && argp->timeo > 0) {
>   nmp->nm_timeo = (argp->timeo * NFS_HZ + 5) / 10;
> Index: nfs/nfsmount.h
> ===
> RCS file: /cvs/src/sys/nfs/nfsmount.h,v
> retrieving revision 1.26
> diff -u -p -r1.26 nfsmount.h
> --- nfs/nfsmount.h27 Sep 2016 01:37:38 -  1.26
> +++ nfs/nfsmount.h13 Feb 2017 11:17:02 -
> @@ -89,8 +89,6 @@ int nfs_mount(struct mount *, const char
>  int  mountnfs(struct nfs_args *, struct mount *, struct mbuf *,
>   const char *, char *);
>  int  nfs_mountroot(void);
> -void nfs_decode_args(struct nfsmount *, struct nfs_args *,
> - struct nfs_args *);
>  int  nfs_start(struct mount *, int, struct proc *);
>  int  nfs_unmount(struct mount *, int, struct proc *);
>  int  nfs_root(struct mount *, struct vnode **);
> 

Re: pflow(4) percpu counters

[Jeremie Courreges-Anglas]

Jeremie Courreges-Anglas  writes:

> Florian Obser  writes:
>
>> On Sat, Feb 18, 2017 at 06:06:01PM +0100, Jeremie Courreges-Anglas wrote:
>>> 
>>> This one is a bit weird, the driver doesn't just increment the stats but
>>> also uses them at runtime, hence the additional helper functions.
>>
>> I'm wondering if we should just drop the reading.
>> We have two cases, the init case and the packet sending case.
>> First the sending case:
>> Isn't this always true?
>>  if (pflowstats.pflow_flows == sc->sc_gcounter)
>> If yes we can just skip that and do the inc.

If it not true if you have multiple interfaces.

>> The init case tries to preserve the flow counter betwen ifdown/ifup

I don't think it is just about preserving an interface's counter.

>> Maybe we should just init the global counter to 0, like on reboot.
>> Benno?
>
>   revision 1.9
>   date: 2009/01/03 21:47:32;  author: gollo;  state: Exp;  lines: +11 -7;
>   sync flow sequence ids on all used pflow interfaces.
>
> Right now I can't tell whether this change makes little or a lot of
> sense, so I'd better not touch this.  :)

Thinking about it some more, this is probably desirable.  After all, it
means that netstat -s shows a flow count that matches the sequence seen
by each collector.  On the other hand, removing this means that the
counters API is enough to handle plow_flows.  Thoughts?

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

Re: poll(2) vs NET_LOCK()

[Theo Buehler]

On Tue, Feb 21, 2017 at 12:07:59PM +0100, Martin Pieuchot wrote:
> Do not grab the NET_LOCK() when polling on unix domain sockets.  This
> was the reason for the "X freeze" reported by pirofti@.
> 
> ok?

Yes, indeed, this fixes the freeze on both, my iwn and my iwm.

ok

> 
> Index: kern/sys_socket.c
> ===
> RCS file: /cvs/src/sys/kern/sys_socket.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 sys_socket.c
> --- kern/sys_socket.c 14 Feb 2017 09:46:21 -  1.29
> +++ kern/sys_socket.c 21 Feb 2017 11:00:16 -
> @@ -142,7 +142,7 @@ soo_poll(struct file *fp, int events, st
>   int revents = 0;
>   int s;
>  
> - NET_LOCK(s);
> + s = solock(so);
>   if (events & (POLLIN | POLLRDNORM)) {
>   if (soreadable(so))
>   revents |= events & (POLLIN | POLLRDNORM);
> @@ -168,7 +168,7 @@ soo_poll(struct file *fp, int events, st
>   so->so_snd.sb_flagsintr |= SB_SEL;
>   }
>   }
> - NET_UNLOCK(s);
> + sounlock(s);
>   return (revents);
>  }
>  

poll(2) vs NET_LOCK()

[Martin Pieuchot]

Do not grab the NET_LOCK() when polling on unix domain sockets.  This
was the reason for the "X freeze" reported by pirofti@.

ok?

Index: kern/sys_socket.c
===
RCS file: /cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.29
diff -u -p -r1.29 sys_socket.c
--- kern/sys_socket.c   14 Feb 2017 09:46:21 -  1.29
+++ kern/sys_socket.c   21 Feb 2017 11:00:16 -
@@ -142,7 +142,7 @@ soo_poll(struct file *fp, int events, st
int revents = 0;
int s;
 
-   NET_LOCK(s);
+   s = solock(so);
if (events & (POLLIN | POLLRDNORM)) {
if (soreadable(so))
revents |= events & (POLLIN | POLLRDNORM);
@@ -168,7 +168,7 @@ soo_poll(struct file *fp, int events, st
so->so_snd.sb_flagsintr |= SB_SEL;
}
}
-   NET_UNLOCK(s);
+   sounlock(s);
return (revents);
 }
 

Re: [PATCH] bc(1) should write error messages to standard error

[Otto Moerbeek]

On Tue, Feb 21, 2017 at 07:19:13AM +0100, Otto Moerbeek wrote:

> On Tue, Feb 21, 2017 at 07:00:34AM +0100, Otto Moerbeek wrote:
> 
> > On Tue, Feb 21, 2017 at 04:08:57AM +0100, Martijn Dekker wrote:
> > 
> > > Upon encountering a parsing error, bc(1) passes an error message on to
> > > dc(1), which writes the error message to standard output along with the
> > > normal output.
> > > 
> > > That is a bug. Error messages should go to standard error instead, as
> > > POSIX specifies:
> > > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/bc.html#tag_20_09_10
> > > 
> > > GNU 'bc' and Solaris 'bc' act like POSIX says and write error messages
> > > to standard error.
> > > 
> > > Bizarrely, the exit status of bc(1) is left unspecified:
> > > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/bc.html#tag_20_09_18
> > > And indeed, all versions of 'bc' exit with status 0 if there is an input
> > > error such as a parsing error, so the exit status cannot be used to
> > > catch it. That leaves examining standard error as the only method for a
> > > program calling bc(1), such as a shell script, to distinguish between an
> > > error state and normal operation. That is, with this bug, there is no
> > > way at all.
> > > 
> > > The following example shell function transparently hardens bc(1) by
> > > intercepting standard error and exiting the program or subshell if an
> > > error was produced.
> > > 
> > > bc() {
> > >   _bc_err=$(command -p bc "$@" 1>&3 2>&1)
> > >   [ -z "${_bc_err}" ] && return
> > >   printf '%s\n' "$0: bc(1) caught errors:" "${_bc_err}" 1>&2
> > >   exit 125
> > > } 3>&1
> > > 
> > > The patch below fixes bc(1) so error messages are written directly to
> > > standard error and the above shell function works as expected. As a side
> > > effect, yyerror() is simplified.
> > > 
> > > Another side effect is that bc(1) error messages are no longer neatly
> > > included in the generated dc(1) source when debugging it using 'bc -c'.
> > > But I don't think that is actually a problem; they are just printed to
> > > standard error instead. In fact, the patch makes 'bc -c' act like
> > > Solaris. If others find this problematic, the patch could be extended to
> > > restore the old behaviour only if '-c' is given.
> > > 
> > > The manual page does not document error message behaviour one way or
> > > another. Since the patch implements standard behaviour, no change seems
> > > necessary there.
> > > 
> > > Thanks,
> > > 
> > > - M.
> > > 
> > 
> > Thanks for the diff. I am now wondering why I wrote it this way
> > Likely beacuse the original bc had a similar approach.
> > Anyway, I'll try to look at this the coming days,
> 
> Indeed, 4.4BSD bc does this:
> 
> yyerror( s ) char *s; {
>   if(ifile > sargc)ss="teletype";
>   printf("c[%s on line %d, %s]pc\n", s ,ln+1,ss);
>   fflush(stdout);
>   cp = cary;
>   crs = rcrs;
>   bindx = 0;
>   lev = 0;
>   b_sp_nxt = _space[0];
> }
> 
> My original goal was to make a bc that produced the same dc commands
> as the reference implementation I used.
> 
> You can now see that your diff skips the 'c' commands an in that
> changes behaviour. Pondering if introducing a way to write to
> stderr in dc(1) would be better...
> 
>   -Otto

This should fix the problem and keep the code more close to the
reference implementation. Also, I noticed an omission in the
determination if we're interactive or not, so I fixed that as well.

-Otto


Index: dc/bcode.c
===
RCS file: /cvs/src/usr.bin/dc/bcode.c,v
retrieving revision 1.49
diff -u -p -r1.49 bcode.c
--- dc/bcode.c  27 Mar 2016 15:55:13 -  1.49
+++ dc/bcode.c  21 Feb 2017 10:25:41 -
@@ -67,6 +67,7 @@ static __inline struct number *pop_numbe
 static __inline char   *pop_string(void);
 static __inline void   clear_stack(void);
 static __inline void   print_tos(void);
+static voidprint_err(void);
 static voidpop_print(void);
 static voidpop_printn(void);
 static __inline void   print_stack(void);
@@ -195,6 +196,7 @@ static const struct jump_entry jump_tabl
{ 'a',  to_ascii},
{ 'c',  clear_stack },
{ 'd',  dup },
+   { 'e',  print_err   },
{ 'f',  print_stack },
{ 'i',  set_ibase   },
{ 'k',  set_scale   },
@@ -491,6 +493,18 @@ print_tos(void)
if (value != NULL) {
print_value(stdout, value, "", bmachine.obase);
(void)putchar('\n');
+   }
+   else
+   warnx("stack empty");
+}
+
+static void
+print_err(void)
+{
+   struct value *value = tos();
+   if (value != NULL) {
+   print_value(stderr, value, "", bmachine.obase);
+   (void)putc('\n', stderr);
}
else
warnx("stack empty");
Index: dc/dc.1

Re: c99 initialize struct protosw

[Martin Pieuchot]

On 20/02/17(Mon) 22:30, Mark Kettenis wrote:
> David Hill schreef op 2017-02-19 03:22:
> > Hello -
> > 
> > This moves the 'struct protosw' declarations to use C99 initializers.
> > Requested by mpi@
> 
> With C99 initializers it is no longer necessary to explicitly
> initialize zero-initialized members (such as null-pointers).
> That could reduce the diff considerably and perhaps make it
> more swallowable for folks.

Yes please.  Since the goal is to reduce the number of "pr_*"
occurrences not having the NULL lines would help even more.

> > Index: kern/uipc_proto.c
> > ===
> > RCS file: /cvs/src/sys/kern/uipc_proto.c,v
> > retrieving revision 1.11
> > diff -u -p -r1.11 uipc_proto.c
> > --- kern/uipc_proto.c   5 Feb 2017 07:57:08 -   1.11
> > +++ kern/uipc_proto.c   19 Feb 2017 02:15:00 -
> > @@ -49,20 +49,53 @@
> >  extern struct domain unixdomain;   /* or at least forward 
> > */
> > 
> >  struct protosw unixsw[] = {
> > -{
> > SOCK_STREAM,,PF_LOCAL,   
> > PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
> > -  0,   0,  0,  0,
> > -  uipc_usrreq,
> > -  0,   0,  0,  0,
> > +{
> > +  .pr_type = SOCK_STREAM,
> > +  .pr_domain   = ,
> > +  .pr_protocol  = PF_LOCAL,
> > +  .pr_flags= PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
> > +  .pr_input= NULL,
> > +  .pr_output   = NULL,
> > +  .pr_ctlinput = NULL,
> > +  .pr_ctloutput= NULL,
> > +  .pr_usrreq   = uipc_usrreq,
> > +  .pr_init = NULL,
> > +  .pr_fasttimo = NULL,
> > +  .pr_slowtimo = NULL,
> > +  .pr_drain= NULL,
> > +  .pr_sysctl   = NULL
> >  },
> > -{
> > SOCK_SEQPACKET,, PF_LOCAL,   
> > PR_ATOMIC|PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
> > -  0,   0,  0,  0,
> > -  uipc_usrreq,
> > -  0,   0,  0,  0,
> > +{
> > +  .pr_type = SOCK_SEQPACKET,
> > +  .pr_domain   = ,
> > +  .pr_protocol = PF_LOCAL,
> > +  .pr_flags= PR_ATOMIC|PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
> > +  .pr_input= NULL,
> > +  .pr_output   = NULL,
> > +  .pr_ctlinput = NULL,
> > +  .pr_ctloutput= NULL,
> > +  .pr_usrreq   = uipc_usrreq,
> > +  .pr_init = NULL,
> > +  .pr_fasttimo = NULL,
> > +  .pr_slowtimo = NULL,
> > +  .pr_drain= NULL,
> > +  .pr_sysctl   = NULL
> >  },
> > -{ SOCK_DGRAM,  ,PF_LOCAL,   
> > PR_ATOMIC|PR_ADDR|PR_RIGHTS,
> > -  0,   0,  0,  0,
> > -  uipc_usrreq,
> > -  0,   0,  0,  0,
> > +{
> > +  .pr_type = SOCK_DGRAM,
> > +  .pr_domain   = ,
> > +  .pr_protocol = PF_LOCAL,
> > +  .pr_flags= PR_ATOMIC|PR_ADDR|PR_RIGHTS,
> > +  .pr_input= NULL,
> > +  .pr_output   = NULL,
> > +  .pr_ctlinput = NULL,
> > +  .pr_ctloutput= NULL,
> > +  .pr_usrreq   = uipc_usrreq,
> > +  .pr_init = NULL,
> > +  .pr_fasttimo = NULL,
> > +  .pr_slowtimo = NULL,
> > +  .pr_drain= NULL,
> > +  .pr_sysctl   = NULL
> >  }
> >  };
> > 
> > Index: net/pfkey.c
> > ===
> > RCS file: /cvs/src/sys/net/pfkey.c,v
> > retrieving revision 1.36
> > diff -u -p -r1.36 pfkey.c
> > --- net/pfkey.c 24 Jan 2017 10:08:30 -  1.36
> > +++ net/pfkey.c 19 Feb 2017 02:15:00 -
> > @@ -266,20 +266,20 @@ struct domain pfkeydomain = {
> >  };
> > 
> >  static struct protosw pfkey_protosw_template = {
> > -   SOCK_RAW,
> > -   ,
> > -   -1, /* protocol */
> > -   PR_ATOMIC | PR_ADDR,
> > -   NULL, /* input */
> > -   (void *) pfkey_output,
> > -   NULL, /* ctlinput */
> > -   NULL, /* ctloutput */
> > -   pfkey_usrreq,
> > -   NULL, /* init */
> > -   NULL, /* fasttimo */
> > -   NULL, /* slowtimo */
> > -   NULL, /* drain */
> > -   NULL/* sysctl */
> > +  .pr_type = SOCK_RAW,
> > +  .pr_domain   = ,
> > +  .pr_protocol = -1,
> > +  .pr_flags= PR_ATOMIC | PR_ADDR,
> > +  .pr_input= NULL,
> > +  .pr_output   = (void *) pfkey_output,
> > +  .pr_ctlinput = NULL,
> > +  .pr_ctloutput= NULL,
> > +  .pr_usrreq   = pfkey_usrreq,
> > +  .pr_init = NULL,
> > +  .pr_fasttimo = NULL,
> > +  .pr_slowtimo = NULL,
> > +  .pr_drain= NULL,
> > +  .pr_sysctl   = NULL
> >  };
> > 
> >  int
> > Index: net/rtsock.c
> > ===
> > RCS file: /cvs/src/sys/net/rtsock.c,v
> > retrieving revision 1.222
> > diff -u -p -r1.222 rtsock.c
> > --- net/rtsock.c1 Feb 2017 20:59:47 -   1.222
> > +++ net/rtsock.c19 Feb 2017 02:15:00 -
> > @@ -1654,13 +1654,23 @@ sysctl_rtable_rtstat(void *oldp, size_t
> >  extern struct domain routedomain;  /* or at least forward 
> > */
> > 
> >  struct protosw routesw[] = 

Re: npppd: reload enables stripping NT domains on radius

[YASUOKA Masahiko]

On Tue, 21 Feb 2017 08:31:13 +0100
Patrick Wildt  wrote:
> On Tue, Feb 21, 2017 at 02:11:05PM +0900, YASUOKA Masahiko wrote:
>> On Mon, 20 Feb 2017 11:38:19 +0100
>> Patrick Wildt  wrote:
>> > when using RADIUS, the NT domains should not be stripped from the
>> > username.
>> 
>> I suppose it depends on the use-case.
>> 
>> npppd.conf(5) mentions "strip-nt-domain" is "yes" by default and
>> adding "strip-nt-domain no" in "authentication  type radius"
>> section of npppd.conf should be able to change that behavior.
>> 
>>   authentication RADIUS type radius {
>>   strip-nt-domain no
>>   authentication-server {
>>:
(snip)
> I like consistency, so this is better.  While there, please adjust the
> manpage, since it is wrong about strip-nt-domain's default value.

Oops.  You are right.  Thanks.

--yasuoka