Recursive splsoftnet() in in6_ifattach_linklocal()
Get rid of them, ok? Index: netinet6/in6_ifattach.c === RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v retrieving revision 1.100 diff -u -p -r1.100 in6_ifattach.c --- netinet6/in6_ifattach.c 30 Jun 2016 08:19:03 - 1.100 +++ netinet6/in6_ifattach.c 20 Dec 2016 10:58:33 - @@ -294,7 +294,9 @@ in6_ifattach_linklocal(struct ifnet *ifp { struct in6_aliasreq ifra; struct in6_ifaddr *ia6; - int s, error, flags; + int error, flags; + + splsoftassert(IPL_SOFTNET); /* * configure link-local address. @@ -338,9 +340,7 @@ in6_ifattach_linklocal(struct ifnet *ifp if (in6if_do_dad(ifp) && ((ifp->if_flags & IFF_POINTOPOINT) == 0)) ifra.ifra_flags |= IN6_IFF_TENTATIVE; - s = splsoftnet(); error = in6_update_ifa(ifp, &ifra, in6ifa_ifpforlinklocal(ifp, 0)); - splx(s); if (error != 0) return (error); @@ -359,15 +359,12 @@ in6_ifattach_linklocal(struct ifnet *ifp if ((ifp->if_flags & IFF_POINTOPOINT) == 0) flags |= RTF_CLONING; - s = splsoftnet(); error = rt_ifa_add(&ia6->ia_ifa, flags, ia6->ia_ifa.ifa_addr); if (error) { in6_purgeaddr(&ia6->ia_ifa); - splx(s); return (error); } dohooks(ifp->if_addrhooks, 0); - splx(s); return (0); }
Re: if attach/detach netlocks
On 29/12/16(Thu) 01:15, Alexander Bluhm wrote: > On Fri, Dec 23, 2016 at 12:09:32AM +0100, Martin Pieuchot wrote: > > On 22/12/16(Thu) 20:45, Mike Belopuhov wrote: > > > I think this is what is required here. Works here, but YMMV. > > > > splnet() in a pseudo-driver seems completely wrong, you could get rid of > > it. > > Yes, but that is another issue. Can we get the netlock splasserts > fixed first? This diff looks good to me. Sure I'm ok with the diff. > > > diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c > > > index e9bc1cb8305..dfb71cf9467 100644 > > > --- sys/net/if_vxlan.c > > > +++ sys/net/if_vxlan.c > > > @@ -178,13 +178,15 @@ int > > > vxlan_clone_destroy(struct ifnet *ifp) > > > { > > > struct vxlan_softc *sc = ifp->if_softc; > > > int s; > > > > > > + NET_LOCK(s); > > > s = splnet(); > > > vxlan_multicast_cleanup(ifp); > > > splx(s); > > > + NET_UNLOCK(s); > > > > > > vxlan_enable--; > > > LIST_REMOVE(sc, sc_entry); > > > > > > ifmedia_delete_instance(&sc->sc_media, IFM_INST_ANY); >
Re: splassert with pppd
On 23/12/16(Fri) 16:33, Stefan Sperling wrote: > When I kill pppd, running on top of umsm(4), I see the following splasserts: Thanks for the report. > End of stack trace. > splassert: sorwakeup: want 1 have 0 > Starting stack trace... > sorwakeup(1,0,d09d9be1,0,9cee1169) at sorwakeup+0x3f > sorwakeup(d90f89f8,d0b58b50,d900c600,0,d0d31bb8) at sorwakeup+0x3f > route_input(d900c600,d0b58b60,d0b58b50,d0b58b40,d09d086b) at route_input+0x26a > rt_ifmsg(d40ee000,d09dd1e7,d09dd1e7,d03baa39,80) at rt_ifmsg+0xb2 > if_linkstate(d40ee000,d09deb02,ffe9,f60d1bac,d03b0da5) at > if_linkstate+0x64 > pppdealloc(d40ee000,3,f60d1bec,d03b10f4,f60d1bd8) at pppdealloc+0x36 > pppclose(d40f8600,3,d90b8300,d9734e18,d9734e18) at pppclose+0x77 > ttioctl(d40f8600,8004741b,f60d1e74,3,d90b8300) at ttioctl+0x6e3 > ucom_do_ioctl(d40f7a00,8004741b,f60d1e74,3,d90b8300) at ucom_do_ioctl+0x81 > ucomioctl(4280,8004741b,f60d1e74,3,d90b8300,0,d96c9218,d03ec5d2,d96c91c8,d96c91c8,f60d1d6c,d03e899f) > at ucomioctl+0x6b > spec_ioctl(f60d1d60,8004667d,f60d1ef0,d960a180,8004741b) at spec_ioctl+0x9b > VOP_IOCTL(d960a180,8004741b,f60d1e74,3,d97a3c00) at VOP_IOCTL+0x4b > vn_ioctl(d90b51b8,8004741b,f60d1e74,d90b8300,27) at vn_ioctl+0x7e > sys_ioctl(d90b8300,f60d1f5c,f60d1f7c,0,286) at sys_ioctl+0x19f > syscall() at syscall+0x250 pppdealloc() is always called in process context, the splsoftassert() is currently valid because the function is called at spltty(), but now we need the NET_LOCK() so the diff below should do the trick. ok? Index: net/if_ppp.c === RCS file: /cvs/src/sys/net/if_ppp.c,v retrieving revision 1.102 diff -u -p -r1.102 if_ppp.c --- net/if_ppp.c16 Nov 2016 14:23:10 - 1.102 +++ net/if_ppp.c29 Dec 2016 08:37:19 - @@ -306,9 +306,9 @@ void pppdealloc(struct ppp_softc *sc) { struct ppp_pkt *pkt; + int s; - splsoftassert(IPL_SOFTNET); - + NET_LOCK(s); if_down(&sc->sc_if); sc->sc_if.if_flags &= ~(IFF_UP|IFF_RUNNING); sc->sc_devp = NULL; @@ -343,6 +343,7 @@ pppdealloc(struct ppp_softc *sc) sc->sc_comp = 0; } #endif + NET_UNLOCK(s); } /*
Re: Recursive splsoftnet() in in6_ifattach_linklocal()
On Thu, Dec 29, 2016 at 09:15:54AM +0100, Martin Pieuchot wrote: > Get rid of them, ok? OK bluhm@ > > Index: netinet6/in6_ifattach.c > === > RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v > retrieving revision 1.100 > diff -u -p -r1.100 in6_ifattach.c > --- netinet6/in6_ifattach.c 30 Jun 2016 08:19:03 - 1.100 > +++ netinet6/in6_ifattach.c 20 Dec 2016 10:58:33 - > @@ -294,7 +294,9 @@ in6_ifattach_linklocal(struct ifnet *ifp > { > struct in6_aliasreq ifra; > struct in6_ifaddr *ia6; > - int s, error, flags; > + int error, flags; > + > + splsoftassert(IPL_SOFTNET); > > /* >* configure link-local address. > @@ -338,9 +340,7 @@ in6_ifattach_linklocal(struct ifnet *ifp > if (in6if_do_dad(ifp) && ((ifp->if_flags & IFF_POINTOPOINT) == 0)) > ifra.ifra_flags |= IN6_IFF_TENTATIVE; > > - s = splsoftnet(); > error = in6_update_ifa(ifp, &ifra, in6ifa_ifpforlinklocal(ifp, 0)); > - splx(s); > if (error != 0) > return (error); > > @@ -359,15 +359,12 @@ in6_ifattach_linklocal(struct ifnet *ifp > if ((ifp->if_flags & IFF_POINTOPOINT) == 0) > flags |= RTF_CLONING; > > - s = splsoftnet(); > error = rt_ifa_add(&ia6->ia_ifa, flags, ia6->ia_ifa.ifa_addr); > if (error) { > in6_purgeaddr(&ia6->ia_ifa); > - splx(s); > return (error); > } > dohooks(ifp->if_addrhooks, 0); > - splx(s); > > return (0); > }
Re: splassert with pppd
On Thu, Dec 29, 2016 at 09:39:08AM +0100, Martin Pieuchot wrote: > On 23/12/16(Fri) 16:33, Stefan Sperling wrote: > > When I kill pppd, running on top of umsm(4), I see the following splasserts: > > Thanks for the report. > > > End of stack trace. > > splassert: sorwakeup: want 1 have 0 > > Starting stack trace... > > sorwakeup(1,0,d09d9be1,0,9cee1169) at sorwakeup+0x3f > > sorwakeup(d90f89f8,d0b58b50,d900c600,0,d0d31bb8) at sorwakeup+0x3f > > route_input(d900c600,d0b58b60,d0b58b50,d0b58b40,d09d086b) at > > route_input+0x26a > > rt_ifmsg(d40ee000,d09dd1e7,d09dd1e7,d03baa39,80) at rt_ifmsg+0xb2 > > if_linkstate(d40ee000,d09deb02,ffe9,f60d1bac,d03b0da5) at > > if_linkstate+0x64 > > pppdealloc(d40ee000,3,f60d1bec,d03b10f4,f60d1bd8) at pppdealloc+0x36 > > pppclose(d40f8600,3,d90b8300,d9734e18,d9734e18) at pppclose+0x77 > > ttioctl(d40f8600,8004741b,f60d1e74,3,d90b8300) at ttioctl+0x6e3 > > ucom_do_ioctl(d40f7a00,8004741b,f60d1e74,3,d90b8300) at ucom_do_ioctl+0x81 > > ucomioctl(4280,8004741b,f60d1e74,3,d90b8300,0,d96c9218,d03ec5d2,d96c91c8,d96c91c8,f60d1d6c,d03e899f) > > at ucomioctl+0x6b > > spec_ioctl(f60d1d60,8004667d,f60d1ef0,d960a180,8004741b) at spec_ioctl+0x9b > > VOP_IOCTL(d960a180,8004741b,f60d1e74,3,d97a3c00) at VOP_IOCTL+0x4b > > vn_ioctl(d90b51b8,8004741b,f60d1e74,d90b8300,27) at vn_ioctl+0x7e > > sys_ioctl(d90b8300,f60d1f5c,f60d1f7c,0,286) at sys_ioctl+0x19f > > syscall() at syscall+0x250 > > pppdealloc() is always called in process context, the splsoftassert() is > currently valid because the function is called at spltty(), but now we > need the NET_LOCK() so the diff below should do the trick. > > ok? OK bluhm@ > > Index: net/if_ppp.c > === > RCS file: /cvs/src/sys/net/if_ppp.c,v > retrieving revision 1.102 > diff -u -p -r1.102 if_ppp.c > --- net/if_ppp.c 16 Nov 2016 14:23:10 - 1.102 > +++ net/if_ppp.c 29 Dec 2016 08:37:19 - > @@ -306,9 +306,9 @@ void > pppdealloc(struct ppp_softc *sc) > { > struct ppp_pkt *pkt; > + int s; > > - splsoftassert(IPL_SOFTNET); > - > + NET_LOCK(s); > if_down(&sc->sc_if); > sc->sc_if.if_flags &= ~(IFF_UP|IFF_RUNNING); > sc->sc_devp = NULL; > @@ -343,6 +343,7 @@ pppdealloc(struct ppp_softc *sc) > sc->sc_comp = 0; > } > #endif > + NET_UNLOCK(s); > } > > /*
ntpd.conf
Markup a forgotten keyword. Jan Index: ntpd.conf.5 === RCS file: /cvs/src/usr.sbin/ntpd/ntpd.conf.5,v retrieving revision 1.33 diff -u -p -r1.33 ntpd.conf.5 --- ntpd.conf.5 23 Oct 2015 14:52:20 - 1.33 +++ ntpd.conf.5 28 Dec 2016 20:58:06 - @@ -127,7 +127,9 @@ sensor nmea0 refid GPS .Ed .Pp A stratum value other than the default of 1 can be assigned using -the stratum keyword. +the +.Ic stratum +keyword. .It Xo Ic server Ar address .Op Ic weight Ar weight-value .Xc
doas parse.y refinement
it occurs to me that arglist and envlist are the same thing, a strlist. Index: parse.y === RCS file: /cvs/src/usr.bin/doas/parse.y,v retrieving revision 1.25 diff -u -p -r1.25 parse.y --- parse.y 29 Dec 2016 19:12:42 - 1.25 +++ parse.y 29 Dec 2016 19:55:33 - @@ -36,6 +36,7 @@ typedef struct { const char **cmdargs; const char **envlist; }; + const char **strlist; const char *str; }; int lineno; @@ -141,21 +142,21 @@ option: TNOPASS { } | TKEEPENV { $$.options = KEEPENV; $$.envlist = NULL; - } | TSETENV '{' envlist '}' { + } | TSETENV '{' strlist '}' { $$.options = 0; - $$.envlist = $3.envlist; + $$.envlist = $3.strlist; } ; -envlist: /* empty */ { - if (!($$.envlist = calloc(1, sizeof(char * - errx(1, "can't allocate envlist"); - } | envlist TSTRING { - int nenv = arraylen($1.envlist); - if (!($$.envlist = reallocarray($1.envlist, nenv + 2, +strlist: /* empty */ { + if (!($$.strlist = calloc(1, sizeof(char * + errx(1, "can't allocate strlist"); + } | strlist TSTRING { + int nstr = arraylen($1.strlist); + if (!($$.strlist = reallocarray($1.strlist, nstr + 2, sizeof(char * - errx(1, "can't allocate envlist"); - $$.envlist[nenv] = $2.str; - $$.envlist[nenv + 1] = NULL; + errx(1, "can't allocate strlist"); + $$.strlist[nstr] = $2.str; + $$.strlist[nstr + 1] = NULL; } ; @@ -179,20 +180,8 @@ cmd: /* optional */ { args: /* empty */ { $$.cmdargs = NULL; - } | TARGS argslist { - $$.cmdargs = $2.cmdargs; - } ; - -argslist: /* empty */ { - if (!($$.cmdargs = calloc(1, sizeof(char * - errx(1, "can't allocate args"); - } | argslist TSTRING { - int nargs = arraylen($1.cmdargs); - if (!($$.cmdargs = reallocarray($1.cmdargs, nargs + 2, - sizeof(char * - errx(1, "can't allocate args"); - $$.cmdargs[nargs] = $2.str; - $$.cmdargs[nargs + 1] = NULL; + } | TARGS strlist { + $$.cmdargs = $2.strlist; } ; %%
Re: doas parse.y refinement
ok benno@ Ted Unangst(t...@tedunangst.com) on 2016.12.29 14:56:47 -0500: > it occurs to me that arglist and envlist are the same thing, a strlist. > > Index: parse.y > === > RCS file: /cvs/src/usr.bin/doas/parse.y,v > retrieving revision 1.25 > diff -u -p -r1.25 parse.y > --- parse.y 29 Dec 2016 19:12:42 - 1.25 > +++ parse.y 29 Dec 2016 19:55:33 - > @@ -36,6 +36,7 @@ typedef struct { > const char **cmdargs; > const char **envlist; > }; > + const char **strlist; > const char *str; > }; > int lineno; > @@ -141,21 +142,21 @@ option: TNOPASS { > } | TKEEPENV { > $$.options = KEEPENV; > $$.envlist = NULL; > - } | TSETENV '{' envlist '}' { > + } | TSETENV '{' strlist '}' { > $$.options = 0; > - $$.envlist = $3.envlist; > + $$.envlist = $3.strlist; > } ; > > -envlist: /* empty */ { > - if (!($$.envlist = calloc(1, sizeof(char * > - errx(1, "can't allocate envlist"); > - } | envlist TSTRING { > - int nenv = arraylen($1.envlist); > - if (!($$.envlist = reallocarray($1.envlist, nenv + 2, > +strlist: /* empty */ { > + if (!($$.strlist = calloc(1, sizeof(char * > + errx(1, "can't allocate strlist"); > + } | strlist TSTRING { > + int nstr = arraylen($1.strlist); > + if (!($$.strlist = reallocarray($1.strlist, nstr + 2, > sizeof(char * > - errx(1, "can't allocate envlist"); > - $$.envlist[nenv] = $2.str; > - $$.envlist[nenv + 1] = NULL; > + errx(1, "can't allocate strlist"); > + $$.strlist[nstr] = $2.str; > + $$.strlist[nstr + 1] = NULL; > } ; > > > @@ -179,20 +180,8 @@ cmd: /* optional */ { > > args:/* empty */ { > $$.cmdargs = NULL; > - } | TARGS argslist { > - $$.cmdargs = $2.cmdargs; > - } ; > - > -argslist:/* empty */ { > - if (!($$.cmdargs = calloc(1, sizeof(char * > - errx(1, "can't allocate args"); > - } | argslist TSTRING { > - int nargs = arraylen($1.cmdargs); > - if (!($$.cmdargs = reallocarray($1.cmdargs, nargs + 2, > - sizeof(char * > - errx(1, "can't allocate args"); > - $$.cmdargs[nargs] = $2.str; > - $$.cmdargs[nargs + 1] = NULL; > + } | TARGS strlist { > + $$.cmdargs = $2.strlist; > } ; > > %% >
libtls syslogd pledge abort
Hi, The previous commit to libtls makes syslogd abort due to pledge if certification verification is turned off. This happens in the chrooted child process. 87878 syslogd CALL open(0x2d203ce4,0) 87878 syslogd NAMI "/etc/ssl/cert.pem" 87878 syslogd PLDG open, "rpath", errno 1 Operation not permitted 87878 syslogd PSIG SIGABRT SIG_DFL code <-538976289> We can either preload the cert in syslogd even if verification is turned off. Index: usr.sbin/syslogd/syslogd.c === RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v retrieving revision 1.225 diff -u -p -r1.225 syslogd.c --- usr.sbin/syslogd/syslogd.c 27 Dec 2016 19:16:24 - 1.225 +++ usr.sbin/syslogd/syslogd.c 29 Dec 2016 22:57:41 - @@ -590,16 +590,14 @@ main(int argc, char *argv[]) if (NoVerify) { tls_config_insecure_noverifycert(client_config); tls_config_insecure_noverifyname(client_config); - } else { - if (tls_config_set_ca_file(client_config, - CAfile) == -1) { - logerrortlsconf("Load client TLS CA failed", - client_config); - /* avoid reading default certs in chroot */ - tls_config_set_ca_mem(client_config, "", 0); - } else - logdebug("CAfile %s\n", CAfile); } + if (tls_config_set_ca_file(client_config, CAfile) == -1) { + logerrortlsconf("Load client TLS CA failed", + client_config); + /* avoid reading default certs in chroot */ + tls_config_set_ca_mem(client_config, "", 0); + } else + logdebug("CAfile %s\n", CAfile); if (ClientCertfile && ClientKeyfile) { if (tls_config_set_cert_file(client_config, ClientCertfile) == -1) Or do not call tls_configure_ssl_verify() if verification is turned off. Index: lib/libtls/tls_client.c === RCS file: /data/mirror/openbsd/cvs/src/lib/libtls/tls_client.c,v retrieving revision 1.38 diff -u -p -r1.38 tls_client.c --- lib/libtls/tls_client.c 26 Dec 2016 16:20:58 - 1.38 +++ lib/libtls/tls_client.c 29 Dec 2016 22:56:23 - @@ -195,7 +195,9 @@ tls_connect_common(struct tls *ctx, cons } } - if (tls_configure_ssl_verify(ctx, ctx->ssl_ctx, SSL_VERIFY_PEER) == -1) + if (ctx->config->verify_cert && + (tls_configure_ssl_verify(ctx, ctx->ssl_ctx, +SSL_VERIFY_PEER) == -1)) goto err; if (SSL_CTX_set_tlsext_status_cb(ctx->ssl_ctx, tls_ocsp_verify_cb) != 1) { I would prefer the fix in libtls as - this problem may also affect other daemons - avoid to do unnecsessary stuff - syslogd could run on a system without cert.pem comments? ok? bluhm
Re: libtls syslogd pledge abort
> Or do not call tls_configure_ssl_verify() if verification is turned > off. This makes sense to me. > > Index: lib/libtls/tls_client.c > === > RCS file: /data/mirror/openbsd/cvs/src/lib/libtls/tls_client.c,v > retrieving revision 1.38 > diff -u -p -r1.38 tls_client.c > --- lib/libtls/tls_client.c 26 Dec 2016 16:20:58 - 1.38 > +++ lib/libtls/tls_client.c 29 Dec 2016 22:56:23 - > @@ -195,7 +195,9 @@ tls_connect_common(struct tls *ctx, cons > } > } > > - if (tls_configure_ssl_verify(ctx, ctx->ssl_ctx, SSL_VERIFY_PEER) == -1) > + if (ctx->config->verify_cert && > + (tls_configure_ssl_verify(ctx, ctx->ssl_ctx, > + SSL_VERIFY_PEER) == -1)) > goto err; > > if (SSL_CTX_set_tlsext_status_cb(ctx->ssl_ctx, tls_ocsp_verify_cb) != > 1) { > ok beck@ > I would prefer the fix in libtls as > - this problem may also affect other daemons > - avoid to do unnecsessary stuff > - syslogd could run on a system without cert.pem > > comments? ok? > > bluhm