Re: new getformat() for jot(1)

[Theo Buehler]

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)

[Martin Natano]

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

[Adam Wolk]

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

[Eric Faurot]

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

[David Coppa]

Il 6 settembre 2016 14:56:32 CEST, Filippo Valsorda  ha 
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()

[Adam Wolk]

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)

[lists]

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)

[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.

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)

[Antoine Jacoutot]

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)

[lists]

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)

[Anthony Coulter]

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)

[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

-- 
-=[rpe]=-

Re: Improve error message in rcctl(8)

[lists]

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)

[Jiri B]

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)

[Anthony Coulter]

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)

[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...

-- 
Antoine

Re: Improve error message in rcctl(8)

[ludovic coues]

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

[Frank Groeneveld]

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)

[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

Re: subr_tree for netinet/ip_ipsp.[ch]

[Mike Belopuhov]

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()

[Mark Lumsden]

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)

[Anthony Coulter]

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

[Otto Moerbeek]

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

[Martin Pieuchot]

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

[Stuart Henderson]

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

[Filippo Valsorda]

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

[Antoine Jacoutot]

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

[Ulf Brosziewski]

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

[Marc Espie]

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

[Jason McIntyre]

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 [

[Jason McIntyre]

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