Re: new getformat() for jot(1)
On Wed, Sep 07, 2016 at 12:00:17AM +0200, Martin Natano wrote: > On Sat, Sep 03, 2016 at 04:55:59PM +0100, Theo Buehler wrote: > > The -w flag to jot() allows the user to specify a format string that > > will be passed to printf after it was verified that it contains only one > > conversion specifier. There are some subtle differences what jot's > > getformat() accepts and what printf will do. Thus, I took vfprintf.c and > > carved out whatever is unneeded for jot itself. The result is a slightly > > less ugly function in a separate file. > > Please see some comments below. > > I really tried to understand all the corner cases in the getformat() > function, but couldn't wrap my head around it. I believe it would be > best to just axe the -w flag. Yes, there are probably scripts out there > using it, but I think carrying that burden around is not worth it. Every > possible implementation either is an adapted reimplementation of printf, > or whitewashing the string before passing it to printf(). I would like > to remind of the patch(1) ed script issue that resulted in shell > injection, just because the whitewash code and the actual parser where > not in sync. It's a losing game. Thanks for taking the time of looking into it. I would very much like to ditch -w if there are no objections. It's just plain horrific and a terrible idea to begin with. I can understand the convenience of something like "jot -w '%02d' 10", but everything more complicated seems to be better left to awk, perl or whatever else your preferred scripting language is. There is a similar printf reimplementation in usr.bin/printf/prinf.c which is just as unpleasant, but mandated by POSIX. Below is a diff addressing your comments. > 'boring', 'infinity' and 'randomize' are unused in getformat.c. yes. > How about something like this instead? > > #define is_digit(c) ((c) >= '0' && (c) <= '9') > > This would also allow to remove the to_digit() macro. I agree. > > + sz = sizeof(fmt) - strlen(fmt) - 1; > > The sizeof() doesn't do what you expect it to do here. that was really stupid, thanks. > Previously the error message for 'jot -w %d%d 10' was "jot: too many > conversions", now it is "jot: illegal or unsupported format '%d%d'". I > think the previous error was more helpful. restored previous error message > > + if (ch == '$') > > What is this check supposed to do? There is no '$' case, so the default > will be invoked after 'got reswitch;'. incomplete purging of cases unneeded for jot Index: Makefile === RCS file: /var/cvs/src/usr.bin/jot/Makefile,v retrieving revision 1.5 diff -u -p -r1.5 Makefile --- Makefile10 Jan 2016 01:15:52 - 1.5 +++ Makefile6 Sep 2016 22:35:48 - @@ -1,6 +1,7 @@ # $OpenBSD: Makefile,v 1.5 2016/01/10 01:15:52 tb Exp $ PROG= jot +SRCS= getformat.c jot.c CFLAGS+= -Wall LDADD+=-lm DPADD+=${LIBM} Index: getformat.c === RCS file: getformat.c diff -N getformat.c --- /dev/null 1 Jan 1970 00:00:00 - +++ getformat.c 6 Sep 2016 22:35:58 - @@ -0,0 +1,180 @@ +/* $OpenBSD$ */ +/*- + * Copyright (c) 1990 The Regents of the University of California. + * All rights reserved. + * + * This code is derived from software contributed to Berkeley by + * Chris Torek. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its contributors + *may be used to endorse or promote products derived from this software + *without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* Based on
Re: new getformat() for jot(1)
On Sat, Sep 03, 2016 at 04:55:59PM +0100, Theo Buehler wrote: > The -w flag to jot() allows the user to specify a format string that > will be passed to printf after it was verified that it contains only one > conversion specifier. There are some subtle differences what jot's > getformat() accepts and what printf will do. Thus, I took vfprintf.c and > carved out whatever is unneeded for jot itself. The result is a slightly > less ugly function in a separate file. Please see some comments below. I really tried to understand all the corner cases in the getformat() function, but couldn't wrap my head around it. I believe it would be best to just axe the -w flag. Yes, there are probably scripts out there using it, but I think carrying that burden around is not worth it. Every possible implementation either is an adapted reimplementation of printf, or whitewashing the string before passing it to printf(). I would like to remind of the patch(1) ed script issue that resulted in shell injection, just because the whitewash code and the actual parser where not in sync. It's a losing game. natano > > Index: Makefile > === > RCS file: /var/cvs/src/usr.bin/jot/Makefile,v > retrieving revision 1.5 > diff -u -p -r1.5 Makefile > --- Makefile 10 Jan 2016 01:15:52 - 1.5 > +++ Makefile 3 Sep 2016 15:48:06 - > @@ -1,6 +1,7 @@ > #$OpenBSD: Makefile,v 1.5 2016/01/10 01:15:52 tb Exp $ > > PROG=jot > +SRCS=getformat.c jot.c > CFLAGS+= -Wall > LDADD+= -lm > DPADD+= ${LIBM} > Index: getformat.c > === > RCS file: getformat.c > diff -N getformat.c > --- /dev/null 1 Jan 1970 00:00:00 - > +++ getformat.c 3 Sep 2016 15:55:21 - > @@ -0,0 +1,188 @@ > +/* $OpenBSD$ */ > +/*- > + * Copyright (c) 1990 The Regents of the University of California. > + * All rights reserved. > + * > + * This code is derived from software contributed to Berkeley by > + * Chris Torek. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + *notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + *notice, this list of conditions and the following disclaimer in the > + *documentation and/or other materials provided with the distribution. > + * 3. Neither the name of the University nor the names of its contributors > + *may be used to endorse or promote products derived from this software > + *without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +/* Based on src/lib/libc/stdio/vfprintf.c r1.77 */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +extern int prec; > +extern bool boring; > +extern bool chardata; > +extern bool infinity; > +extern bool intdata; > +extern bool longdata; > +extern bool nosign; > +extern bool randomize; 'boring', 'infinity' and 'randomize' are unused in getformat.c. > + > +int getformat(char *); > + > +/* > + * Macros for converting digits to letters and vice versa > + */ > +#define to_digit(c) ((c) - '0') > +#define is_digit(c) ((unsigned)to_digit(c) <= 9) I would prefer this to be less magic. How about something like this instead? #define is_digit(c) ((c) >= '0' && (c) <= '9') This would also allow to remove the to_digit() macro. > + > +int > +getformat(char *fmt) > +{ > + int ch; /* character from fmt */ > + wchar_t wc; > + mbstate_t ps; > + size_t sz; > + bool firsttime = true; > + > + sz = sizeof(fmt) - strlen(fmt) - 1; The sizeof() doesn't do what you expect it to do here. 'fmt' is just a pointer here, so the value returned is far to low. What we want is the size of the original array instead. $ jot -w 'xxx' 10 jot: -w word too long > + > + memset(, 0, sizeof(ps)); > + /* > + * Scan the
mg - fix modeline segfault
Hi tech@ attaching a fix for the following crash caused by a null pointer dereference while the modeline is trying to work on a unusable display #0 0x0bf6a4e04433 in modeline (wp=0xbf948d9d400, modelinecolor=2) at display.c:800 800 vscreen[n]->v_color = modelinecolor;/* Mode line color. */ (gdb) bt #0 0x0bf6a4e04433 in modeline (wp=0xbf948d9d400, modelinecolor=2) at display.c:800 #1 0x0bf6a4e04ecf in update (modelinecolor=2) at display.c:501 #2 0x0bf6a4e0ee28 in main (argc=Variable "argc" is not available. ) at main.c:199 quite easy to reproduce: 1. start a tmux session 2. split the screen in half (^B ") 3. start mg in one screen 4. resize the mg screen to 2 lines (smallest allow by tmux) 5. by now tmux should be showing unusable display 6. type something or do a modeline command segfault. The interesting thing is that mg works without a crash if it's started from a 2 line display regardless of what you do. So I am having doubts how sane that check for 'unusable' display is. I also assume there might be more places that die when trying to work with an unusable display (I didn't find/hit them yet). Thinking about it made me try another diff. Which removes the 'window is unusable' check completely. So far I havent seen a single crash with it and I can resize the window down to 2 lines and back. I guess I'm asking for an OK for the second diff (or a reason why we should not) versus OK'ing the first one :) Regards, awolk Index: display.c === RCS file: /cvs/src/usr.bin/mg/display.c,v retrieving revision 1.47 diff -u -p -r1.47 display.c --- display.c 3 Apr 2015 22:10:29 - 1.47 +++ display.c 6 Sep 2016 21:15:51 - @@ -797,6 +797,8 @@ modeline(struct mgwin *wp, int modelinec int len; n = wp->w_toprow + wp->w_ntrows;/* Location. */ + if (!vscreen[n]) + return; vscreen[n]->v_color = modelinecolor;/* Mode line color. */ vscreen[n]->v_flag |= (VFCHG | VFHBAD); /* Recompute, display. */ vtmove(n, 0); /* Seek to right line. */ Index: window.c === RCS file: /cvs/src/usr.bin/mg/window.c,v retrieving revision 1.36 diff -u -p -r1.36 window.c --- window.c18 Nov 2015 18:21:06 - 1.36 +++ window.c6 Sep 2016 21:29:48 - @@ -89,12 +89,6 @@ do_redraw(int f, int n, int force) while (wp->w_wndp != NULL) wp = wp->w_wndp; - /* check if too small */ - if (nrow < wp->w_toprow + 3) { - dobeep(); - ewprintf("Display unusable"); - return (FALSE); - } wp->w_ntrows = nrow - wp->w_toprow - 2; sgarbf = TRUE; update(CMODE);
smtpd shutdown cleanup
Previously, all processes would shutdown on receiving SIGINT or SIGTERM. When going down, the parent process would kill all the other process and waitpid() them. Now, only the parent process handles SIGTERM and SIGINT, other processes ignore them. Upon receiving one of these signals, the parent process all imsg sockets and waitpid() for the children. It fatal()s if one of the imsg sockets is closed unexpectedly. Other processes exit() "normally" when one of their imsg socket is closed (except for client connection on the control socket of course). That's how they are supposed to stop now. When doing so, they log as "debug" instead of "info" because useless logs are useless. This makes the shutdown sequence much saner. Eric. Index: ca.c === RCS file: /cvs/src/usr.sbin/smtpd/ca.c,v retrieving revision 1.24 diff -u -p -r1.24 ca.c --- ca.c4 Sep 2016 16:10:31 - 1.24 +++ ca.c6 Sep 2016 19:33:45 - @@ -66,29 +66,14 @@ static uint64_t rsae_reqid = 0; static void ca_shutdown(void) { - log_info("info: ca agent exiting"); + log_debug("debug: ca agent exiting"); _exit(0); } -static void -ca_sig_handler(int sig, short event, void *p) -{ - switch (sig) { - case SIGINT: - case SIGTERM: - ca_shutdown(); - break; - default: - fatalx("ca_sig_handler: unexpected signal"); - } -} - int ca(void) { struct passwd *pw; - struct event ev_sigint; - struct event ev_sigterm; purge_config(PURGE_LISTENERS|PURGE_TABLES|PURGE_RULES); @@ -110,10 +95,8 @@ ca(void) imsg_callback = ca_imsg; event_init(); - signal_set(_sigint, SIGINT, ca_sig_handler, NULL); - signal_set(_sigterm, SIGTERM, ca_sig_handler, NULL); - signal_add(_sigint, NULL); - signal_add(_sigterm, NULL); + signal(SIGINT, SIG_IGN); + signal(SIGTERM, SIG_IGN); signal(SIGPIPE, SIG_IGN); signal(SIGHUP, SIG_IGN); @@ -242,6 +225,9 @@ ca_imsg(struct mproc *p, struct imsg *im int ret = 0; uint64_t id; int v; + + if (imsg == NULL) + ca_shutdown(); if (p->proc == PROC_PARENT) { switch (imsg->hdr.type) { Index: control.c === RCS file: /cvs/src/usr.sbin/smtpd/control.c,v retrieving revision 1.116 diff -u -p -r1.116 control.c --- control.c 4 Sep 2016 16:10:31 - 1.116 +++ control.c 6 Sep 2016 19:33:45 - @@ -63,7 +63,6 @@ static void control_shutdown(void); static void control_listen(void); static void control_accept(int, short, void *); static void control_close(struct ctl_conn *); -static void control_sig_handler(int, short, void *); static void control_dispatch_ext(struct mproc *, struct imsg *); static void control_digest_update(const char *, size_t, int); static void control_broadcast_verbose(int, int); @@ -89,6 +88,12 @@ control_imsg(struct mproc *p, struct ims const void *data; size_t sz; + if (imsg == NULL) { + if (p->proc != PROC_CLIENT) + control_shutdown(); + return; + } + if (p->proc == PROC_PONY) { switch (imsg->hdr.type) { case IMSG_CTL_SMTP_SESSION: @@ -186,19 +191,6 @@ control_imsg(struct mproc *p, struct ims imsg_to_str(imsg->hdr.type)); } -static void -control_sig_handler(int sig, short event, void *p) -{ - switch (sig) { - case SIGINT: - case SIGTERM: - control_shutdown(); - break; - default: - fatalx("control_sig_handler: unexpected signal"); - } -} - int control_create_socket(void) { @@ -245,8 +237,6 @@ int control(void) { struct passwd *pw; - struct event ev_sigint; - struct event ev_sigterm; purge_config(PURGE_EVERYTHING); @@ -271,10 +261,8 @@ control(void) imsg_callback = control_imsg; event_init(); - signal_set(_sigint, SIGINT, control_sig_handler, NULL); - signal_set(_sigterm, SIGTERM, control_sig_handler, NULL); - signal_add(_sigint, NULL); - signal_add(_sigterm, NULL); + signal(SIGINT, SIG_IGN); + signal(SIGTERM, SIG_IGN); signal(SIGPIPE, SIG_IGN); signal(SIGHUP, SIG_IGN); @@ -305,7 +293,7 @@ control(void) static void control_shutdown(void) { - log_info("info: control process exiting"); + log_debug("debug: control agent exiting"); _exit(0); } Index: lka.c === RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v retrieving revision 1.196 diff -u -p -r1.196 lka.c --- lka.c 4 Sep
Re: Default softraid crypto PBKDF2 rounds
Il 6 settembre 2016 14:56:32 CEST, Filippo Valsordaha scritto: >Hello, > >I recently had the occasion to dive into the softraid crypto code [1] >and was quite pleased with the cleanliness of it all. However, I found >surprising the default value of 8k PBKDF2 rounds. > >I know it is easy to override and I should have RTFM, but I (naively, >I'll admit) assumed OpenBSD would pick very robust defaults, erring on >the conservative side. Is it maybe time to bump it up, or pick it based >on a quick machine benchmark? > >If there's consensus I might also provide a patch for the live >benchmark >option. yes, autodetection of a sensible value would be cool... Cheers David
Re: mg - Check pointer before calling showbuffer()
On Tue, Sep 06, 2016 at 05:10:39PM +, Mark Lumsden wrote: > Source Joachim Nilsson: > > Found by Coverity Scan. The popbuf() function iterated over a list to > find a wp pointer, then sent it to showbuffer() which immediately went > ahead and dereferenced it. This patch simply adds a NULL pointer check > before calling showbuffer(), if NULL then just return NULL to callee. > > The missing NULL check is actually referenced in a comment a few lines > earlier in the code. ok? > > -lum > I tested the diff and that's OK awolk@ with a slight suggestion to also grab the for loop with { } since you are already adding it for the dangling else. > Index: buffer.c > === > RCS file: /cvs/src/usr.bin/mg/buffer.c,v > retrieving revision 1.101 > diff -u -p -u -p -r1.101 buffer.c > --- buffer.c 31 Aug 2016 12:22:28 - 1.101 > +++ buffer.c 6 Sep 2016 17:04:22 - > @@ -713,12 +713,16 @@ popbuf(struct buffer *bp, int flags) > > while (wp != NULL && wp == curwp) > wp = wp->w_wndp; > - } else > + } else { > for (wp = wheadp; wp != NULL; wp = wp->w_wndp) > if (wp->w_bufp == bp) { > wp->w_rflag |= WFFULL | WFFRAME; > return (wp); > } > + } > + if (!wp) > + return (NULL); > + > if (showbuffer(bp, wp, WFFULL) != TRUE) > return (NULL); > return (wp); >
Re: Improve error message in rcctl(8)
Tue, 06 Sep 2016 22:41:53 +0200 Jeremie Courreges-Anglas> li...@wrant.com writes: > > > Tue, 6 Sep 2016 19:54:33 + Robert Peichaer > >> > Hi tech@, > >> > > >> > Daemon names historically match Antoine's alphanumeric proposal, and I > >> > think underscore is a bit too much, if it's present use minus instead. > >> > The logic behind this? Match this to word termination symbols in ksh. > >> > > >> > Kind regards, > >> > Anton > >> > >> $ find /usr/ports -name '*_*.rc' -type f | wc -l > >> 85 > > > > Hi Robert, > > > > I'll not be arguing about this, from usability end point the underscore > > is pretty bad especially if you want to go back word at a time. If you > > think the underscore is enough, I agree yet I would still prefer minus, > > where we have variation & need to go back to the differentiation point. > > On such a trivial subject, you suggest costly changes to existing > practices, just to please your preferences. Of course, without even > publishing a possible implementation. Hi Jeremie, Could you, please just fix the word delimiters in ksh(1) no? Then this costly change is further not acceptable and I merely mentioned it, only to selfishly please my preferences, sorry for the noise and time waste. Kind regards, Anton
Re: Improve error message in rcctl(8)
li...@wrant.com writes: > Tue, 6 Sep 2016 19:54:33 + Robert Peichaer>> > Hi tech@, >> > >> > Daemon names historically match Antoine's alphanumeric proposal, and I >> > think underscore is a bit too much, if it's present use minus instead. >> > The logic behind this? Match this to word termination symbols in ksh. >> > >> > Kind regards, >> > Anton >> >> $ find /usr/ports -name '*_*.rc' -type f | wc -l >> 85 > > Hi Robert, > > I'll not be arguing about this, from usability end point the underscore > is pretty bad especially if you want to go back word at a time. If you > think the underscore is enough, I agree yet I would still prefer minus, > where we have variation & need to go back to the differentiation point. On such a trivial subject, you suggest costly changes to existing practices, just to please your preferences. Of course, without even publishing a possible implementation. I'd find it funny, if only you were not wasting the time of the folks on this mailing-list. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: Improve error message in rcctl(8)
On Tue, Sep 06, 2016 at 04:09:49PM -0400, Anthony Coulter wrote: > Regarding Jiri's suggestion: Here is a diff that makes > `rcctl ls all' only list executable files with valid service > names. > > This diff also fixes two problems with my original submission: > 1. The use of `[' instead of `[[' causes filename expansion to >take place on the right-hand side of the comparison; you get >different results depending on which directory you're sitting >in when you test. I have switched to `[[' to fix that problem. > 2. There was a stray closing brace that somehow did not cause >problems in testing. That's not enough. It cannot start with a digit either. I'm working on a better diff. > Index: usr.sbin/rcctl/rcctl.sh > === > RCS file: /open/anoncvs/cvs/src/usr.sbin/rcctl/rcctl.sh,v > retrieving revision 1.104 > diff -u -p -r1.104 rcctl.sh > --- usr.sbin/rcctl/rcctl.sh 30 Jul 2016 06:25:21 - 1.104 > +++ usr.sbin/rcctl/rcctl.sh 6 Sep 2016 20:03:33 - > @@ -53,8 +53,8 @@ ls_rcscripts() > > cd /etc/rc.d && set -- * > for _s; do > - [[ ${_s} = *.* ]] && continue > - [ ! -d "${_s}" ] && echo "${_s}" > + [[ "${_s}" != +([_0-9A-Za-z]) ]] && continue > + [ -x "${_s}" ] && echo "${_s}" > done > } > > @@ -182,7 +182,7 @@ svc_is_meta() > svc_is_special() > { > local _svc=$1 > - [ -n "${_svc}" ] || return > + [[ "${_svc}" = +([_0-9A-Za-z]) ]] || return > > local _cached _ret > > -- Antoine
Re: Improve error message in rcctl(8)
Tue, 6 Sep 2016 19:54:33 + Robert Peichaer> > Hi tech@, > > > > Daemon names historically match Antoine's alphanumeric proposal, and I > > think underscore is a bit too much, if it's present use minus instead. > > The logic behind this? Match this to word termination symbols in ksh. > > > > Kind regards, > > Anton > > $ find /usr/ports -name '*_*.rc' -type f | wc -l > 85 Hi Robert, I'll not be arguing about this, from usability end point the underscore is pretty bad especially if you want to go back word at a time. If you think the underscore is enough, I agree yet I would still prefer minus, where we have variation & need to go back to the differentiation point. Kind regards, Anton
Re: Improve error message in rcctl(8)
Regarding Jiri's suggestion: Here is a diff that makes `rcctl ls all' only list executable files with valid service names. This diff also fixes two problems with my original submission: 1. The use of `[' instead of `[[' causes filename expansion to take place on the right-hand side of the comparison; you get different results depending on which directory you're sitting in when you test. I have switched to `[[' to fix that problem. 2. There was a stray closing brace that somehow did not cause problems in testing. Index: usr.sbin/rcctl/rcctl.sh === RCS file: /open/anoncvs/cvs/src/usr.sbin/rcctl/rcctl.sh,v retrieving revision 1.104 diff -u -p -r1.104 rcctl.sh --- usr.sbin/rcctl/rcctl.sh 30 Jul 2016 06:25:21 - 1.104 +++ usr.sbin/rcctl/rcctl.sh 6 Sep 2016 20:03:33 - @@ -53,8 +53,8 @@ ls_rcscripts() cd /etc/rc.d && set -- * for _s; do - [[ ${_s} = *.* ]] && continue - [ ! -d "${_s}" ] && echo "${_s}" + [[ "${_s}" != +([_0-9A-Za-z]) ]] && continue + [ -x "${_s}" ] && echo "${_s}" done } @@ -182,7 +182,7 @@ svc_is_meta() svc_is_special() { local _svc=$1 - [ -n "${_svc}" ] || return + [[ "${_svc}" = +([_0-9A-Za-z]) ]] || return local _cached _ret
Re: Improve error message in rcctl(8)
> Hi tech@, > > Daemon names historically match Antoine's alphanumeric proposal, and I > think underscore is a bit too much, if it's present use minus instead. > The logic behind this? Match this to word termination symbols in ksh. > > Kind regards, > Anton $ find /usr/ports -name '*_*.rc' -type f | wc -l 85 -- -=[rpe]=-
Re: Improve error message in rcctl(8)
Tue, 6 Sep 2016 21:04:55 +0200 Antoine Jacoutot> On Tue, Sep 06, 2016 at 09:01:08PM +0200, ludovic coues wrote: > > 2016-09-06 20:53 GMT+02:00 Antoine Jacoutot : > > > On Tue, Sep 06, 2016 at 12:29:58PM -0400, Anthony Coulter wrote: > > >> Sometimes when I restart a service after changing its configuration file > > >> I accidentally type: > > >> > > >> # rcctl restart smtpd.conf > > >> /usr/sbin/rcctl: ${cached_svc_is_special_smtpd.conf}: bad substitution > > >> /usr/sbin/rcctl[556]: set: cached_svc_is_special_smtpd.conf: is not an > > >> identifier > > >> rcctl: service smtpd.conf does not exist > > >> > > >> The message about a bad substitution is not helpful to the user, who > > >> only needs to know that smtpd.conf is not a service. > > >> > > >> The problem is the period in "smtpd.conf". Line 189 of rcctl fails: > > >> _cached=$(eval print \${cached_svc_is_special_${_svc}}) > > >> > > >> Special service names are thus limited to underscores and alphanumerics > > >> because they're concatenated into shell variable names. So instead of > > >> checking for [ -n ${_svc} ] at the top of svc_is_special, we ought to > > >> check that ${_svc} contains only legal characters. > > >> > > >> I check only in svc_is_special and not in any of the other places that > > >> test [ -n ${_svc} ] my only goal is to fix the error message people get > > >> when they try to start or enable configuration files, and this is the > > >> only place that needs the error. Adding a similar check to svc_is_avail > > >> would block an error message when someone creates an executable file > > >> called /etc/rc.d/foo.bar and then calls "rcctl enable foo.bar", but in > > >> that case I think the message "${foo.bar_flags}: bad substitution" is > > >> more helpful---the user is trying to create a service with an illegal > > >> name and the system is telling him why it will never work. > > > > > > Yes I agree this should be fixed. > > > What about this? > > > > > > Index: rcctl.sh > > > === > > > RCS file: /cvs/src/usr.sbin/rcctl/rcctl.sh,v > > > retrieving revision 1.104 > > > diff -u -p -r1.104 rcctl.sh > > > --- rcctl.sh30 Jul 2016 06:25:21 - 1.104 > > > +++ rcctl.sh6 Sep 2016 18:51:18 - > > > @@ -139,7 +139,7 @@ rcconf_edit_end() > > > svc_is_avail() > > > { > > > local _svc=$1 > > > - [ -n "${_svc}" ] || return > > > + [[ "${_svc}" == +([_/+[:alnum:]]) ]] || return > > > > > > [ -x "/etc/rc.d/${_svc}" ] && return > > > svc_is_special ${_svc} > > > > > > > > > -- > > > Antoine > > > > > > > If people are using daemon named like fastcgi.exemple.com, this will > > break there config. > > The daemon name has no importance. Only the rc.d script name does. > And in this case, we can install it as /etc/rc.d/fastcgi_exemple_com > We need to draw a line somewhere to prevent the crazyness... Hi tech@, Daemon names historically match Antoine's alphanumeric proposal, and I think underscore is a bit too much, if it's present use minus instead. The logic behind this? Match this to word termination symbols in ksh. Kind regards, Anton
Re: Improve error message in rcctl(8)
Could a change solve also this annoying situations? (saved files by editors...) # rcctl ls all | grep ^tor tor tor_2 tor_2~ j.
Re: Improve error message in rcctl(8)
Antoine writes: > What about this? > + [[ "${_svc}" == +([_/+[:alnum:]]) ]] || return That doesn't fix the problem. You cannot use plus signs or slashes in a service name because the options to a service are set in rc.conf.local with the line foo_flags="" where `foo' is replaced by the service name. The ksh man page states that only letters, numbers, and underscores can be used in variable names. Ludovic writes: > If people are using daemon named like fastcgi.exemple.com, this will > break there config. They cannot use that name anyway, for the same reason listed above. Service names are already restricted to letters, digits, and underscores. The only thing we can do here is validate user input to the rcctl command. Anthony
Re: Improve error message in rcctl(8)
On Tue, Sep 06, 2016 at 09:01:08PM +0200, ludovic coues wrote: > 2016-09-06 20:53 GMT+02:00 Antoine Jacoutot: > > On Tue, Sep 06, 2016 at 12:29:58PM -0400, Anthony Coulter wrote: > >> Sometimes when I restart a service after changing its configuration file > >> I accidentally type: > >> > >> # rcctl restart smtpd.conf > >> /usr/sbin/rcctl: ${cached_svc_is_special_smtpd.conf}: bad substitution > >> /usr/sbin/rcctl[556]: set: cached_svc_is_special_smtpd.conf: is not an > >> identifier > >> rcctl: service smtpd.conf does not exist > >> > >> The message about a bad substitution is not helpful to the user, who > >> only needs to know that smtpd.conf is not a service. > >> > >> The problem is the period in "smtpd.conf". Line 189 of rcctl fails: > >> _cached=$(eval print \${cached_svc_is_special_${_svc}}) > >> > >> Special service names are thus limited to underscores and alphanumerics > >> because they're concatenated into shell variable names. So instead of > >> checking for [ -n ${_svc} ] at the top of svc_is_special, we ought to > >> check that ${_svc} contains only legal characters. > >> > >> I check only in svc_is_special and not in any of the other places that > >> test [ -n ${_svc} ] my only goal is to fix the error message people get > >> when they try to start or enable configuration files, and this is the > >> only place that needs the error. Adding a similar check to svc_is_avail > >> would block an error message when someone creates an executable file > >> called /etc/rc.d/foo.bar and then calls "rcctl enable foo.bar", but in > >> that case I think the message "${foo.bar_flags}: bad substitution" is > >> more helpful---the user is trying to create a service with an illegal > >> name and the system is telling him why it will never work. > > > > Yes I agree this should be fixed. > > What about this? > > > > Index: rcctl.sh > > === > > RCS file: /cvs/src/usr.sbin/rcctl/rcctl.sh,v > > retrieving revision 1.104 > > diff -u -p -r1.104 rcctl.sh > > --- rcctl.sh30 Jul 2016 06:25:21 - 1.104 > > +++ rcctl.sh6 Sep 2016 18:51:18 - > > @@ -139,7 +139,7 @@ rcconf_edit_end() > > svc_is_avail() > > { > > local _svc=$1 > > - [ -n "${_svc}" ] || return > > + [[ "${_svc}" == +([_/+[:alnum:]]) ]] || return > > > > [ -x "/etc/rc.d/${_svc}" ] && return > > svc_is_special ${_svc} > > > > > > -- > > Antoine > > > > If people are using daemon named like fastcgi.exemple.com, this will > break there config. The daemon name has no importance. Only the rc.d script name does. And in this case, we can install it as /etc/rc.d/fastcgi_exemple_com We need to draw a line somewhere to prevent the crazyness... -- Antoine
Re: Improve error message in rcctl(8)
2016-09-06 20:53 GMT+02:00 Antoine Jacoutot: > On Tue, Sep 06, 2016 at 12:29:58PM -0400, Anthony Coulter wrote: >> Sometimes when I restart a service after changing its configuration file >> I accidentally type: >> >> # rcctl restart smtpd.conf >> /usr/sbin/rcctl: ${cached_svc_is_special_smtpd.conf}: bad substitution >> /usr/sbin/rcctl[556]: set: cached_svc_is_special_smtpd.conf: is not an >> identifier >> rcctl: service smtpd.conf does not exist >> >> The message about a bad substitution is not helpful to the user, who >> only needs to know that smtpd.conf is not a service. >> >> The problem is the period in "smtpd.conf". Line 189 of rcctl fails: >> _cached=$(eval print \${cached_svc_is_special_${_svc}}) >> >> Special service names are thus limited to underscores and alphanumerics >> because they're concatenated into shell variable names. So instead of >> checking for [ -n ${_svc} ] at the top of svc_is_special, we ought to >> check that ${_svc} contains only legal characters. >> >> I check only in svc_is_special and not in any of the other places that >> test [ -n ${_svc} ] my only goal is to fix the error message people get >> when they try to start or enable configuration files, and this is the >> only place that needs the error. Adding a similar check to svc_is_avail >> would block an error message when someone creates an executable file >> called /etc/rc.d/foo.bar and then calls "rcctl enable foo.bar", but in >> that case I think the message "${foo.bar_flags}: bad substitution" is >> more helpful---the user is trying to create a service with an illegal >> name and the system is telling him why it will never work. > > Yes I agree this should be fixed. > What about this? > > Index: rcctl.sh > === > RCS file: /cvs/src/usr.sbin/rcctl/rcctl.sh,v > retrieving revision 1.104 > diff -u -p -r1.104 rcctl.sh > --- rcctl.sh30 Jul 2016 06:25:21 - 1.104 > +++ rcctl.sh6 Sep 2016 18:51:18 - > @@ -139,7 +139,7 @@ rcconf_edit_end() > svc_is_avail() > { > local _svc=$1 > - [ -n "${_svc}" ] || return > + [[ "${_svc}" == +([_/+[:alnum:]]) ]] || return > > [ -x "/etc/rc.d/${_svc}" ] && return > svc_is_special ${_svc} > > > -- > Antoine > If people are using daemon named like fastcgi.exemple.com, this will break there config. -- Cordialement, Coues Ludovic +336 148 743 42
Re: Fix Wacom Intuos S 2 descriptor and make wsmouse work
On Tue, Sep 06, 2016 at 02:19:28PM +0200, Ulf Brosziewski wrote: > Just a remark on your use of the wsmouse interface (which isn't well > known and documented yet): wsmouse_set is a function for uncommon > cases. To report a pair of absolute coordinates use wsmouse_position, > that is, instead of > wsmouse_set(ms->sc_wsmousedev, WSMOUSE_ABS_X, x, 0); > wsmouse_set(ms->sc_wsmousedev, WSMOUSE_ABS_Y, y, 0); > you should use > wsmouse_position(ms->sc_wsmousedev, x, y); > Likewise, for reporting the button state there is > wsmouse_buttons(ms->sc_wsmousedev, buttons); > There is no need for the WSMOUSE_INPUT macro here. Seems like a cleaner API indeed, but those functions don't seem to work for me. Both mouse movement and button reporting stop functioning when replaced by those calls. How old is this API, might it be some outdated sources on my side? Something else I'm doing wrong? Frank
Re: Improve error message in rcctl(8)
On Tue, Sep 06, 2016 at 12:29:58PM -0400, Anthony Coulter wrote: > Sometimes when I restart a service after changing its configuration file > I accidentally type: > > # rcctl restart smtpd.conf > /usr/sbin/rcctl: ${cached_svc_is_special_smtpd.conf}: bad substitution > /usr/sbin/rcctl[556]: set: cached_svc_is_special_smtpd.conf: is not an > identifier > rcctl: service smtpd.conf does not exist > > The message about a bad substitution is not helpful to the user, who > only needs to know that smtpd.conf is not a service. > > The problem is the period in "smtpd.conf". Line 189 of rcctl fails: > _cached=$(eval print \${cached_svc_is_special_${_svc}}) > > Special service names are thus limited to underscores and alphanumerics > because they're concatenated into shell variable names. So instead of > checking for [ -n ${_svc} ] at the top of svc_is_special, we ought to > check that ${_svc} contains only legal characters. > > I check only in svc_is_special and not in any of the other places that > test [ -n ${_svc} ] my only goal is to fix the error message people get > when they try to start or enable configuration files, and this is the > only place that needs the error. Adding a similar check to svc_is_avail > would block an error message when someone creates an executable file > called /etc/rc.d/foo.bar and then calls "rcctl enable foo.bar", but in > that case I think the message "${foo.bar_flags}: bad substitution" is > more helpful---the user is trying to create a service with an illegal > name and the system is telling him why it will never work. Yes I agree this should be fixed. What about this? Index: rcctl.sh === RCS file: /cvs/src/usr.sbin/rcctl/rcctl.sh,v retrieving revision 1.104 diff -u -p -r1.104 rcctl.sh --- rcctl.sh30 Jul 2016 06:25:21 - 1.104 +++ rcctl.sh6 Sep 2016 18:51:18 - @@ -139,7 +139,7 @@ rcconf_edit_end() svc_is_avail() { local _svc=$1 - [ -n "${_svc}" ] || return + [[ "${_svc}" == +([_/+[:alnum:]]) ]] || return [ -x "/etc/rc.d/${_svc}" ] && return svc_is_special ${_svc} -- Antoine
Re: subr_tree for netinet/ip_ipsp.[ch]
On Sep 6, 2016 07:07, "David Gwynne"wrote: > > this gives us back 5k on amd64. > > ok? > I can't test this ATM, but I endorse the idea. The diff looks good to me.
mg - Check pointer before calling showbuffer()
Source Joachim Nilsson: Found by Coverity Scan. The popbuf() function iterated over a list to find a wp pointer, then sent it to showbuffer() which immediately went ahead and dereferenced it. This patch simply adds a NULL pointer check before calling showbuffer(), if NULL then just return NULL to callee. The missing NULL check is actually referenced in a comment a few lines earlier in the code. ok? -lum Index: buffer.c === RCS file: /cvs/src/usr.bin/mg/buffer.c,v retrieving revision 1.101 diff -u -p -u -p -r1.101 buffer.c --- buffer.c31 Aug 2016 12:22:28 - 1.101 +++ buffer.c6 Sep 2016 17:04:22 - @@ -713,12 +713,16 @@ popbuf(struct buffer *bp, int flags) while (wp != NULL && wp == curwp) wp = wp->w_wndp; - } else + } else { for (wp = wheadp; wp != NULL; wp = wp->w_wndp) if (wp->w_bufp == bp) { wp->w_rflag |= WFFULL | WFFRAME; return (wp); } + } + if (!wp) + return (NULL); + if (showbuffer(bp, wp, WFFULL) != TRUE) return (NULL); return (wp);
Improve error message in rcctl(8)
Sometimes when I restart a service after changing its configuration file I accidentally type: # rcctl restart smtpd.conf /usr/sbin/rcctl: ${cached_svc_is_special_smtpd.conf}: bad substitution /usr/sbin/rcctl[556]: set: cached_svc_is_special_smtpd.conf: is not an identifier rcctl: service smtpd.conf does not exist The message about a bad substitution is not helpful to the user, who only needs to know that smtpd.conf is not a service. The problem is the period in "smtpd.conf". Line 189 of rcctl fails: _cached=$(eval print \${cached_svc_is_special_${_svc}}) Special service names are thus limited to underscores and alphanumerics because they're concatenated into shell variable names. So instead of checking for [ -n ${_svc} ] at the top of svc_is_special, we ought to check that ${_svc} contains only legal characters. I check only in svc_is_special and not in any of the other places that test [ -n ${_svc} ] my only goal is to fix the error message people get when they try to start or enable configuration files, and this is the only place that needs the error. Adding a similar check to svc_is_avail would block an error message when someone creates an executable file called /etc/rc.d/foo.bar and then calls "rcctl enable foo.bar", but in that case I think the message "${foo.bar_flags}: bad substitution" is more helpful---the user is trying to create a service with an illegal name and the system is telling him why it will never work. Regards, Anthony Index: usr.sbin/rcctl/rcctl.sh === RCS file: /open/anoncvs/cvs/src/usr.sbin/rcctl/rcctl.sh,v retrieving revision 1.104 diff -u -p -u -r1.104 rcctl.sh --- usr.sbin/rcctl/rcctl.sh 30 Jul 2016 06:25:21 - 1.104 +++ usr.sbin/rcctl/rcctl.sh 6 Sep 2016 15:07:47 - @@ -182,7 +182,7 @@ svc_is_meta() svc_is_special() { local _svc=$1 - [ -n "${_svc}" ] || return + [ "${_svc}" = +([_0-9A-Za-z])} ] || return local _cached _ret
Re: Default softraid crypto PBKDF2 rounds
On Tue, Sep 06, 2016 at 01:56:32PM +0100, Filippo Valsorda wrote: > Hello, > > I recently had the occasion to dive into the softraid crypto code [1] > and was quite pleased with the cleanliness of it all. However, I found > surprising the default value of 8k PBKDF2 rounds. > > I know it is easy to override and I should have RTFM, but I (naively, > I'll admit) assumed OpenBSD would pick very robust defaults, erring on > the conservative side. Is it maybe time to bump it up, or pick it based > on a quick machine benchmark? > > If there's consensus I might also provide a patch for the live benchmark > option. > > Thank you > > [1]: https://blog.filippo.io/so-i-lost-my-openbsd-fde-password/ Since we do something like that for password bcrypt I'd say we are interested. -Otto
Re: "route add -ifp pppoe..", take care if updating remote boxes
On 06/09/16(Tue) 14:03, Stuart Henderson wrote: > [...] > And > if you're trying to "route change" to a non-allowed address when > you _already_ have a default route, you hit a repeatable kassert > '"rt->rt_gwroute != NULL" failed: file "../../../../net/route.c", > line 221'. (see below for trace/table for this one). Diff below fixes that. Only free the old cached route if the new one is valid. ok? Index: net/route.c === RCS file: /cvs/src/sys/net/route.c,v retrieving revision 1.325 diff -u -p -r1.325 route.c --- net/route.c 4 Sep 2016 15:45:42 - 1.325 +++ net/route.c 6 Sep 2016 13:33:11 - @@ -382,7 +382,6 @@ rt_setgwroute(struct rtentry *rt, u_int KERNEL_ASSERT_LOCKED(); KASSERT(ISSET(rt->rt_flags, RTF_GATEWAY)); - KASSERT(rt->rt_gwroute == NULL); /* If we cannot find a valid next hop bail. */ nhrt = rt_match(rt->rt_gateway, NULL, RT_RESOLVE, rtable_l2(rtableid)); @@ -404,6 +403,10 @@ rt_setgwroute(struct rtentry *rt, u_int return (ELOOP); } + /* Next hop is valid so remove possible old cache. */ + rt_putgwroute(rt); + KASSERT(rt->rt_gwroute == NULL); + /* * If the MTU of next hop is 0, this will reset the MTU of the * route to run PMTUD again from scratch. @@ -1139,10 +1142,8 @@ rt_setgate(struct rtentry *rt, struct so } memmove(rt->rt_gateway, gate, glen); - if (ISSET(rt->rt_flags, RTF_GATEWAY)) { - rt_putgwroute(rt); + if (ISSET(rt->rt_flags, RTF_GATEWAY)) return (rt_setgwroute(rt, rtableid)); - } return (0); }
"route add -ifp pppoe..", take care if updating remote boxes
Now g2k16 is over I thought I'd update my home router from 6.0 to (self-built) -current. I know this area is in flux at the moment, posting to tech rather than mailing specific devs so that people know to take a bit more care than usual when updating remote machines that are doing this. "route add -ifp pppoeX $ip" is a bit problematic at the moment, the address you are adding must be assigned as the dest address of the interface, otherwise it cannot be added. In the normal case you get EHOSTUNREACH, if you're trying to add when you already have a default route, ELOOP ("Too many levels of symbolic links"). And if you're trying to "route change" to a non-allowed address when you _already_ have a default route, you hit a repeatable kassert '"rt->rt_gwroute != NULL" failed: file "../../../../net/route.c", line 221'. (see below for trace/table for this one). I noticed this because I was using different bogus "route add" destination addresses as a hold-over from when I had multiple pppoe interfaces on this machine, but I think there's also a race when you use the "0.0.0.1" pppoe-magic; if IPCP completes and changes the dest address before the "route add" command runs, you might not be able to add the default route because that address is no longer on the interface. ... panic: kernel diagnostic assertion "rt->rt_gwroute != NULL" failed: file "../../../../net/route.c", line 221 Stopped at Debugger+0x9: leave TIDPIDUID PRFLAGS PFLAGS CPU COMMAND *93400 93400 0 0x14000 0x2001 softnet Debugger() at Debugger+0x9 panic() at panic+0xfe __assert() at __assert+0x25 rtisvalid() at rtisvalid+0x63 rt_hash() at rt_hash+0x3a rtable_match() at rtable_match+0x97 rt_match() at rt_match+0x5b in_ouraddr() at in_ouraddr+0x83 ipv4_input() at ipv4_input+0x2d2 ipintr() at ipintr+0x1e if_netisr() at if_netisr+0x105 taskq_thread() at taskq_thread+0x6c end trace frame: 0x0, count: 3 https://www.openbsd.org/ddb.html describes the minimum info required in bug reports. Insufficient info makes it difficult to find and fix bugs. ddb{1}> call db_show_arptab Route tree for AF_INET rtentry=0xff00612a89b8 flags=0x803 refcnt=3 use=1318 expire=0 rtableid=0 key=[16,2,0,0,0,0,0,0,0,0,0,0,0,0,0,0] plen=0 gw=[16,2,0,0,0,0,0,1,0,0,0,0,0,0,0,0] ifidx=19 ifa=0x80711100 ifa_addr=[16,2,0,0,82,68,199,142,0,0,0,0,0,0,0,0] ifa_dsta=[16,2,0,0,62,3,80,17,0,0,0,0,0,0,0,0] ifa_mask=[8,0,0,0,255,255,255,255] flags=0x0, refcnt=2, metric=0 gwroute=0x0 llinfo=0x0 rtentry=0xff0076e713f8 flags=0x800101 refcnt=1 use=0 expire=5 rtableid=0 key=[16,2,0,0,10,0,0,0,0,0,0,0,0,0,0,0] plen=24 gw=[16,2,0,0,10,0,0,140,0,0,0,0,0,0,0,0] ifidx=14 ifa=0x80704e00 ifa_addr=[16,2,0,0,10,0,0,140,0,0,0,0,0,0,0,0] ifa_dsta=[16,2,0,0,10,0,0,255,0,0,0,0,0,0,0,0] ifa_mask=[7,0,0,0,255,255,255] flags=0x0, refcnt=3, metric=0 gwroute=0x0 llinfo=0x0 rtentry=0xff0076e71388 flags=0x200405 refcnt=1 use=0 expire=0 rtableid=0 key=[16,2,0,0,10,0,0,140,0,0,0,0,0,0,0,0] plen=32 gw=[32,18,14,0,6,5,6,0,118,108,97,110,54,0,13,185,65,126,72,0,0,0,0,0, 0,0,0,0,0,0,0,0] ifidx=14 ifa=0x80704e00 ifa_addr=[16,2,0,0,10,0,0,140,0,0,0,0,0,0,0,0] ifa_dsta=[16,2,0,0,10,0,0,255,0,0,0,0,0,0,0,0] ifa_mask=[7,0,0,0,255,255,255] flags=0x0, refcnt=3, metric=0 gwroute=0x0 llinfo=0xff0076e75150 rtentry=0xff0076e71468 flags=0x45 refcnt=1 use=0 expire=0 rtableid=0 key=[16,2,0,0,10,0,0,255,0,0,0,0,0,0,0,0] plen=32 gw=[16,2,0,0,10,0,0,140,0,0,0,0,0,0,0,0] ifidx=14 ifa=0x80704e00 ifa_addr=[16,2,0,0,10,0,0,140,0,0,0,0,0,0,0,0] ifa_dsta=[16,2,0,0,10,0,0,255,0,0,0,0,0,0,0,0] ifa_mask=[7,0,0,0,255,255,255] flags=0x0, refcnt=3, metric=0 gwroute=0x0 llinfo=0x0 rtentry=0xff0076e71008 flags=0x800101 refcnt=2 use=2 expire=5 rtableid=0 key=[16,2,0,0,10,15,8,0,0,0,0,0,0,0,0,0] plen=22 gw=[16,2,0,0,10,15,8,1,0,0,0,0,0,0,0,0] ifidx=12 ifa=0x806ffc00 ifa_addr=[16,2,0,0,10,15,8,1,0,0,0,0,0,0,0,0] ifa_dsta=[16,2,0,0,10,15,11,255,0,0,0,0,0,0,0,0] ifa_mask=[7,0,0,0,255,255,252] flags=0x0, refcnt=4, metric=0 gwroute=0x0 llinfo=0x0 rtentry=0xff006872a8c0 flags=0x800101 refcnt=4 use=0 expire=5 rtableid=0 key=[16,2,0,0,10,15,3,0,0,0,0,0,0,0,0,0] plen=24 gw=[16,2,0,0,10,15,3,1,0,0,0,0,0,0,0,0] ifidx=9 ifa=0x806ff400 ifa_addr=[16,2,0,0,10,15,3,1,0,0,0,0,0,0,0,0]
Default softraid crypto PBKDF2 rounds
Hello, I recently had the occasion to dive into the softraid crypto code [1] and was quite pleased with the cleanliness of it all. However, I found surprising the default value of 8k PBKDF2 rounds. I know it is easy to override and I should have RTFM, but I (naively, I'll admit) assumed OpenBSD would pick very robust defaults, erring on the conservative side. Is it maybe time to bump it up, or pick it based on a quick machine benchmark? If there's consensus I might also provide a patch for the live benchmark option. Thank you [1]: https://blog.filippo.io/so-i-lost-my-openbsd-fde-password/
Re: sysmerge.8: Mention PAGER behavior when undefined/empty
On Mon, Sep 05, 2016 at 05:56:03PM -0400, Michael Reed wrote: > As is done in other man pages. Committed, thank you. > === > RCS file: /cvs/src/usr.sbin/sysmerge/sysmerge.8,v > retrieving revision 1.78 > diff -u -p -r1.78 sysmerge.8 > --- sysmerge.8 2 Sep 2016 12:17:33 - 1.78 > +++ sysmerge.8 5 Sep 2016 21:54:28 - > @@ -126,6 +126,11 @@ the default is > .Xr vi 1 . > .It Ev PAGER > Specifies the pagination program to use. > +If > +.Ev PAGER > +is empty or not set, > +.Xr more 1 > +will be used. > .El > .Sh FILES > .Bl -tag -width "/var/sysmerge/xetc.tgz" -compact > -- Antoine
Re: Fix Wacom Intuos S 2 descriptor and make wsmouse work
Just a remark on your use of the wsmouse interface (which isn't well known and documented yet): wsmouse_set is a function for uncommon cases. To report a pair of absolute coordinates use wsmouse_position, that is, instead of wsmouse_set(ms->sc_wsmousedev, WSMOUSE_ABS_X, x, 0); wsmouse_set(ms->sc_wsmousedev, WSMOUSE_ABS_Y, y, 0); you should use wsmouse_position(ms->sc_wsmousedev, x, y); Likewise, for reporting the button state there is wsmouse_buttons(ms->sc_wsmousedev, buttons); There is no need for the WSMOUSE_INPUT macro here. On 09/05/2016 09:04 PM, Frank Groeneveld wrote: > On Sun, Sep 04, 2016 at 02:25:06PM +0200, Martin Pieuchot wrote: >>> - One bug still left: when the device is attached after X has started, >>> it seems the scaling is done wrongly. I had this problem with the >>> hacked ums driver as well. Most of the time it is fixed by switching >>> between console and X. > > Correction: the problem can only be fixed by making sure the device is > plugged in before starting X. After suspend a restart of X is needed to > make scaling work again. > >> >> This is a common problem for drivers needing calibration. Does the X driver >> opens your device through /dev/wsmouseN or does it uses the mux? > > I think that's weird for this device tough: input is always report from > zero till the maximum values in the driver. No matter how or when you > attach it. > I'm not sure how the X driver opens the device. I've based it off ums(4) > mostly, so it will probably be the same as ums(4) does. > >>> - Documentation is still absent, I'll gladly write it when you guys >>> apporve of the code I wrote. >> >> I'll be happy to do so, could you provide a man page for this driver? > > Attached a new diff with documentation. > >>> What do you guys think? Any comments or suggestions? Any ideas on how to >>> attach to all three uhidevs? >> >> Yes, you have to match the device id. uhidev(4) attaches to the 3 first >> interfaces of your first configuration. And you want a single piece of code >> driving all these interfaces. Do you have some documentation for the device >> you're hacking on? Do you know what you can do with these interfaces? It's >> important to know otherwise you >> might spend as much work refactoring the driver to extend it than you >> spent in the beginning. > > Unfortunately I couldn't find any documentation except for the Linux > device driver implementation (but it supports all kinds of Wacom > tablets, so it isn't exactly readable). > I've written this driver by using the broken device descriptor > and reverse-engeneering with commands such as: > > cat /dev/uhid6 | hexdump -e '9/1 "%02x " "\n"' > > And just moving the pen and seeing which bytes change. > > It seems only the first uhidev device actually reports inputs. The last > uhidev device seems to have the most correct descriptor, but never > reports any data. My guess is that it's there for Windows users without > the driver: they can recognize the tablet as a reported generic mouse. > > As far as I can understand the Linux driver, they only use the data from > the reportid that my driver uses. For completeness I've attached lsusb > output from a Linux computer as well. > > Frank >
byebye SIGNING_PARAMETERS
seemed unclear. There's no longer any benefit to pkg_create signing packages, since signing packages is going to be signify -zS -s /etc/signify/whateverr-pkg.sec -m pkg.tgz -x signed/pkg.tgz (e.g., you have to produce the full archive first THEN you can sign it) I'll have a version of pkg_sign to handle the small details (parallelism) out shortly.
Re: acme-client.1: Use indented display for source code
On Mon, Sep 05, 2016 at 06:02:51PM -0400, Michael Reed wrote: > I find that keeping prose at a different indentation level than source > code makes the man page easier to read. Besides, it's already done > in most other man pages. > fixed, thanks. jmc > > > Index: acme-client.1 > === > RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v > retrieving revision 1.7 > diff -u -p -r1.7 acme-client.1 > --- acme-client.1 1 Sep 2016 13:42:45 - 1.7 > +++ acme-client.1 5 Sep 2016 21:59:35 - > @@ -222,11 +222,11 @@ The default challenge directory > can be served by > .Xr httpd 8 > with this location block: > -.Bd -literal > - location "/.well-known/acme-challenge/*" { > - root "/acme" > - root strip 2 > - } > +.Bd -literal -offset indent > +location "/.well-known/acme-challenge/*" { > + root "/acme" > + root strip 2 > +} > .Ed > .Pp > This way, the files placed in > @@ -261,14 +261,14 @@ web server has already been configured t > as in the > .Sx Challenges > section: > -.Bd -literal > +.Bd -literal -offset indent > # acme-client -vNn foo.com www.foo.com smtp.foo.com > .Ed > .Pp > A daily > .Xr cron 8 > job can renew the certificates: > -.Bd -literal > +.Bd -literal -offset indent > #! /bin/sh > > acme-client foo.com www.foo.com smtp.foo.com >
Re: Make rc scripts use [[ instead of [
On Mon, Sep 05, 2016 at 04:39:49PM -0400, Anthony Coulter wrote: > > While I vaguely remember reading long ago that `echo' and `[' were > shell builtins, I'm much more strongly aware that /bin/echo and /bin/[ > are also available as separate executables. Their man pages don't > mention that they have builtin ksh implementations, and I'm very aware > that `[[' parses arguments differently from `[', e.g. > hi. any commands which have shell builtin equivalents do make note of this, in the STANDARDS section of the relevant man page. jmc