myx_intr without a mutex for the sts block
myx_intr should be able to run without a lock, except in teh situation where are bringing an interface down. this interlock was done by serialising the changes myx_down and myx_intr needed to do with a mutex, but for the many millions and billions of interrupts you get when you're actually using the interface, this is an annoying overhead. this plays tricks to avoid the mutex. it works for me, but im throwing it out so someone else can try and hopefully break it. Index: if_myx.c === RCS file: /cvs/src/sys/dev/pci/if_myx.c,v retrieving revision 1.82 diff -u -p -r1.82 if_myx.c --- if_myx.c15 Aug 2015 01:17:01 - 1.82 +++ if_myx.c1 Sep 2015 05:09:34 - @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -123,7 +124,6 @@ struct myx_softc { struct myx_dmamemsc_sts_dma; volatile struct myx_status *sc_sts; - struct mutex sc_sts_mtx; int sc_intx; void*sc_irqh; @@ -226,26 +226,6 @@ void myx_tx_free(struct myx_softc *); void myx_refill(void *); -static inline void -myx_sts_enter(struct myx_softc *sc) -{ - bus_dmamap_t map = sc->sc_sts_dma.mxm_map; - - mtx_enter(&sc->sc_sts_mtx); - bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, - BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE); -} - -static inline void -myx_sts_leave(struct myx_softc *sc) -{ - bus_dmamap_t map = sc->sc_sts_dma.mxm_map; - - bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, - BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE); - mtx_leave(&sc->sc_sts_mtx); -} - struct cfdriver myx_cd = { NULL, "myx", DV_IFNET }; @@ -285,8 +265,6 @@ myx_attach(struct device *parent, struct timeout_set(&sc->sc_rx_ring[MYX_RXBIG].mrr_refill, myx_refill, &sc->sc_rx_ring[MYX_RXBIG]); - mtx_init(&sc->sc_sts_mtx, IPL_NET); - /* Map the PCI memory space */ memtype = pci_mapreg_type(sc->sc_pc, sc->sc_tag, MYXBAR0); if (pci_mapreg_map(pa, MYXBAR0, memtype, BUS_SPACE_MAP_PREFETCHABLE, @@ -889,6 +867,7 @@ void myx_media_status(struct ifnet *ifp, struct ifmediareq *imr) { struct myx_softc*sc = (struct myx_softc *)ifp->if_softc; + bus_dmamap_t map = sc->sc_sts_dma.mxm_map; u_int32_tsts; imr->ifm_active = IFM_ETHER | IFM_AUTO; @@ -897,9 +876,11 @@ myx_media_status(struct ifnet *ifp, stru return; } - myx_sts_enter(sc); + bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, + BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE); sts = sc->sc_sts->ms_linkstate; - myx_sts_leave(sc); + bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, + BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE); myx_link_state(sc, sts); @@ -1220,9 +1201,7 @@ myx_up(struct myx_softc *sc) goto empty_rx_ring_big; } - mtx_enter(&sc->sc_sts_mtx); sc->sc_state = MYX_S_RUNNING; - mtx_leave(&sc->sc_sts_mtx); if (myx_cmd(sc, MYXCMD_SET_IFUP, &mc, NULL) != 0) { printf("%s: failed to start the device\n", DEVNAME(sc)); @@ -1349,24 +1328,28 @@ myx_down(struct myx_softc *sc) struct ifnet*ifp = &sc->sc_ac.ac_if; volatile struct myx_status *sts = sc->sc_sts; bus_dmamap_t map = sc->sc_sts_dma.mxm_map; + struct sleep_state sls; struct myx_cmd mc; int s; int ring; - myx_sts_enter(sc); + bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, + BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE); sc->sc_linkdown = sts->ms_linkdown; + bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, + BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE); + sc->sc_state = MYX_S_DOWN; + membar_producer(); memset(&mc, 0, sizeof(mc)); (void)myx_cmd(sc, MYXCMD_SET_IFDOWN, &mc, NULL); - bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, - BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE); - while (sc->sc_state != MYX_S_OFF) - msleep(sts, &sc->sc_sts_mtx, 0, "myxdown", 0); - bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize, - BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE); - mtx_leave(&sc->sc_sts_mtx); + while (sc->sc_state != MYX_S_OFF) { + sleep_setup(&sls, sts, PWAIT, "myxdown"); + membar_consumer(); + sleep_finish(&sls, sc->sc_state != MYX_S_OFF); + } s = splnet(); if (ifp->if_link_state != LINK_STATE_UNKNOWN) { @@ -1604,23 +1587,22 @@ myx_intr(void *arg) struct myx_softc*sc = (struct myx
Re: escape minus signs in sndio manpages
Ingo Schwarze wrote: Until somebody shows me a specific list of roff implementations that do so, i challenge that statement as urban legend, and the advice to use \- for flags as cargo cult programming. The reason lintian complains about this is that Debian had problems with - rendering as a unicode hyphen in their groff at one time[1]. As far as I can tell this no longer applies to current groff (which does indeed weaken the rationale here). Can you tell me who the author and who the maintainer of that tool is? I'd like to question them about the reasons for their recommendation, if possible. lintian is maintained by a team who can be contacted at mailto:lintian-ma...@debian.org. ...however, digging further, they also don't care about this anymore[2]. In this light I'm sorry to have wasted everybody's time with this patch. [1] https://lists.debian.org/debian-devel/2003/debian-devel-200303/msg01481.html [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=785353
syslogd reuseaddr *:514
Hi, I had some problems running syslogd on a machine where another process had a 514 socket bound to a specific address. As this is not a real conflict, I think syslogd should bind *:514 with SO_REUSEADDR. This is already the case with all other sockets. ok? bluhm Index: usr.sbin/syslogd/syslogd.c === RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v retrieving revision 1.179 diff -u -p -r1.179 syslogd.c --- usr.sbin/syslogd/syslogd.c 27 Aug 2015 17:53:35 - 1.179 +++ usr.sbin/syslogd/syslogd.c 31 Aug 2015 00:10:02 - @@ -334,7 +334,7 @@ voidusage(void); void wallmsg(struct filed *, struct iovec *); intloghost_parse(char *, char **, char **, char **); intgetmsgbufsize(void); -intsocket_bind(const char *, const char *, const char *, int, int, +intsocket_bind(const char *, const char *, const char *, int, int *, int *); intunix_socket(char *, int, mode_t); void double_rbuf(int); @@ -450,7 +450,7 @@ main(int argc, char *argv[]) die(0); } - if (socket_bind("udp", NULL, "syslog", 0, SecureMode, + if (socket_bind("udp", NULL, "syslog", SecureMode, &fd_udp, &fd_udp6) == -1) { errno = 0; logerror("socket bind *"); @@ -458,7 +458,7 @@ main(int argc, char *argv[]) die(0); } fd_bind = -1; - if (bind_host && socket_bind("udp", bind_host, bind_port, 1, 0, + if (bind_host && socket_bind("udp", bind_host, bind_port, 0, &fd_bind, &fd_bind) == -1) { errno = 0; logerror("socket bind udp"); @@ -466,7 +466,7 @@ main(int argc, char *argv[]) die(0); } fd_listen = -1; - if (listen_host && socket_bind("tcp", listen_host, listen_port, 1, 0, + if (listen_host && socket_bind("tcp", listen_host, listen_port, 0, &fd_listen, &fd_listen) == -1) { errno = 0; logerror("socket listen tcp"); @@ -695,12 +695,12 @@ main(int argc, char *argv[]) int socket_bind(const char *proto, const char *host, const char *port, -int reuseaddr, int shutread, int *fd, int *fd6) +int shutread, int *fd, int *fd6) { struct addrinfo hints, *res, *res0; char hostname[NI_MAXHOST], servname[NI_MAXSERV]; char ebuf[ERRBUFSIZE]; - int *fdp, error; + int *fdp, error, reuseaddr; *fd = *fd6 = -1; if (proto == NULL) @@ -761,6 +761,7 @@ socket_bind(const char *proto, const cha *fdp = -1; continue; } + reuseaddr = 1; if (setsockopt(*fdp, SOL_SOCKET, SO_REUSEADDR, &reuseaddr, sizeof(reuseaddr)) == -1) { snprintf(ebuf, sizeof(ebuf), "setsockopt SO_REUSEADDR "
Re: ksh sanatizing argv redundant
> Martijn van Duren wrote: > > Hello tech@, > > > > I took a quick glance at ksh and one of the first things I noticed was > > that it uses some sanatizing code on argv. When looking at execve(2) I > > see that EINVAL or EFAULT are returned when argv isn't properly > > formatted. I've also verified this quickly by a small PoC and in > > sys/kern/kern_exec.c. > > > > Would it make sense to remove the check all together? > > I think this is ok. You used to have to worry about it, because the kernel let > you exec something with empty argv. And there's still perhaps a portability > concern. But old workarounds need to die sometime. I support removing this, > but I'd like some one else to comment. with fire (or else i wonder if doas needs this checking...)
Re: ksh sanatizing argv redundant
Martijn van Duren wrote: > Hello tech@, > > I took a quick glance at ksh and one of the first things I noticed was > that it uses some sanatizing code on argv. When looking at execve(2) I > see that EINVAL or EFAULT are returned when argv isn't properly > formatted. I've also verified this quickly by a small PoC and in > sys/kern/kern_exec.c. > > Would it make sense to remove the check all together? I think this is ok. You used to have to worry about it, because the kernel let you exec something with empty argv. And there's still perhaps a portability concern. But old workarounds need to die sometime. I support removing this, but I'd like some one else to comment.
Re: Like free(), ksh's afree() is NULL-safe
Michael McConville wrote: > Ted Unangst wrote: > > Michael McConville wrote: > > > Also, why is fs on line 390 cast to char* when afree() takes void*? > > > > this code is older than void. and NULL apparently. please remove the casts > > too. > > Does this look good? I had to leave one cast because the freed pointer > is a const. yup. only binary change is in two places, where the null check was removed.
Re: Like free(), ksh's afree() is NULL-safe
Ted Unangst wrote: > Michael McConville wrote: > > Also, why is fs on line 390 cast to char* when afree() takes void*? > > this code is older than void. and NULL apparently. please remove the casts > too. Does this look good? I had to leave one cast because the freed pointer is a const. Index: c_ksh.c === RCS file: /cvs/src/bin/ksh/c_ksh.c,v retrieving revision 1.34 diff -u -p -r1.34 c_ksh.c --- c_ksh.c 17 Dec 2013 16:37:05 - 1.34 +++ c_ksh.c 1 Sep 2015 00:22:03 - @@ -943,7 +943,7 @@ c_alias(char **wp) if ((val && !tflag) || (!val && tflag && !Uflag)) { if (ap->flag&ALLOC) { ap->flag &= ~(ALLOC|ISSET); - afree((void*)ap->val.s, APERM); + afree(ap->val.s, APERM); } /* ignore values for -t (at&t ksh does this) */ newval = tflag ? search(alias, path, X_OK, (int *) 0) : @@ -998,7 +998,7 @@ c_unalias(char **wp) } if (ap->flag&ALLOC) { ap->flag &= ~(ALLOC|ISSET); - afree((void*)ap->val.s, APERM); + afree(ap->val.s, APERM); } ap->flag &= ~(DEFINED|ISSET|EXPORT); } @@ -1009,7 +1009,7 @@ c_unalias(char **wp) for (ktwalk(&ts, t); (ap = ktnext(&ts)); ) { if (ap->flag&ALLOC) { ap->flag &= ~(ALLOC|ISSET); - afree((void*)ap->val.s, APERM); + afree(ap->val.s, APERM); } ap->flag &= ~(DEFINED|ISSET|EXPORT); } Index: edit.c === RCS file: /cvs/src/bin/ksh/edit.c,v retrieving revision 1.40 diff -u -p -r1.40 edit.c --- edit.c 12 Mar 2015 10:20:30 - 1.40 +++ edit.c 1 Sep 2015 00:22:04 - @@ -489,7 +489,7 @@ x_command_glob(int flags, const char *st path_order_cmp); for (i = 0; i < nwords; i++) words[i] = info[i].word; - afree((void *) info, ATEMP); + afree(info, ATEMP); } else { /* Sort and remove duplicate entries */ char **words = (char **) XPptrv(w); Index: emacs.c === RCS file: /cvs/src/bin/ksh/emacs.c,v retrieving revision 1.50 diff -u -p -r1.50 emacs.c --- emacs.c 25 Mar 2015 12:10:52 - 1.50 +++ emacs.c 1 Sep 2015 00:22:04 - @@ -1149,7 +1149,7 @@ x_push(int nchars) { char*cp = str_nsave(xcp, nchars, AEDIT); if (killstack[killsp]) - afree((void *)killstack[killsp], AEDIT); + afree(killstack[killsp], AEDIT); killstack[killsp] = cp; killsp = (killsp + 1) % KILLSIZE; } @@ -1307,8 +1307,7 @@ static void kb_del(struct kb_entry *k) { TAILQ_REMOVE(&kblist, k, entry); - if (k->args) - free(k->args); + free(k->args); afree(k, AEDIT); } Index: expand.h === RCS file: /cvs/src/bin/ksh/expand.h,v retrieving revision 1.6 diff -u -p -r1.6 expand.h --- expand.h30 Mar 2005 17:16:37 - 1.6 +++ expand.h1 Sep 2015 00:22:04 - @@ -55,7 +55,7 @@ typedef char * XStringP; #define Xcheck(xs, xp) XcheckN(xs, xp, 1) /* free string */ -#defineXfree(xs, xp) afree((void*) (xs).beg, (xs).areap) +#defineXfree(xs, xp) afree((xs).beg, (xs).areap) /* close, return string */ #defineXclose(xs, xp) (char*) aresize((void*)(xs).beg, \ @@ -104,4 +104,4 @@ typedef struct XPtrV { #defineXPclose(x) (void**) aresize((void*)(x).beg, \ sizeofN(void*, XPsize(x)), ATEMP) -#defineXPfree(x) afree((void*) (x).beg, ATEMP) +#defineXPfree(x) afree((x).beg, ATEMP) Index: history.c === RCS file: /cvs/src/bin/ksh/history.c,v retrieving revision 1.40 diff -u -p -r1.40 history.c --- history.c 20 Nov 2014 15:22:39 - 1.40 +++ history.c 1 Sep 2015 00:22:04 - @@ -417,7 +417,7 @@ histbackup(void) if (histptr >= history && last_line != hist_source->line) { hist_source->line--; - afree((void*)*histptr, APERM); + afree(*histptr, APERM); histptr--; last_line = hist_source->line; } @@ -589,7 +589,7 @@ histsave(int lno, const char *cmd, int d hp = histptr; if (++hp >= history + histsize) { /* remove oldest command */ - afree((void*)*history, APERM); +
doas path
I think we can relax the path restriction if there's no restriction on command. Index: doas.c === RCS file: /cvs/src/usr.bin/doas/doas.c,v retrieving revision 1.39 diff -u -p -r1.39 doas.c --- doas.c 27 Aug 2015 16:31:02 - 1.39 +++ doas.c 31 Aug 2015 19:03:27 - @@ -433,8 +433,10 @@ main(int argc, char **argv, char **envp) syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s", myname, cmdline, pw->pw_name, cwd); - if (setenv("PATH", safepath, 1) == -1) - err(1, "failed to set PATH '%s'", safepath); + if (rule->cmd) { + if (setenv("PATH", safepath, 1) == -1) + err(1, "failed to set PATH '%s'", safepath); + } execvpe(cmd, argv, envp); if (errno == ENOENT) errx(1, "%s: command not found", cmd); Index: doas.conf.5 === RCS file: /cvs/src/usr.bin/doas/doas.conf.5,v retrieving revision 1.14 diff -u -p -r1.14 doas.conf.5 --- doas.conf.5 30 Jul 2015 14:02:04 - 1.14 +++ doas.conf.5 31 Aug 2015 19:05:58 - @@ -73,6 +73,9 @@ The default is all users. The command the user is allowed or denied to run. The default is all commands. Be advised that it's best to specify absolute paths. +If a cmd is specified, only a restricted +.Ev PATH +will be searched. .It Ic args ... Arguments to command. If specified, the command arguments provided by the user
Re: Introducing rtvalid(9)
On Fri, Aug 28, 2015 at 12:47:51PM +0200, Martin Pieuchot wrote: > The rtvalid() function checks if the route entry rt is still valid and > can be used. Cached entries that are no longer valid should be released > by calling rtfree(). I like it. As it does some checks and returns a boolian value, I wonder wether rtisvalid() would be a better name. > + if (rtvalid(rt) == 0) { ... As the function returns a boolean value I perfer the logical check with !. if (rtvalid(rt)) { route is valid } if (!rtvalid(rt)) { route is not valid } > @@ -1239,8 +1233,10 @@ ip_rtaddr(struct in_addr dst, u_int rtab > ipforward_rt.ro_rt = rtalloc(&ipforward_rt.ro_dst, > RT_REPORT|RT_RESOLVE, rtableid); > } > - if (ipforward_rt.ro_rt == 0) > + if (rtvalid(ipforward_rt.ro_rt) == 0) { > + rtfree(ipforward_rt.ro_rt); > return (NULL); If you free the global variable ipforward_rt.ro_rt you have to reset the pointer with ipforward_rt.ro_rt = NULL. Otherwise you will run into a use after free. > @@ -1417,12 +1412,13 @@ ip_forward(struct mbuf *m, struct ifnet > > ipforward_rt.ro_rt = rtalloc_mpath(&ipforward_rt.ro_dst, > &ip->ip_src.s_addr, ipforward_rt.ro_tableid); > - if (ipforward_rt.ro_rt == 0) { > + if (rtvalid(ipforward_rt.ro_rt) == 0) { > + rtfree(ipforward_rt.ro_rt); > icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0); > return; Same here, ipforward_rt.ro_rt = NULL > +++ sys/netinet6/ip6_forward.c28 Aug 2015 09:40:40 - > @@ -240,24 +238,23 @@ reroute: > ip6_forward_rt.ro_tableid); > } > > - if (ip6_forward_rt.ro_rt == NULL) { > + if (rtvalid(ip6_forward_rt.ro_rt) == 0) { > ip6stat.ip6s_noroute++; > /* XXX in6_ifstat_inc(rt->rt_ifp, ifs6_in_noroute) */ > if (mcopy) { > icmp6_error(mcopy, ICMP6_DST_UNREACH, > ICMP6_DST_UNREACH_NOROUTE, 0); > } > + rtfree(rt); > m_freem(m); > return; rt is uninitialized. You want to free ip6_forward_rt.ro_rt and set it to NULL. > @@ -268,13 +265,14 @@ reroute: > &ip6->ip6_src.s6_addr32[0], > ip6_forward_rt.ro_tableid); > > - if (ip6_forward_rt.ro_rt == NULL) { > + if (rtvalid(ip6_forward_rt.ro_rt) == 0) { > ip6stat.ip6s_noroute++; > /* XXX in6_ifstat_inc(rt->rt_ifp, ifs6_in_noroute) */ > if (mcopy) { > icmp6_error(mcopy, ICMP6_DST_UNREACH, > ICMP6_DST_UNREACH_NOROUTE, 0); > } > + rtfree(rt); > m_freem(m); > return; Same here. > @@ -429,10 +429,9 @@ ip6_input(struct mbuf *m) > /* >* Unicast check >*/ > - if (ip6_forward_rt.ro_rt != NULL && > - (ip6_forward_rt.ro_rt->rt_flags & RTF_UP) != 0 && > + if (rtvalid(ip6_forward_rt.ro_rt) && > IN6_ARE_ADDR_EQUAL(&ip6->ip6_dst, > -&ip6_forward_rt.ro_dst.sin6_addr) && > +&ip6_forward_rt.ro_dst.sin6_addr) && Strange indentation. All other conversions look correct to me. bluhm
Re: escape minus signs in sndio manpages
On Sun, Aug 30, 2015 at 03:35:21PM -0400, Peter Piwowarski wrote: > This patch replaces some literal '-' with '\-' in examples for sndio- > related manpages. Certain roff implementations may render '-' as a > hyphen (unicode U+2010) rather than a minus sign (unicode U+002D or ascii > 0x2D); while mandoc renders them as intended with or without > the escape, it should be more portable to use it. Found with Debian's > lintian tool while packaging portable sndio for Debian; if there is > interest I can fix any other instances in the tree that I can find. replacing - by \- in literals produces identical files; tried mandoc and latest groff (1.22.3) with utf8, ps and html outputs, so I don't see how this could hurt. schwarze, jmc, opinions?
Re: [patch] return instead of exit(3) in src/bin/
On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote: > As suggested by deraadt@ and tobias@ it might be better to use the *return* > statement instead of exit(3) > inside the *main* function, to let the stack protector do its work. > > This diff removes such calls in all *src/bin/* tools, except those who > already use it. > I think I didn't miss a call and didn't introduce any bugs. > New diff with help from tobias@, as theo@ pointed me to a downside. > --F. > Index: cat/cat.c === RCS file: /cvs/src/bin/cat/cat.c,v retrieving revision 1.21 diff -u -p -r1.21 cat.c --- cat/cat.c 16 Jan 2015 06:39:28 - 1.21 +++ cat/cat.c 31 Aug 2015 20:44:20 - @@ -103,8 +103,7 @@ main(int argc, char *argv[]) raw_args(argv); if (fclose(stdout)) err(1, "stdout"); - exit(rval); - /* NOTREACHED */ + return (rval); } void Index: chio/chio.c === RCS file: /cvs/src/bin/chio/chio.c,v retrieving revision 1.25 diff -u -p -r1.25 chio.c --- chio/chio.c 16 Mar 2014 18:38:30 - 1.25 +++ chio/chio.c 31 Aug 2015 20:44:21 - @@ -148,7 +148,7 @@ main(int argc, char *argv[]) if (commands[i].cc_name == NULL) errx(1, "unknown command: %s", *argv); - exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); + return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); } static int Index: chmod/chmod.c === RCS file: /cvs/src/bin/chmod/chmod.c,v retrieving revision 1.34 diff -u -p -r1.34 chmod.c --- chmod/chmod.c 25 Jun 2015 02:04:08 - 1.34 +++ chmod/chmod.c 31 Aug 2015 20:44:22 - @@ -279,7 +279,7 @@ done: if (errno) err(1, "fts_read"); fts_close(ftsp); - exit(rval); + return (rval); } /* Index: cp/cp.c === RCS file: /cvs/src/bin/cp/cp.c,v retrieving revision 1.38 diff -u -p -r1.38 cp.c --- cp/cp.c 7 May 2015 17:32:20 - 1.38 +++ cp/cp.c 31 Aug 2015 20:44:22 - @@ -224,7 +224,7 @@ main(int argc, char *argv[]) type = FILE_TO_DIR; } - exit(copy(argv, type, fts_options)); + return (copy(argv, type, fts_options)); } char * Index: date/date.c === RCS file: /cvs/src/bin/date/date.c,v retrieving revision 1.47 diff -u -p -r1.47 date.c --- date/date.c 17 Apr 2015 16:47:47 - 1.47 +++ date/date.c 31 Aug 2015 20:44:22 - @@ -143,7 +143,7 @@ main(int argc, char *argv[]) errx(1, "conversion error"); (void)strftime(buf, sizeof(buf), format, tp); (void)printf("%s\n", buf); - exit(0); + return (0); } #defineATOI2(ar) ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - '0')) Index: dd/dd.c === RCS file: /cvs/src/bin/dd/dd.c,v retrieving revision 1.21 diff -u -p -r1.21 dd.c --- dd/dd.c 16 Jan 2015 06:39:31 - 1.21 +++ dd/dd.c 31 Aug 2015 20:44:22 - @@ -85,7 +85,7 @@ main(int argc, char *argv[]) } dd_close(); - exit(0); + return (0); } static void Index: df/df.c === RCS file: /cvs/src/bin/df/df.c,v retrieving revision 1.52 diff -u -p -r1.52 df.c --- df/df.c 16 Jan 2015 06:39:31 - 1.52 +++ df/df.c 31 Aug 2015 20:44:23 - @@ -175,7 +175,7 @@ main(int argc, char *argv[]) bsdprint(mntbuf, mntsize, maxwidth); } - exit(mntsize ? 0 : 1); + return (mntsize ? 0 : 1); } char * Index: domainname/domainname.c === RCS file: /cvs/src/bin/domainname/domainname.c,v retrieving revision 1.9 diff -u -p -r1.9 domainname.c --- domainname/domainname.c 16 Jan 2015 06:39:31 - 1.9 +++ domainname/domainname.c 31 Aug 2015 20:44:23 - @@ -66,7 +66,7 @@ main(int argc, char *argv[]) err(1, "getdomainname"); (void)printf("%s\n", domainname); } - exit(0); + return (0); } void Index: expr/expr.c === RCS file: /cvs/src/bin/expr/expr.c,v retrieving revision 1.20 diff -u -p -r1.20 expr.c --- expr/expr.c 11 Aug 2015 17:15:46 - 1.20 +++ expr/expr.c 31 Aug 2015 20:44:23 - @@ -518,5 +518,5 @@ main(int argc, char *argv[]) else printf("%s\n", vp->u.s); - exit(is_zero_or_null(vp)); + return (is_zero_or_null(vp)); } Index: hostname/hostname.c === RCS file:
PF ignores block action when rule contains route-to/dup-to action
Hello, Dilli Paudel in Oracle was playing with PF enough to find funny glitch. He used rule as follows: block in on vnic4 from 192.168.1.0/24 to any route-to 172.16.1.1@vnic5 Many people expect the route-to action is somewhat futile as 'block' action takes precedence here, so packet gets always dropped. Well the reality is very different (and still makes a sort of sense) from PF point of view. The snippet comes from pf_test(): 6586 switch (action) { 6587 case PF_SYNPROXY_DROP: 6593 case PF_DIVERT: 6594 switch (pd.af) { 6595 case AF_INET: 6610 case PF_AFRT: 6611 if (pf_translate_af(&pd)) { 6624 #endif /* INET6 */ 6625 default: 6626 /* pf_route can free the mbuf causing *m0 to become NULL */ 6627 if (r->rt) { 6628 switch (pd.af) { 6629 case AF_INET: 6630 pf_route(m0, r, pd.dir, pd.kif->pfik_ifp, s); 6631 break; the action comes from matching rule. It's PF_DROP in case of Dilli's rule. As you can see there is no case-branch for PF_DROP in switch statement at line 6586, so a default: is executed. For route-to action the r->rt is set and PF executes the route_to*(). Dilli suggests to introduce PF_DROP case branch to switch() at line 6586 Issue has been also discussed on Friday over icb, where Mr. bluhm further suggested we should try to add some sanity check to parse.y. As a side effect the patch breaks block rules with dup-to action. dup-to action as a part of block rule might make some sense... So if there is someone, who really needs block ... dup-to he should opt for equivalent rule using pass ... route-to Also there is one more question: shall we implement similar sanity checks for nat-to/rdr-to/... actions? no one should expect those in block rule, so making pfctl to refuse such rules loudly sounds like a right thing to do... regards sasha 8<---8<---8<--8< Index: sbin/pfctl/parse.y === RCS file: /cvs/src/sbin/pfctl/parse.y,v retrieving revision 1.648 diff -u -p -r1.648 parse.y --- sbin/pfctl/parse.y 21 Apr 2015 16:34:59 - 1.648 +++ sbin/pfctl/parse.y 31 Aug 2015 20:30:56 - @@ -3997,8 +3997,11 @@ rule_consistent(struct pf_rule *r, int a problems++; } - /* match rules rules */ - if (r->action == PF_MATCH) { + /* +* Basic rule sanity check. +*/ + switch (r->action) { + case PF_MATCH: if (r->divert.port) { yyerror("divert is not supported on match rules"); problems++; @@ -4016,6 +4019,15 @@ rule_consistent(struct pf_rule *r, int a yyerror("af-to is not supported on match rules"); problems++; } + break; + case PF_DROP: + if (r->rt) { + yyerror("route-to, reply-to and dup-to " + "must not be used on block rules"); + problems++; + } + break; + default:; } return (-problems); } Index: sys/net/pf.c === RCS file: /cvs/src/sys/net/pf.c,v retrieving revision 1.936 diff -u -p -r1.936 pf.c --- sys/net/pf.c19 Aug 2015 21:22:41 - 1.936 +++ sys/net/pf.c31 Aug 2015 20:31:02 - @@ -6622,6 +6622,10 @@ done: action = PF_PASS; break; #endif /* INET6 */ + case PF_DROP: + m_freem(*m0); + *m0 = NULL; + break; default: /* pf_route can free the mbuf causing *m0 to become NULL */ if (r->rt) {
[WIP PATCH] SR RAID1 checksumming support
Hello, attached my work in progress on checksumming support for softraid RAID1. Currently it does just: - computation of checksums (crc32) - verification of checksums - signal bad checksum to console and to sensors E.g.: $ sysctl hw.sensors.softraid0 hw.sensors.softraid0.raw0=0 (sd0f), OK hw.sensors.softraid0.raw1=0 (sd0g), OK hw.sensors.softraid0.drive0=online (sd1), OK Next TODO items: - hang-over to another chunk (restart wu) in case of checksum error - properly handle errors hapenning on all chunks - "self-healing" of bad sector Note: checksums are computed per sector basis, saved in the area allocated at the end of the drive. Due to this design, LBA collision detection in softraid.c was enhanced/fixed to support also this case of application and currently it may not be compatible with RAID5/6 usage. Any comments welcome! Thanks! Karel Index: sbin/bioctl/bioctl.8 === RCS file: /cvs/src/sbin/bioctl/bioctl.8,v retrieving revision 1.96 diff -u -p -u -r1.96 bioctl.8 --- sbin/bioctl/bioctl.829 May 2015 00:33:37 -1.96 +++ sbin/bioctl/bioctl.831 Aug 2015 20:02:47 - @@ -199,6 +199,11 @@ for example, force the creation of volum with unclean data in the metadata areas. .It Ar noauto Do not automatically assemble this volume at boot time. +.It Ar chksum +Enforce usage of checksums on the device blocks. The checksum area is +located at the end of the device data area and since it accupies some +space it makes actual usable device size smaller. We need exactly 8 +bytes of checksum per device data block. .El .It Fl c Ar raidlevel Create a Index: sbin/bioctl/bioctl.c === RCS file: /cvs/src/sbin/bioctl/bioctl.c,v retrieving revision 1.129 diff -u -p -u -r1.129 bioctl.c --- sbin/bioctl/bioctl.c18 Jul 2015 23:23:20 -1.129 +++ sbin/bioctl/bioctl.c31 Aug 2015 20:02:47 - @@ -1053,6 +1053,9 @@ bio_createflags(char *lst) case 'n': flags |= BIOC_SCNOAUTOASSEMBLE; break; +case 'c': +flags |= BIOC_SCCHKSUM; +break; default: strlcpy(fs, s, sz + 1); errx(1, "invalid flag %s", fs); Index: sys/dev/biovar.h === RCS file: /cvs/src/sys/dev/biovar.h,v retrieving revision 1.44 diff -u -p -u -r1.44 biovar.h --- sys/dev/biovar.h29 May 2015 00:33:37 -1.44 +++ sys/dev/biovar.h31 Aug 2015 20:02:49 - @@ -213,6 +213,7 @@ struct bioc_createraid { #define BIOC_SCDEVT0x02/* dev_t array or string in dev_list */ #define BIOC_SCNOAUTOASSEMBLE0x04/* do not assemble during autoconf */ #define BIOC_SCBOOTABLE0x08/* device is bootable */ +#define BIOC_SCCHKSUM0x10/* device provides chksum capability */ u_int32_tbc_opaque_size; u_int32_tbc_opaque_flags; #defineBIOC_SOINVALID0x00/* no opaque pointer */ Index: sys/dev/softraid.c === RCS file: /cvs/src/sys/dev/softraid.c,v retrieving revision 1.364 diff -u -p -u -r1.364 softraid.c --- sys/dev/softraid.c19 Aug 2015 19:05:24 -1.364 +++ sys/dev/softraid.c31 Aug 2015 20:02:50 - @@ -71,6 +71,7 @@ uint32_tsr_debug = 0 /* | SR_D_DIS */ /* | SR_D_STATE */ /* | SR_D_REBUILD */ +/* | SR_D_CHKSUM */ ; #endif @@ -144,6 +145,8 @@ intsr_chunk_in_use(struct sr_softc *, intsr_rw(struct sr_softc *, dev_t, char *, size_t, daddr_t, long); voidsr_wu_done_callback(void *); +intsr_wu_collision(struct sr_workunit *, +struct sr_workunit *); /* don't include these on RAMDISK */ #ifndef SMALL_KERNEL @@ -2264,6 +2267,9 @@ sr_wu_done_callback(void *xwu) s = splbio(); +DNPRINTF(SR_D_WU, "%s: sr_wu_done: %p\n", + DEVNAME(sd->sd_sc), wu); + if (xs != NULL) { if (wu->swu_ios_failed) xs->error = XS_DRIVER_STUFFUP; @@ -2286,11 +2292,54 @@ sr_wu_done_callback(void *xwu) TAILQ_REMOVE(&sd->sd_wu_pendq, wu, swu_link); if (wu->swu_collider) { -if (wu->swu_ios_failed) -sr_raid_recreate_wu(wu->swu_collider); +DNPRINTF(SR_D_WU, "%s: sr_wu_done, searching for collider: %p\n", + DEVNAME(sd->sd_sc), wu->swu_collider); +if (wu->swu_ios_failed) { + DNPRINTF(SR_D_WU, "%s: sr_wu_done, recreate collider?: %p WHY???\n", + DEVNAME(sd->sd_sc), wu->swu_collider); + sr_raid_recreate_wu(wu->swu_collider); +} +/* + * We're searching for wu which do have the same collider + * like current wu. If we find such wu we can continue + * without starting the collider. If we do not find su
ksh sanatizing argv redundant
Hello tech@, I took a quick glance at ksh and one of the first things I noticed was that it uses some sanatizing code on argv. When looking at execve(2) I see that EINVAL or EFAULT are returned when argv isn't properly formatted. I've also verified this quickly by a small PoC and in sys/kern/kern_exec.c. Would it make sense to remove the check all together? Furthermore I see that kshname is based on directly dereferencing argv. Although it's semantics I personally reckon it's cleaner to do it by array index. Sincerely, Martijn van Duren Index: main.c === RCS file: /cvs/src/bin/ksh/main.c,v retrieving revision 1.55 diff -u -p -r1.55 main.c --- main.c 9 Feb 2015 09:09:30 - 1.55 +++ main.c 31 Aug 2015 19:51:25 - @@ -100,16 +100,7 @@ main(int argc, char *argv[]) struct env env; pid_t ppid; - /* make sure argv[] is sane */ - if (!*argv) { - static const char *empty_argv[] = { - "ksh", (char *) 0 - }; - - argv = (char **) empty_argv; - argc = 1; - } - kshname = *argv; + kshname = argv[0]; ainit(&aperm); /* initialize permanent Area */
[patch] in usb(4) check for dangerous requests
Hi all, This patch adds the usb control request validity checks already present in ugen(4) to usb(4). Grant Index: usb.c === RCS file: /cvs/src/sys/dev/usb/usb.c,v retrieving revision 1.107 diff -u -p -r1.107 usb.c --- usb.c 14 Mar 2015 03:38:50 - 1.107 +++ usb.c 31 Aug 2015 19:37:22 - @@ -622,7 +622,15 @@ usbioctl(dev_t devt, u_long cmd, caddr_t return (EBADF); DPRINTF(("usbioctl: USB_REQUEST addr=%d len=%d\n", addr, len)); - if (len < 0 || len > 32768) + /* Avoid requests that would damage the bus integrity. */ + if ((ur->ucr_request.bmRequestType == UT_WRITE_DEVICE && +ur->ucr_request.bRequest == UR_SET_ADDRESS) || + (ur->ucr_request.bmRequestType == UT_WRITE_DEVICE && +ur->ucr_request.bRequest == UR_SET_CONFIG) || + (ur->ucr_request.bmRequestType == UT_WRITE_INTERFACE && +ur->ucr_request.bRequest == UR_SET_INTERFACE)) + return (EINVAL); + if (len < 0 || len > 32767) return (EINVAL); if (addr < 0 || addr >= USB_MAX_DEVICES) return (EINVAL);
Re: rt_ifa_del() tweak
On Tue, Aug 25, 2015 at 01:01:38PM +0200, Martin Pieuchot wrote: > I want to remove this chunked introduce in r7.19 in 1991 by sklower@ > because it no longer makes any sense, it is a layer violation and > does no play well with rt refcounting. > > When this chunk was introduced rtrequest1(RTM_DELETE...) was *not* doing > a route lookup. So this check is now (and since a long time) redundant > with the rtable_lookup()+rtable_mpath_match() done inside rtrequest1(9). > > Remember that when this code was introduced ``ifa'' where linked to > ``rt'' in 1:1 fashion and rtrequest() started by rtfree(9)ing route > entries. That's why you wanted to be sure that the following line was > correct: > > error = rtrequest(cmd, dst, ifa->ifa_addr, ifa->ifa_netmask, > flags | ifa->ifa_flags, &ifa->ifa_rt) > > Ok? It looks that the error changes from EHOSTUNREACH/ENETUNREACH to ESRCH. That is good, when you try to remove an non existing address/route. The check in the old code and in rtable_mpath_match() is not equivalent. The ifa was used to compare it with every route. In rtable_mpath_match() the RTAX_GATEWAY is used for that purpose. But this field is only set, if RTF_LLINFO is set in flags. So at least the old check does not look completely redundant. But it still looks wrong to search for a route by comparing rt_ifa with ifa and, after that has been found, delete another one. So I would say, delete that chunk. bluhm > Index: net/route.c > === > RCS file: /cvs/src/sys/net/route.c,v > retrieving revision 1.225 > diff -u -p -r1.225 route.c > --- net/route.c 24 Aug 2015 22:11:33 - 1.225 > +++ net/route.c 25 Aug 2015 10:43:32 - > @@ -1230,21 +1230,6 @@ rt_ifa_del(struct ifaddr *ifa, int flags > rt_maskedcopy(dst, deldst, ifa->ifa_netmask); > dst = deldst; > } > - if ((rt = rtalloc(dst, 0, rtableid)) != NULL) { > - rt->rt_refcnt--; > -#ifndef ART > - /* try to find the right route */ > - while (rt && rt->rt_ifa != ifa) > - rt = (struct rtentry *) > - ((struct radix_node *)rt)->rn_dupedkey; > - if (!rt) { > - if (m != NULL) > - (void) m_free(m); > - return (flags & RTF_HOST ? EHOSTUNREACH > - : ENETUNREACH); > - } > -#endif > - } > > memset(&info, 0, sizeof(info)); > info.rti_ifa = ifa;
Re: Like free(), ksh's afree() is NULL-safe
Michael McConville wrote: > Also, why is fs on line 390 cast to char* when afree() takes void*? this code is older than void. and NULL apparently. please remove the casts too.
Re: ntpd(8): Make -n quieter
On 08/31/15 07:36, Sebastian Benoit wrote: > Michael Reed(m.r...@mykolab.com) on 2015.08.30 14:58:35 -0400: >> Hi all, >> >> If ntpd is run with the -n flag, and /etc/ntpd.conf is parsed without >> error, then "Configuration OK" is printed. I don't think this is >> particularly useful, as both a lack of an error message and an exit >> value of 0 already indicate success in this case. This seems to be the >> case for most (many?) programs in the base system, such as doas(1). > > I like the message. Why is it a problem? > > /Benno > It's admittedly not much of a problem, more just to follow the Unix principle of saying nothing if there's nothing wrong. Regards, Michael
Re: syslogd host matches ip
On Sun, Aug 30, 2015 at 07:18:34PM +0200, Alexander Bluhm wrote: > > I reconsidered it. It does not make sense to truncate the hostname > in the config at some character from a list. Just take whatever > the user specified as progname or hostname. > > ok? visual ok + slightly tested OK semarie@ > bluhm > > Index: usr.sbin/syslogd/syslogd.c > === > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v > retrieving revision 1.179 > diff -u -p -r1.179 syslogd.c > --- usr.sbin/syslogd/syslogd.c27 Aug 2015 17:53:35 - 1.179 > +++ usr.sbin/syslogd/syslogd.c30 Aug 2015 17:06:13 - > @@ -1937,7 +1937,7 @@ die(int signo) > void > init(void) > { > - char progblock[NAME_MAX+1], hostblock[NAME_MAX+1], *cline, *p; > + char progblock[NAME_MAX+1], hostblock[NAME_MAX+1], *cline, *p, *q; > struct filed_list mb; > struct filed *f, *m; > FILE *cf; > @@ -2025,40 +2025,22 @@ init(void) > continue; > if (*p == '\0' || *p == '#') > continue; > - if (*p == '!') { > + if (*p == '!' || *p == '+') { > + q = (*p == '!') ? progblock : hostblock; > p++; > while (isspace((unsigned char)*p)) > p++; > - if (!*p || (*p == '*' && (!p[1] || > + if (*p == '\0' || (*p == '*' && (p[1] == '\0' || > isspace((unsigned char)p[1] { > - strlcpy(progblock, "*", sizeof(progblock)); > + strlcpy(q, "*", NAME_MAX+1); > continue; > } > for (i = 0; i < NAME_MAX; i++) { > - if (!isalnum((unsigned char)p[i]) && > - p[i] != '-' && p[i] != '!') > + if (*p == '\0' || isspace((unsigned char)*p)) > break; > - progblock[i] = p[i]; > + *q++ = *p++; > } > - progblock[i] = 0; > - continue; > - } > - if (*p == '+') { > - p++; > - while (isspace((unsigned char)*p)) > - p++; > - if (!*p || (*p == '*' && (!p[1] || > - isspace((unsigned char)p[1] { > - strlcpy(hostblock, "*", sizeof(hostblock)); > - continue; > - } > - for (i = 0; i < NAME_MAX; i++) { > - if (!isalnum((unsigned char)p[i]) && > - p[i] != '-' && p[i] != '+') > - break; > - hostblock[i] = p[i]; > - } > - hostblock[i] = 0; > + *q = '\0'; > continue; > } > -- Sebastien Marie
virtualization support
TL;DR - a native hypervisor is coming. stay tuned. For the last few months, I've been working on a hypervisor for OpenBSD. The idea for this started a few years ago, and after playing around with it from time to time, things really started to take shape around the time of the Brisbane hackathon earlier this year. As development accelerated, the OpenBSD Foundation generously offered to fund the project so that I could focus on it in more earnest. At this point, I think I've made sufficient progress that a public announcement is in order. I've also reached the point where I think other developers can step in and help out as much of the gooey bits in the core of the vmm are functioning the way I want. Presently, the vmm code I've built is capable of launching a kernel and asking for the root filesystem; it doesn't do much more than that for now. However, getting to this point invovled building most of the scaffolding needed to finish the rest, and like I stated before, more people can now more easily help with what's left (basically the virtio(4) emulation for disks and network interfaces). One might ask - why not port one of the other hypervisors out there instead of rolling your own from scratch? Fair question. However, for various technical reasons, choosing to port an existing vmm just didn't make a whole lot of sense. For example, I've been baking in support for things that the other implementations don't care about (namely i386 support, shadow paging, nested virtualization, support for legacy peripherals, etc) and trying to backfit support for those things into another hypervisor would probably have been just as hard as building it from the ground up. The inevitable questions: Q. What OSes can I run? A. To start, OSes that support virtio-based devices (most of the CPU goo is done and is basically the same across most OSes once you get that part finished). Later, we'll see if we can expose a place for qemu to attach (maybe mimic KVM) to run legacy OSes or OSes that require BIOS/UEFI boot support. There is no legacy-free mandate in this vmm. Q. When will it be ready? A. Hard to say. I hope by the end of October, but no promises. A lot also depends on how much help I get writing some of the virtio backends. Q. What will be the CPU requirements? A. Any AMD or Intel CPU that supports hardware virtualization (SVM for AMD, and VMX for Intel). For those CPUs that don't support RVI or EPT, we'll use shadow paging. Q. What will be the OS requirements? A. The core tools (vmd(8) and vmmctl(8)) and the main vmm (vmm(4)) will be built in to base, on i386 and amd64. This should be sufficient to run virtio based guests without any additional software requirements. Q. Yuck, i386? A. Yes. It helps find bugs. And I urge you to review my last dozen or so commits to i386 for another reason. And now, the most important question: Q. How can I help? A. I would have likely had no time to work on this project for the past few months had it not been for the generous sponsorship of the OpenBSD Foundation. Your donations made that possible, and other projects like this. So, you can help by donating. If you think a hypervisor in OpenBSD is something you'd like to take advantage of, I urge you to go make a donation right now. http://www.openbsdfoundation.org/donations.html --- And finally, a sample boot: Enable vmm mode (ala apm/apmd): # vmmctl -e cpu0: entered VMM mode Start a VM: # vmmctl -S -c openbsd_amd64.vmrc /dev/ttyp0 Attach to VM console: # cu -l /dev/ttyp0 exit save va/pa 0xf000bcff000 0xbcff000 exit load va/pa 0xf000bd1e000 0xbd1e000 entry load va/pa 0xff000bd2c000 0xbd2c000 vmd: new vm with id 1 created loading 64 bit kernel warning: bcopy during ELF kernel load not supported warning: bcopy during ELF kernel load not supported mark[start] = 0x100 mark[entry] = 0x1000160 mark[nsym] = 0x1 mark[sym] = 0x1939000 mark[end] = 0x1a1de00 mark[random] = 0x18a4000 mark[erandom] = 0x18a6408 vmd: all vcpu run threads for vm 3 launched vcpu_run_vmx: yielding the cpu vmd: waiting on thread 0 vcpu_run_vmx: yielding the cpu vcpu_run_vmx: yielding the cpu loading 0x1a44000-0x400 (0x1a44-0x4000) loading 0x10-0x100 avail_start = 0x26000 avail_end = 0x400 First_avail = 0x1a44000 [ bsd ELF symbol table not valid: bad magic ] [ no symbol table formats found ] Copyright (c) 1982, 1986, 1989, 1991, 1993 The Regents of the University of California. All rights reserved. Copyright (c) 1995-2015 OpenBSD. All rights reserved. http://www.OpenBSD.org OpenBSD 5.8-current (GENERIC) #204: Mon Aug 31 01:13:40 PDT 2015 mlar...@miskatonic.azathoth.net:/export/bin/src/OpenBSD/vmm/src/sys/arch/amd64/compile/GENERIC vmd: i8253 PIT: 16 bit counter I/O not supported RTC BIOS diagnostic error ff real mem = 50331648 (48MB) avail mem = 45101056 (43MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0 acpi at bios0 not configured cpu0 at mainbus0: (uniprocessor) cpu0:
Re: synaptics touchpads: w mode and resolution
On Sat, Aug 29, 2015 at 6:10 PM, Ulf Brosziewski < ulf.brosziew...@t-online.de> wrote: > On 08/29/2015 01:13 PM, Alexandr Shadchin wrote: > >> On Fri, Aug 28, 2015 at 10:04:51PM +0200, Ulf Brosziewski wrote: >> >>> >>> Some weeks ago a change was made in pms to support Synaptics touchpads >>> that don't provide W mode. I assume that only fairly old models are >>> concerned, and that the variant in the patch below is more accurate. It >>> only fakes W values where necessary. >>> >>> The patch contains a second change, a check whether the resolution query >>> was successful. According to the "Synaptics PS/2 Interfacing Guide", bit >>> 7 of the second response byte will be set in case of success. It seems >>> that if it isn't set, the other bytes are garbage or have an unknown >>> meaning. >>> >>> Unfortunately I only have that old model - with firmware 4.1 - for >>> testing. Could someone confirm that newer models aren't affected by >>> these changes? >>> >>> >>> >> Tested on X201 (Synaptics touchpad, firmware 7.4), no regress. >> >> See comment below. >> >> Index: dev/pckbc/pms.c >>> === >>> RCS file: /cvs/src/sys/dev/pckbc/pms.c,v >>> retrieving revision 1.65 >>> diff -u -p -r1.65 pms.c >>> --- dev/pckbc/pms.c 23 Aug 2015 04:45:23 - 1.65 >>> +++ dev/pckbc/pms.c 28 Aug 2015 18:04:45 - >>> @@ -949,8 +949,10 @@ synaptics_get_hwinfo(struct pms_softc *s >>> return (-1); >>> } >>> >>> - syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution); >>> - syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution); >>> + if (syn->resolution& 0x8000) { >>> >> >> I think it makes sense to define SYNAPTICS_RESOLUTION_VALID >> instead of magic numbers. >> >> + syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution); >>> + syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution); >>> + } >>> [...] >>> >> > > Thanks a lot. I have added the constant to pmsreg.h, this is the > updated diff: > > ok shadchin@ > Index: dev/pckbc/pms.c > === > RCS file: /cvs/src/sys/dev/pckbc/pms.c,v > retrieving revision 1.65 > diff -u -p -r1.65 pms.c > --- dev/pckbc/pms.c 23 Aug 2015 04:45:23 - 1.65 > +++ dev/pckbc/pms.c 29 Aug 2015 12:33:57 - > @@ -949,8 +949,10 @@ synaptics_get_hwinfo(struct pms_softc *s > return (-1); > } > > - syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution); > - syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution); > + if (syn->resolution & SYNAPTICS_RESOLUTION_VALID) { > > + syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution); > + syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution); > + } > syn->min_x = SYNAPTICS_XMIN_BEZEL; > syn->min_y = SYNAPTICS_YMIN_BEZEL; > syn->max_x = (syn->dimension) ? > @@ -1163,30 +1165,21 @@ pms_proc_synaptics(struct pms_softc *sc) > > w = ((sc->packet[0] & 0x30) >> 2) | ((sc->packet[0] & 0x04) >> 1) | > ((sc->packet[3] & 0x04) >> 2); > + z = sc->packet[2]; > > - /* > -* Conform to the encoding understood by > -* /usr/xenocara/driver/xf86-input-synaptics/src/wsconscomm.c > -*/ > - switch (w) { > - case 0: > - /* fingerwidth 5, numfingers 2 */ > - break; > - case 1: > - /* fingerwidth 5, numfingers 3 */ > - break; > - case 5: > - /* fingerwidth 5, numfingers 1 */ > - break; > - case 4: > - case 8: > - /* fingerwidth 4, numfingers 1 */ > - w = 4; > - break; > - default: > - break; > + if ((syn->capabilities & SYNAPTICS_CAP_EXTENDED) == 0) { > + /* > +* Emulate W mode for models that don't provide it. Bit 3 > +* of the w-input signals a touch ("finger"), Bit 2 and > +* the "gesture" bits 1-0 can be ignored. > +*/ > + if (w & 8) > + w = 4; > + else > + z = w = 0; > } > > + > if ((syn->capabilities & SYNAPTICS_CAP_PASSTHROUGH) && w == 3) { > synaptics_sec_proc(sc); > return; > @@ -1203,7 +1196,6 @@ pms_proc_synaptics(struct pms_softc *sc) > sc->packet[4]; > y = ((sc->packet[3] & 0x20) << 7) | ((sc->packet[1] & 0xf0) << 4) | > sc->packet[5]; > - z = sc->packet[2]; > > buttons = ((sc->packet[0] & sc->packet[3]) & 0x01) ? > WSMOUSE_BUTTON(1) : 0; > Index: dev/pckbc/pmsreg.h > === > RCS file: /cvs/src/sys/dev/pckbc/pmsreg.h,v > retrieving revision 1.11 > diff -u -p -r1.11 pmsreg.h > --- dev/pckbc/pmsre
Re: Call for Testing: rtalloc(9) change
On 25/08/15(Tue) 12:27, Martin Pieuchot wrote: > On 12/08/15(Wed) 17:03, Martin Pieuchot wrote: > > I'm currently working on the routing table interface to make is safe > > to use by multiple CPUs at the same time. The diff below is a big > > step in this direction and I'd really appreciate if people could test > > it with their usual network setup and report back. > > Updated version to match recent changes. I'm still looking for test > reports and reviews. Thanks to all the testers. Here's a third version that should fix a regression reported by phessler@. The idea is to use rtvalid(9) to check early if the gateway route is still valid or not. If it isn't then call rtalloc(9) again to perform the ARP/NDP resolution. This diff includes rtvalid(9) and makes use of it in ip{,6}_output(), more conversions will be done afterward, but these should hopefully be all the necessary bits to fix the regression. Tests & reviews are still welcome! Index: net/route.c === RCS file: /cvs/src/sys/net/route.c,v retrieving revision 1.226 diff -u -p -r1.226 route.c --- net/route.c 30 Aug 2015 10:39:16 - 1.226 +++ net/route.c 31 Aug 2015 09:33:27 - @@ -153,6 +153,7 @@ int rtable_alloc(void ***, u_int); intrtflushclone1(struct rtentry *, void *, u_int); void rtflushclone(unsigned int, struct rtentry *); intrt_if_remove_rtdelete(struct rtentry *, void *, u_int); +struct rtentry *rt_match(struct sockaddr *, int, unsigned int); struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *, u_int); @@ -300,19 +301,68 @@ rtable_exists(u_int id) /* verify table return (1); } + +/* + * Do the actual checks for rtvalid(9), do not use directly! + */ +static inline int +rt_is_valid(struct rtentry *rt) +{ + if (rt == NULL) + return (0); + + if ((rt->rt_flags & RTF_UP) == 0) + return (0); + + /* Routes attached to stall ifas should be freed. */ + if (rt->rt_ifa == NULL || rt->rt_ifa->ifa_ifp == NULL) + return (0); + + return (1); +} + +/* + * Returns 1 if the cached ``rt'' entry is still valid, 0 otherwise. + */ +int +rtvalid(struct rtentry *rt) +{ + if (rt_is_valid(rt) == 0) + return (0); + + /* Next hop must also be valid. */ + if ((rt->rt_flags & RTF_GATEWAY) && rt_is_valid(rt->rt_gwroute) == 0) + return (0); + + return (1); +} + +/* + * Do the actual lookup for rtalloc(9), do not use directly! + * + * Return the best matching entry for the destination ``dst''. + * + * "RT_RESOLVE" means that a corresponding L2 entry should + * be added to the routing table and resolved (via ARP or + * NDP), if it does not exist. + * + * "RT_REPORT" indicates that a message should be sent to + * userland if no matching route has been found or if an + * error occured while adding a L2 entry. + */ struct rtentry * -rtalloc(struct sockaddr *dst, int flags, unsigned int tableid) +rt_match(struct sockaddr *dst, int flags, unsigned int tableid) { struct rtentry *rt; struct rtentry *newrt = NULL; struct rt_addrinfo info; - int s, error = 0, msgtype = RTM_MISS; + int s, error = 0; - s = splsoftnet(); bzero(&info, sizeof(info)); info.rti_info[RTAX_DST] = dst; + s = splsoftnet(); rt = rtable_match(tableid, dst); if (rt != NULL) { newrt = rt; @@ -325,10 +375,6 @@ rtalloc(struct sockaddr *dst, int flags, goto miss; } rt = newrt; - if (rt->rt_flags & RTF_XRESOLVE) { - msgtype = RTM_RESOLVE; - goto miss; - } /* Inform listeners of the new route */ rt_sendmsg(rt, RTM_ADD, tableid); } else @@ -336,11 +382,8 @@ rtalloc(struct sockaddr *dst, int flags, } else { rtstat.rts_unreach++; miss: - if (ISSET(flags, RT_REPORT)) { - bzero((caddr_t)&info, sizeof(info)); - info.rti_info[RTAX_DST] = dst; - rt_missmsg(msgtype, &info, 0, NULL, error, tableid); - } + if (ISSET(flags, RT_REPORT)) + rt_missmsg(RTM_MISS, &info, 0, NULL, error, tableid); } splx(s); return (newrt); @@ -374,6 +417,75 @@ rtalloc_mpath(struct sockaddr *dst, uint } #endif /* SMALL_KERNEL */ +/* + * Look in the routing table for the best matching entry for + * ``dst''. + * + * If a route with a gateway is found and its next hop is no + * longer valid, try to cache it. + */ +struct rtentry * +rtalloc(struct sockaddr *dst, int flags, unsigned int rta
Re: tap-and-drag on ALPS touchpads
On Sat, Aug 15, 2015 at 4:32 AM, Ulf Brosziewski < ulf.brosziew...@t-online.de> wrote: > > The tap-and-drag gesture doesn't work reliably with the ALPS touchpads > supported by pms. To make it work, it is necessary to start dragging > immediately with the second touch, which doesn't always succeed. > Increasing the tap timeout helps, but doesn't change the principle. The > patch below solves that problem. Tests and comments would be welcome. > > In case someone is interested in the background: I have observed the > same problem with a Linux installation on a machine with an ALPS > touchpad; however, increasing the tap timeout to a sufficiently high > value (>= 260ms) makes it work normally. The somewhat special behaviour > of those ALPS models is described in this LKML posting: > https://lkml.org/lkml/2004/7/28/210 > The approach put forward there - which is partially reproduced in the > current version of pms - is improved by the patch as follows: When the > hardware signals a tap, the handler does nothing. The missing events > will be emulated when the next packet arrives, which either signals the > end of the gesture, or the start of a drag action. No timeout will occur > even if the hardware introduces a long delay between the first and the > second packet (which just happens when the finger is resting after the > second contact). > > > Now is a good time to commit it :-) ok shadchin@ > Index: dev/pckbc/pms.c > === > RCS file: /cvs/src/sys/dev/pckbc/pms.c,v > retrieving revision 1.64 > diff -u -p -r1.64 pms.c > --- dev/pckbc/pms.c 20 Jul 2015 00:55:06 - 1.64 > +++ dev/pckbc/pms.c 14 Aug 2015 18:27:57 - > @@ -120,7 +120,8 @@ struct alps_softc { > > int min_x, min_y; > int max_x, max_y; > - int old_fin; > + > + u_int gesture; > > u_int sec_buttons; /* trackpoint */ > > @@ -1521,8 +1522,7 @@ pms_proc_alps(struct pms_softc *sc) > { > struct alps_softc *alps = sc->alps; > int x, y, z, w, dx, dy; > - u_int buttons; > - int fin, ges; > + u_int buttons, gesture; > > if ((alps->model & ALPS_DUALPOINT) && alps_sec_proc(sc)) > return; > @@ -1557,28 +1557,44 @@ pms_proc_alps(struct pms_softc *sc) > y = ALPS_YMAX_BEZEL - y + ALPS_YMIN_BEZEL; > > if (alps->wsmode == WSMOUSE_NATIVE) { > - ges = sc->packet[2] & 0x01; > - fin = sc->packet[2] & 0x02; > + if (alps->gesture == ALPS_TAP) { > + /* Report a touch with the tap coordinates. */ > + wsmouse_input(sc->sc_wsmousedev, buttons, > + alps->old_x, alps->old_y, ALPS_PRESSURE, 4, > + WSMOUSE_INPUT_ABSOLUTE_X > + | WSMOUSE_INPUT_ABSOLUTE_Y > + | WSMOUSE_INPUT_ABSOLUTE_Z > + | WSMOUSE_INPUT_ABSOLUTE_W); > + if (z > 0) { > + /* > +* The hardware doesn't send a null > pressure > +* event when dragging starts. > +*/ > + wsmouse_input(sc->sc_wsmousedev, buttons, > + alps->old_x, alps->old_y, 0, 0, > + WSMOUSE_INPUT_ABSOLUTE_X > + | WSMOUSE_INPUT_ABSOLUTE_Y > + | WSMOUSE_INPUT_ABSOLUTE_Z > + | WSMOUSE_INPUT_ABSOLUTE_W); > + } > + } > > - /* Simulate click (tap) */ > - if (ges && !fin) > - z = 35; > - > - /* Generate a null pressure event (needed for tap & drag) > */ > - if (ges && fin && !alps->old_fin) > - z = 0; > - > - /* Generate a width value corresponding to one finger */ > - if (z > 0) > - w = 4; > - else > - w = 0; > + gesture = sc->packet[2] & 0x03; > + if (gesture != ALPS_TAP) { > + w = z ? 4 : 0; > + wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, > w, > + WSMOUSE_INPUT_ABSOLUTE_X > + | WSMOUSE_INPUT_ABSOLUTE_Y > + | WSMOUSE_INPUT_ABSOLUTE_Z > + | WSMOUSE_INPUT_ABSOLUTE_W); > + } > > - wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, w, > - WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y | > - WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W); > + if (alps->gesture != ALPS_DRAG || gesture != ALPS_TAP) > + alps
Re: rt_ifa_del() tweak
On 25/08/15(Tue) 13:01, Martin Pieuchot wrote: > I want to remove this chunked introduce in r7.19 in 1991 by sklower@ > because it no longer makes any sense, it is a layer violation and > does no play well with rt refcounting. > > When this chunk was introduced rtrequest1(RTM_DELETE...) was *not* doing > a route lookup. So this check is now (and since a long time) redundant > with the rtable_lookup()+rtable_mpath_match() done inside rtrequest1(9). > > Remember that when this code was introduced ``ifa'' where linked to > ``rt'' in 1:1 fashion and rtrequest() started by rtfree(9)ing route > entries. That's why you wanted to be sure that the following line was > correct: > > error = rtrequest(cmd, dst, ifa->ifa_addr, ifa->ifa_netmask, > flags | ifa->ifa_flags, &ifa->ifa_rt) > > Ok? Anyone? > Index: net/route.c > === > RCS file: /cvs/src/sys/net/route.c,v > retrieving revision 1.225 > diff -u -p -r1.225 route.c > --- net/route.c 24 Aug 2015 22:11:33 - 1.225 > +++ net/route.c 25 Aug 2015 10:43:32 - > @@ -1230,21 +1230,6 @@ rt_ifa_del(struct ifaddr *ifa, int flags > rt_maskedcopy(dst, deldst, ifa->ifa_netmask); > dst = deldst; > } > - if ((rt = rtalloc(dst, 0, rtableid)) != NULL) { > - rt->rt_refcnt--; > -#ifndef ART > - /* try to find the right route */ > - while (rt && rt->rt_ifa != ifa) > - rt = (struct rtentry *) > - ((struct radix_node *)rt)->rn_dupedkey; > - if (!rt) { > - if (m != NULL) > - (void) m_free(m); > - return (flags & RTF_HOST ? EHOSTUNREACH > - : ENETUNREACH); > - } > -#endif > - } > > memset(&info, 0, sizeof(info)); > info.rti_ifa = ifa; >
Re: uow patch
On 30/08/15(Sun) 09:31, John L. Scarfone wrote: > On Sun, Aug 30, 2015 at 12:32:24PM +0200, Martin Pieuchot stated: > > That's good but the original design of allocating an xfer for every > > usbd_transfer(9) call does not make much sense. > > > > You could allocate two xfers (one for read and one for write) at the > > same time the pipes are opened and free them when they're closed. > > > > You could even allocate the DMA buffer with usb_alloc_buffer() and pass > > the USBD_NO_COPY flag to usbd_transfer(9) to avoid a supplementary copy. > > Thanks for the feedback, Martin. I've been running with your first > suggestion for a bit with no issues. I wonder though if the xfer can't > just be shared for rx and tx? You cannot easily do that because the CPU executing usbd_transfer(9) with a USBD_SYNCHRONOUS flag might sleep. So if you try to submit another transfer at this moment using the same "xfer" descriptor bad things will happen ;) > I also NULLed out the pipes on failure > since it looks like otherwise they'd be re-closed on detach. I'll look > into the usbd_alloc_buffer() idea. That'd great, thanks. I've just committed your diff, good work.
Re: [PATCH] PF: cksum modification & refactor [0/24]
Hello, I'm fine with this change. It certainly posses no thread to SMP branch... > * Initialise pd->pcksum for icmp6 > > - ensures pcksum is set for all known checksummed protocols > - later patches rely on this may be this patch/change should be folded to patch, which really expects pd->pcksum to be set to address of icmp6_cksum field. just to keep things sane in CVS tree. regards sasha
Re: remove RH0 support from ping6(8)
On 30 August 2015 at 15:44, Florian Obser wrote: > RH0 has been deprecated for quite some time now in RFC 5095. It's > quite useless on OpenBSD since our stack unconditionally drops packets > with a RH0 header so you can't get the packet out anyway. > And last but not least it might get in the way if I ever manage to > unify ping(8) and ping6(8). > > OK? > Aye! Kill it with fire.
Re: pool allocator names
Mark Kettenis wrote: > > From: "Ted Unangst" > > Date: Sat, 29 Aug 2015 18:38:45 -0400 > > > > Mark Kettenis wrote: > > > This diff is purely mechanical. This means that it also changes some > > > pool_allocator_nointr into pool_allocator_single where the intention > > > was to signal that the pool would never be used in interrupt context. > > > However, using pool_allocator_single in those cases isn't a big deal > > > as there is no downside. But we could consider changing those into > > > passing NULL and setting the PR_WAITOK flag to the pool_init calls in > > > question. > > > > I would prefer we do this (NULL and WAITOK). I've done some before as I came > > across them, but didn't scan the whole tree. > > This converts sys/isofs. These pools are clearly not used in > interrupt handlers since all pool_get calls specify PR_WAITOK already. > > ok? ok
Re: ntpd(8): Make -n quieter
Michael Reed(m.r...@mykolab.com) on 2015.08.30 14:58:35 -0400: > Hi all, > > If ntpd is run with the -n flag, and /etc/ntpd.conf is parsed without > error, then "Configuration OK" is printed. I don't think this is > particularly useful, as both a lack of an error message and an exit > value of 0 already indicate success in this case. This seems to be the > case for most (many?) programs in the base system, such as doas(1). I like the message. Why is it a problem? /Benno
Like free(), ksh's afree() is NULL-safe
Also, why is fs on line 390 cast to char* when afree() takes void*? Index: emacs.c === RCS file: /cvs/src/bin/ksh/emacs.c,v retrieving revision 1.50 diff -u -p -r1.50 emacs.c --- emacs.c 25 Mar 2015 12:10:52 - 1.50 +++ emacs.c 31 Aug 2015 04:02:29 - @@ -1307,8 +1307,7 @@ static void kb_del(struct kb_entry *k) { TAILQ_REMOVE(&kblist, k, entry); - if (k->args) - free(k->args); + free(k->args); afree(k, AEDIT); } Index: var.c === RCS file: /cvs/src/bin/ksh/var.c,v retrieving revision 1.42 diff -u -p -r1.42 var.c --- var.c 19 Aug 2015 16:05:46 - 1.42 +++ var.c 31 Aug 2015 04:02:29 - @@ -385,8 +385,7 @@ setstr(struct tbl *vq, const char *s, in vq->flag |= ISSET; if ((vq->flag&SPECIAL)) setspec(vq); - if (fs) - afree((char *)fs, ATEMP); + afree((char *)fs, ATEMP); return 1; } @@ -576,8 +575,7 @@ export(struct tbl *vp, const char *val) *xp++ = '='; vp->type = xp - vp->val.s; /* offset to value */ memcpy(xp, val, vallen); - if (op != NULL) - afree((void*)op, vp->areap); + afree((void*)op, vp->areap); } /* @@ -704,8 +702,7 @@ typeset(const char *var, Tflag set, Tfla t->type = 0; } } - if (free_me) - afree((void *) free_me, t->areap); + afree((void *) free_me, t->areap); } } if (!ok) @@ -952,8 +949,7 @@ setspec(struct tbl *vp) switch (special(vp->name)) { case V_PATH: - if (path) - afree(path, APERM); + afree(path, APERM); path = str_save(str_val(vp), APERM); flushcom(1);/* clear tracked aliases */ break; @@ -1059,8 +1055,7 @@ unsetspec(struct tbl *vp) { switch (special(vp->name)) { case V_PATH: - if (path) - afree(path, APERM); + afree(path, APERM); path = str_save(def_path, APERM); flushcom(1);/* clear tracked aliases */ break;
Re: [PATCH] PF: cksum modification & refactor [3/24]
Hello Richard, the code in patch looks good for the first glance. However it seems to me the newly introduced pf_cksum_fixup*() are not called yet. Do you think you can reshuffle changes between your set of patches a bit, so the newly introduced functions will become alive (get called)? Also I think your patch 0/24, you've sent earlier, can be fold here (setting pd->pcksum to point to icmp6 header chksum field). thanks a lot regards sasha