Re: time(1): use monotonic clock for computing elapsed time

[David Gwynne]

> On 13 Jul 2017, at 11:16 am, Scott Cheloha  wrote:
> 
> Hi,
> 
> The "real" elapsed time for time(1) and the ksh/csh time builtins is
> currently computed with gettimeofday(2), so it's subject to changes
> by adjtime(2) and, if you're really unlucky, clock_settime(2) or
> settimeofday(2).  In pathological cases you can get negative values
> in the output.
> 
> This seems wrong to me.  I personally use these tools like a stopwatch,
> and I was surprised to see that the elapsed difference wasn't (more)
> immune to changes to the system clock.
> 
> The attached patches change the "real" listing for time(1), ksh's time
> builtin, and csh's time builtin to use a monotonic clock, which I think
> more closely matches what the typical user and programmer expects.  This
> interpretation is, near as I can tell, also compatible with the POSIX.1
> 2008 description of the time(1) utility.  In particular, the use of
> "elapsed," implying a scalar value, makes me think that this is the
> intended behavior. [1]
> 
> NetBSD did this in 2011 without much fanfare, though for some reason they
> did it for time(1) and csh's builtin but not for ksh's builtin. [2]
> 
> I've tested pathological cases in each of the three and these patches
> correct the result in said cases without (perceptibly) changing the
> result in the typical case.
> 
> Thoughts?  Feedback?

this makes sense to me, id like to see it go in.

> 
> --
> Scott Cheloha
> 
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/time.html
> [2] http://gnats.netbsd.org/45592
> 
> Index: bin/csh/csh.h
> ===
> RCS file: /cvs/src/bin/csh/csh.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 csh.h
> --- bin/csh/csh.h 26 Dec 2015 13:48:38 -  1.28
> +++ bin/csh/csh.h 12 Jul 2017 04:15:04 -
> @@ -122,7 +122,7 @@ char   *seterr;   /* Error message from 
> #include 
> #include 
> 
> -struct timeval time0;/* Time at which the shell started */
> +struct timespec time0;   /* Time at which the shell started */
> struct rusage ru0;
> 
> /*
> Index: bin/csh/extern.h
> ===
> RCS file: /cvs/src/bin/csh/extern.h,v
> retrieving revision 1.25
> diff -u -p -r1.25 extern.h
> --- bin/csh/extern.h  26 Dec 2015 13:48:38 -  1.25
> +++ bin/csh/extern.h  12 Jul 2017 04:15:04 -
> @@ -272,7 +272,7 @@ void   plist(struct varent *);
> void  donice(Char **, struct command *);
> void  dotime(Char **, struct command *);
> void  prusage(struct rusage *, struct rusage *,
> - struct timeval *, struct timeval *);
> + struct timespec *, struct timespec *);
> void  ruadd(struct rusage *, struct rusage *);
> void  settimes(void);
> void  pcsecs(long);
> Index: bin/csh/proc.c
> ===
> RCS file: /cvs/src/bin/csh/proc.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 proc.c
> --- bin/csh/proc.c26 Dec 2015 13:48:38 -  1.30
> +++ bin/csh/proc.c12 Jul 2017 04:15:04 -
> @@ -108,7 +108,7 @@ found:
> }
> else {
>   if (pp->p_flags & (PTIME | PPTIME) || adrof(STRtime))
> - (void) gettimeofday(>p_etime, NULL);
> + (void) clock_gettime(CLOCK_MONOTONIC, >p_etime);
> 
>   pp->p_rusage = ru;
>   if (WIFSIGNALED(w)) {
> @@ -494,7 +494,7 @@ palloc(int pid, struct command *t)
> }
> pp->p_next = proclist.p_next;
> proclist.p_next = pp;
> -(void) gettimeofday(>p_btime, NULL);
> +(void) clock_gettime(CLOCK_MONOTONIC, >p_btime);
> }
> 
> static void
> @@ -799,8 +799,8 @@ prcomd:
> static void
> ptprint(struct process *tp)
> {
> -struct timeval tetime, diff;
> -static struct timeval ztime;
> +struct timespec tetime, diff;
> +static struct timespec ztime;
> struct rusage ru;
> static struct rusage zru;
> struct process *pp = tp;
> @@ -809,8 +809,8 @@ ptprint(struct process *tp)
> tetime = ztime;
> do {
>   ruadd(, >p_rusage);
> - timersub(>p_etime, >p_btime, );
> - if (timercmp(, , >))
> + timespecsub(>p_etime, >p_btime, );
> + if (timespeccmp(, , >))
>   tetime = diff;
> } while ((pp = pp->p_friends) != tp);
> prusage(, , , );
> Index: bin/csh/proc.h
> ===
> RCS file: /cvs/src/bin/csh/proc.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 proc.h
> --- bin/csh/proc.h2 Jun 2003 23:32:07 -   1.3
> +++ bin/csh/proc.h12 Jul 2017 04:15:04 -
> @@ -50,8 +50,8 @@ struct process {
> pid_t   p_pid;
> pid_t   p_jobid;  /* pid of job leader */
> /* if a job is stopped/background p_jobid gives its pgrp */
> -struct timeval p_btime;  /* begin time */
> -struct timeval p_etime;  /* end time */
> +struct timespec p_btime; /* begin time */
> +struct timespec p_etime; 

Re: time(1): use monotonic clock for computing elapsed time

[Scott Cheloha]

Whoops, prior diff for usr.bin/time/time.c has a dumb typo,
corrected diff attached.

--
Scott Cheloha

Index: usr.bin/time/time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.21
diff -u -p -r1.21 time.c
--- usr.bin/time/time.c 10 Oct 2015 14:49:23 -  1.21
+++ usr.bin/time/time.c 13 Jul 2017 01:29:00 -
@@ -52,7 +52,7 @@ main(int argc, char *argv[])
 {
pid_t pid;
int ch, status;
-   struct timeval before, after;
+   struct timespec before, after, during;
struct rusage ru;
int exitonsig = 0;
 
@@ -79,7 +79,7 @@ main(int argc, char *argv[])
if (argc < 1)
usage();
 
-   gettimeofday(, (struct timezone *)NULL);
+   clock_gettime(CLOCK_MONOTONIC, );
switch(pid = vfork()) {
case -1:/* error */
perror("time");
@@ -97,24 +97,23 @@ main(int argc, char *argv[])
(void)signal(SIGQUIT, SIG_IGN);
while (wait3(, 0, ) != pid)
;
-   gettimeofday(, (struct timezone *)NULL);
+   clock_gettime(CLOCK_MONOTONIC, );
if (WIFSIGNALED(status))
exitonsig = WTERMSIG(status);
if (!WIFEXITED(status))
fprintf(stderr, "Command terminated abnormally.\n");
-   timersub(, , );
+   timespecsub(, , );
 
if (portableflag) {
fprintf(stderr, "real %9lld.%02ld\n",
-   (long long)after.tv_sec, after.tv_usec/1);
+   (long long)during.tv_sec, during.tv_nsec/1000);
fprintf(stderr, "user %9lld.%02ld\n",
(long long)ru.ru_utime.tv_sec, 
ru.ru_utime.tv_usec/1);
fprintf(stderr, "sys  %9lld.%02ld\n",
(long long)ru.ru_stime.tv_sec, 
ru.ru_stime.tv_usec/1);
} else {
-
fprintf(stderr, "%9lld.%02ld real ",
-   (long long)after.tv_sec, after.tv_usec/1);
+   (long long)during.tv_sec, during.tv_nsec/1000);
fprintf(stderr, "%9lld.%02ld user ",
(long long)ru.ru_utime.tv_sec, 
ru.ru_utime.tv_usec/1);
fprintf(stderr, "%9lld.%02ld sys\n",

time(1): use monotonic clock for computing elapsed time

[Scott Cheloha]

Hi,

The "real" elapsed time for time(1) and the ksh/csh time builtins is
currently computed with gettimeofday(2), so it's subject to changes
by adjtime(2) and, if you're really unlucky, clock_settime(2) or
settimeofday(2).  In pathological cases you can get negative values
in the output.

This seems wrong to me.  I personally use these tools like a stopwatch,
and I was surprised to see that the elapsed difference wasn't (more)
immune to changes to the system clock.

The attached patches change the "real" listing for time(1), ksh's time
builtin, and csh's time builtin to use a monotonic clock, which I think
more closely matches what the typical user and programmer expects.  This
interpretation is, near as I can tell, also compatible with the POSIX.1
2008 description of the time(1) utility.  In particular, the use of
"elapsed," implying a scalar value, makes me think that this is the
intended behavior. [1]

NetBSD did this in 2011 without much fanfare, though for some reason they
did it for time(1) and csh's builtin but not for ksh's builtin. [2]

I've tested pathological cases in each of the three and these patches
correct the result in said cases without (perceptibly) changing the
result in the typical case.

Thoughts?  Feedback?

--
Scott Cheloha

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/time.html
[2] http://gnats.netbsd.org/45592

Index: bin/csh/csh.h
===
RCS file: /cvs/src/bin/csh/csh.h,v
retrieving revision 1.28
diff -u -p -r1.28 csh.h
--- bin/csh/csh.h   26 Dec 2015 13:48:38 -  1.28
+++ bin/csh/csh.h   12 Jul 2017 04:15:04 -
@@ -122,7 +122,7 @@ char   *seterr; /* Error message from 
 #include 
 #include 
 
-struct timeval time0;  /* Time at which the shell started */
+struct timespec time0; /* Time at which the shell started */
 struct rusage ru0;
 
 /*
Index: bin/csh/extern.h
===
RCS file: /cvs/src/bin/csh/extern.h,v
retrieving revision 1.25
diff -u -p -r1.25 extern.h
--- bin/csh/extern.h26 Dec 2015 13:48:38 -  1.25
+++ bin/csh/extern.h12 Jul 2017 04:15:04 -
@@ -272,7 +272,7 @@ void plist(struct varent *);
 void   donice(Char **, struct command *);
 void   dotime(Char **, struct command *);
 void   prusage(struct rusage *, struct rusage *,
-   struct timeval *, struct timeval *);
+   struct timespec *, struct timespec *);
 void   ruadd(struct rusage *, struct rusage *);
 void   settimes(void);
 void   pcsecs(long);
Index: bin/csh/proc.c
===
RCS file: /cvs/src/bin/csh/proc.c,v
retrieving revision 1.30
diff -u -p -r1.30 proc.c
--- bin/csh/proc.c  26 Dec 2015 13:48:38 -  1.30
+++ bin/csh/proc.c  12 Jul 2017 04:15:04 -
@@ -108,7 +108,7 @@ found:
 }
 else {
if (pp->p_flags & (PTIME | PPTIME) || adrof(STRtime))
-   (void) gettimeofday(>p_etime, NULL);
+   (void) clock_gettime(CLOCK_MONOTONIC, >p_etime);
 
pp->p_rusage = ru;
if (WIFSIGNALED(w)) {
@@ -494,7 +494,7 @@ palloc(int pid, struct command *t)
 }
 pp->p_next = proclist.p_next;
 proclist.p_next = pp;
-(void) gettimeofday(>p_btime, NULL);
+(void) clock_gettime(CLOCK_MONOTONIC, >p_btime);
 }
 
 static void
@@ -799,8 +799,8 @@ prcomd:
 static void
 ptprint(struct process *tp)
 {
-struct timeval tetime, diff;
-static struct timeval ztime;
+struct timespec tetime, diff;
+static struct timespec ztime;
 struct rusage ru;
 static struct rusage zru;
 struct process *pp = tp;
@@ -809,8 +809,8 @@ ptprint(struct process *tp)
 tetime = ztime;
 do {
ruadd(, >p_rusage);
-   timersub(>p_etime, >p_btime, );
-   if (timercmp(, , >))
+   timespecsub(>p_etime, >p_btime, );
+   if (timespeccmp(, , >))
tetime = diff;
 } while ((pp = pp->p_friends) != tp);
 prusage(, , , );
Index: bin/csh/proc.h
===
RCS file: /cvs/src/bin/csh/proc.h,v
retrieving revision 1.3
diff -u -p -r1.3 proc.h
--- bin/csh/proc.h  2 Jun 2003 23:32:07 -   1.3
+++ bin/csh/proc.h  12 Jul 2017 04:15:04 -
@@ -50,8 +50,8 @@ struct process {
 pid_t   p_pid;
 pid_t   p_jobid;   /* pid of job leader */
 /* if a job is stopped/background p_jobid gives its pgrp */
-struct timeval p_btime;/* begin time */
-struct timeval p_etime;/* end time */
+struct timespec p_btime;   /* begin time */
+struct timespec p_etime;   /* end time */
 struct rusage p_rusage;
 Char   *p_command; /* first PMAXLEN chars of command */
 };
Index: bin/csh/time.c
===
RCS file: /cvs/src/bin/csh/time.c,v
retrieving revision 1.14
diff -u -p -r1.14 time.c
--- 

Re: ypldap debugging tweak

[Jeremie Courreges-Anglas]

Kurt Mosiejczuk  writes:

> While trying out ypldap (using ypldap -dv) I got a bit confused at one point
> because I just saw "flattening trees" and nothing else.  My LDAP source does
> have a *lot* of users, so thought maybe it was crunching for a bit.  Nope.
> My config was wrong and ypldap got nothing.
>
> This diff just adds a couple extra debugging lines having ypldap announce
> when it it done pushing users and done pushing groups. Just to make it 
> obvious when it's done.  It calls out everything it's doing until then,
> so I think this makes sense.

Makes sense to me too.  Committed.

> --Kurt
>
> Index: entries.c
> ===
> RCS file: /cvs/src/usr.sbin/ypldap/entries.c,v
> retrieving revision 1.3
> diff -u -p -u -p -r1.3 entries.c
> --- entries.c 16 Jan 2015 06:40:22 -  1.3
> +++ entries.c 11 Jul 2017 21:23:47 -
> @@ -90,6 +90,7 @@ flatten_entries(struct env *env)
>   free(tmp);
>   }
>   env->sc_user_lines = linep;
> + log_debug("done pushing users");
>  
>   wrlen = env->sc_group_line_len;
>   if ((linep = calloc(1, env->sc_group_line_len + 1)) == NULL) {
> @@ -115,6 +116,7 @@ flatten_entries(struct env *env)
>   wrlen -= len;
>   }
>   env->sc_group_lines = linep;
> + log_debug("done pushing groups");
>  }
>  
>  int
>

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

Re: Remove getopt from vipw

[Jeremie Courreges-Anglas]

Matthew Martin  writes:

> As far as I can tell the only thing gained from using getopt is handling
> vipw --
> as vipw takes no flags or arguments, is not intended for non-interactive
> use, and is not POSIX, I don't see a reason -- should be handled.

I don't see a reason not to handle -- as in other utilities.  Standard
getopt(3) handling in all programs give a consistent user experience.
I don't think this code costs much.

> If
> anyone prefers proper handling of -- perhaps
>   if (!( argc == 1 || (argc == 2 && strcmp(argv[1], "--") == 0)))
>
> Also kill a needless include.

I have committed this hunk, thanks.

> - Matthew Martin
>
> diff --git vipw.c vipw.c
> index e9595b02198..88a741f1c15 100644
> --- vipw.c
> +++ vipw.c
> @@ -37,7 +37,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
> @@ -49,18 +48,8 @@ main(int argc, char *argv[])
>  {
>   int pfd, tfd;
>   struct stat begin, end;
> - int ch;
>  
> - while ((ch = getopt(argc, argv, "")) != -1) {
> - switch (ch) {
> - default:
> - usage();
> - }
> - }
> - argc -= optind;
> - argv += optind;
> -
> - if (argc != 0)
> + if (argc != 1)
>   usage();
>  
>   if (pledge("stdio rpath wpath cpath fattr proc exec", NULL) == -1)
>

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

Re: relayd ipv6 ttl check_icmp / check_tcp

[Jeremie Courreges-Anglas]

Kapetanakis Giannis  writes:

> On 12/07/17 22:00, Jeremie Courreges-Anglas wrote:
>> The tweak I had in mind: consistently use "ttl" for all the
>> get/setsockopt calls.
>>
>> ok?
>
> nice,
> you can also replace sizeof(int) to sizeof(ttl) on the else{} block of
> case AF_INET6

Indeed, thanks.  Patch committed.

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

Re: relayd ipv6 ttl check_icmp / check_tcp

[Kapetanakis Giannis]

On 12/07/17 22:00, Jeremie Courreges-Anglas wrote:

The tweak I had in mind: consistently use "ttl" for all the
get/setsockopt calls.

ok?


nice,
you can also replace sizeof(int) to sizeof(ttl) on the else{} block of 
case AF_INET6



G




Index: check_icmp.c
===
RCS file: /d/cvs/src/usr.sbin/relayd/check_icmp.c,v
retrieving revision 1.46
diff -u -p -p -u -r1.46 check_icmp.c
--- check_icmp.c11 Jul 2017 19:41:30 -  1.46
+++ check_icmp.c12 Jul 2017 18:57:52 -
@@ -220,11 +220,12 @@ send_icmp(int s, short event, void *arg)
sizeof(packet));
}
  
+			ttl = host->conf.ttl;

switch(cie->af) {
case AF_INET:
-   if ((ttl = host->conf.ttl) > 0) {
+   if (ttl > 0) {
if (setsockopt(s, IPPROTO_IP, IP_TTL,
-   >conf.ttl, sizeof(int)) == -1)
+   , sizeof(ttl)) == -1)
log_warn("%s: setsockopt",
__func__);
} else {
@@ -243,10 +244,10 @@ send_icmp(int s, short event, void *arg)
}
break;
case AF_INET6:
-   if ((ttl = host->conf.ttl) > 0) {
+   if (ttl > 0) {
if (setsockopt(s, IPPROTO_IPV6,
-   IPV6_UNICAST_HOPS, >conf.ttl,
-   sizeof(int)) == -1)
+   IPV6_UNICAST_HOPS, ,
+   sizeof(ttl)) == -1)
log_warn("%s: setsockopt",
__func__);
} else {

Re: relayd ipv6 ttl check_icmp / check_tcp

[Sebastian Benoit]

Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2017.07.12 21:00:44 +0200:
> Jeremie Courreges-Anglas  writes:
> 
> > Kapetanakis Giannis  writes:
> >
> >> On 10/07/17 17:22, Jeremie Courreges-Anglas wrote:
> >>> Using -1 for IPV6_UNICAST_HOPS is correct.
> >>> 
> >>> Note that you can also use -1 for IP_TTL on OpenBSD, sadly some systems
> >>> out there don't support it.
> >>> 
>  comments?
> >>> 
> >>> ok jca@ with the nits below.
> >>> 
> >>> It would be nice to factor this out in a helper function and use it
> >>> elsewhere in relayd.
> >>
> >> Thanks for the comments.
> >>
> >> My guess is that the helper function should go outside of relayd so it can 
> >> be used by others as well?
> >> I leave that to a more competent programmer.
> >>
> >> Would you like me to set -1 to IP_TTL as well and drop the call to 
> >> getsockopt(2)?
> >
> > I'm not sure about this.  Maybe it should be discussed separately?
> >
> >> updated diff bellow (in case not) with jca@ recommendations.
> >
> > I have tweaks, but this already looks fine to me.  ok jca@
> 
> The tweak I had in mind: consistently use "ttl" for all the
> get/setsockopt calls.
> 
> ok?

yes please

> 
> 
> Index: check_icmp.c
> ===
> RCS file: /d/cvs/src/usr.sbin/relayd/check_icmp.c,v
> retrieving revision 1.46
> diff -u -p -p -u -r1.46 check_icmp.c
> --- check_icmp.c  11 Jul 2017 19:41:30 -  1.46
> +++ check_icmp.c  12 Jul 2017 18:57:52 -
> @@ -220,11 +220,12 @@ send_icmp(int s, short event, void *arg)
>   sizeof(packet));
>   }
>  
> + ttl = host->conf.ttl;
>   switch(cie->af) {
>   case AF_INET:
> - if ((ttl = host->conf.ttl) > 0) {
> + if (ttl > 0) {
>   if (setsockopt(s, IPPROTO_IP, IP_TTL,
> - >conf.ttl, sizeof(int)) == -1)
> + , sizeof(ttl)) == -1)
>   log_warn("%s: setsockopt",
>   __func__);
>   } else {
> @@ -243,10 +244,10 @@ send_icmp(int s, short event, void *arg)
>   }
>   break;
>   case AF_INET6:
> - if ((ttl = host->conf.ttl) > 0) {
> + if (ttl > 0) {
>   if (setsockopt(s, IPPROTO_IPV6,
> - IPV6_UNICAST_HOPS, >conf.ttl,
> - sizeof(int)) == -1)
> + IPV6_UNICAST_HOPS, ,
> + sizeof(ttl)) == -1)
>   log_warn("%s: setsockopt",
>   __func__);
>   } else {
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

Re: urndis issues

[Jonathan Armani]

Hi,

Thanks I was cooking the same diff.

Ok armani@

Le mar. 11 juil. 2017 à 12:32, Mike Belopuhov  a écrit :

> On Sun, Jul 09, 2017 at 09:57 +0300, Artturi Alm wrote:
> > Hi,
> >
> > anyone else having issues w/urndis(android)?
> > victim of circumstances, i have to rely on it at times during the summer.
> > When i plug phone into usb, and enable usb tethering or w/e it is called,
> > i never get ip on first try, i have nothing but "dhcp"
> > in /etc/hostname.urndis0, so i just ^C on the first i"ksh /etc/netstart"
> > and get ip pretty much as expected in seconds on the successive run
> > right after ^C, the first dhclient would end up sleeping if not ^C'ed..
> >
> > this is what i see in dmesg:
> > urndis0 at uhub0 port 1 configuration 1 interface 0 "SAMSUNG
> SAMSUNG_Android" rev 2.00/ff.ff addr 2
> > urndis0: using RNDIS, address 02:56:66:63:30:3c
> > urndis0: ctrl message error: unknown event 0x7
> >
> > no dmesg, as i've ran into this issue on every installation of OpenBSD
> > i have tried w/.
> > unrelated issue is this spam i get which i haven't noticed to affect
> > anything:
> > urndis0: urndis_decap invalid buffer len 1 < minimum header 44
> >
> > for which i ended up w/diff below.
> > -Artturi
> >
>
> What happens if you apply the diff below w/o your modifications?
>
> diff --git sys/dev/usb/if_urndis.c sys/dev/usb/if_urndis.c
> index 4af6b55cf05..bdca361713d 100644
> --- sys/dev/usb/if_urndis.c
> +++ sys/dev/usb/if_urndis.c
> @@ -88,10 +88,12 @@ u_int32_t urndis_ctrl_handle_init(struct urndis_softc
> *,
>  const struct rndis_comp_hdr *);
>  u_int32_t urndis_ctrl_handle_query(struct urndis_softc *,
>  const struct rndis_comp_hdr *, void **, size_t *);
>  u_int32_t urndis_ctrl_handle_reset(struct urndis_softc *,
>  const struct rndis_comp_hdr *);
> +u_int32_t urndis_ctrl_handle_status(struct urndis_softc *,
> +const struct rndis_comp_hdr *);
>
>  u_int32_t urndis_ctrl_init(struct urndis_softc *);
>  u_int32_t urndis_ctrl_halt(struct urndis_softc *);
>  u_int32_t urndis_ctrl_query(struct urndis_softc *, u_int32_t, void *,
> size_t,
>  void **, size_t *);
> @@ -233,10 +235,14 @@ urndis_ctrl_handle(struct urndis_softc *sc, struct
> rndis_comp_hdr *hdr,
> case REMOTE_NDIS_KEEPALIVE_CMPLT:
> case REMOTE_NDIS_SET_CMPLT:
> rval = letoh32(hdr->rm_status);
> break;
>
> +   case REMOTE_NDIS_INDICATE_STATUS_MSG:
> +   rval = urndis_ctrl_handle_status(sc, hdr);
> +   break;
> +
> default:
> printf("%s: ctrl message error: unknown event
> 0x%x\n",
> DEVNAME(sc), letoh32(hdr->rm_type));
> rval = RNDIS_STATUS_FAILURE;
> }
> @@ -400,10 +406,48 @@ urndis_ctrl_handle_reset(struct urndis_softc *sc,
>
> return rval;
>  }
>
>  u_int32_t
> +urndis_ctrl_handle_status(struct urndis_softc *sc,
> +const struct rndis_comp_hdr *hdr)
> +{
> +   const struct rndis_status_msg   *msg;
> +   u_int32_trval;
> +
> +   msg = (struct rndis_status_msg *)hdr;
> +
> +   rval = letoh32(msg->rm_status);
> +
> +   DPRINTF(("%s: urndis_ctrl_handle_status: len %u status 0x%x "
> +   "stbuflen %u\n",
> +   DEVNAME(sc),
> +   letoh32(msg->rm_len),
> +   rval,
> +   letoh32(msg->rm_stbuflen)));
> +
> +   switch (rval) {
> +   case RNDIS_STATUS_MEDIA_CONNECT:
> +   printf("%s: link up\n", DEVNAME(sc));
> +   break;
> +
> +   case RNDIS_STATUS_MEDIA_DISCONNECT:
> +   printf("%s: link down\n", DEVNAME(sc));
> +   break;
> +
> +   /* Ignore these */
> +   case RNDIS_STATUS_OFFLOAD_CURRENT_CONFIG:
> +   break;
> +
> +   default:
> +   printf("%s: unknown status 0x%x\n", DEVNAME(sc),
> rval);
> +   }
> +
> +   return RNDIS_STATUS_SUCCESS;
> +}
> +
> +u_int32_t
>  urndis_ctrl_init(struct urndis_softc *sc)
>  {
> struct rndis_init_req   *msg;
> u_int32_trval;
> struct rndis_comp_hdr   *hdr;
>
>

Re: armv7 small XXX fix

[Dale Rahn]

On Wed, Jul 12, 2017 at 11:06:23PM +0300, Artturi Alm wrote:
> On Wed, Jul 12, 2017 at 06:12:34PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 10 Jul 2017 23:18:59 +0300
> > > From: Artturi Alm 
> > > 
> > > Hi,
> > > 
> > > this does clutter my diffs, and the XXX comment is correct,
> > 
> > It probably isn't. None of the other architectures have those macros
> > in their .
> > 
> 
> Ok, didn't consider that, you're right and the diff withdrawn, but anyway,
> what i was imprecisely after was that i'd prefer
> having something like RODATA(name) found from sparc64/include/asm.h,
> instead of the useless _C_LABEL() to use in armv7 md .S assembly, and
> just because of the weird name i didn't find intuitive enough, i didn't
> even suggest the arm64 EENTRY()..
> Don't get me wrong, i'm more than happy to drop all the labeling
> macros out of my diffs, and choose my self what is global and what is not,
> while it's against the minimalism i'm now aiming at to even get the diffs
> read when freetime does meet:)

_C_LABEL() is a holdover from the pre-ELF days. At one time (in NetBSD)
this code still compiled using an a.out toolchain. In that era, it was
defined to be _C_LABEL(x) _/**/x (this was also pre-ansi)
(or _ ## x post ansi). On ELF _C_LABEL evaluates to just the symbol. It
cannot have any association to either text or data because the proper use
of it can refer to either text or data eg: bl   _C_LABEL(c_func).

Basically it is a relic from the past, it might make sense to just remove
all references to _C_LABEL() at this point and just the symbol itself.

Dale Rahn   dr...@dalerahn.com

Re: armv7 small XXX fix

[Artturi Alm]

On Wed, Jul 12, 2017 at 06:12:34PM +0200, Mark Kettenis wrote:
> > Date: Mon, 10 Jul 2017 23:18:59 +0300
> > From: Artturi Alm 
> > 
> > Hi,
> > 
> > this does clutter my diffs, and the XXX comment is correct,
> 
> It probably isn't. None of the other architectures have those macros
> in their .
> 

Ok, didn't consider that, you're right and the diff withdrawn, but anyway,
what i was imprecisely after was that i'd prefer
having something like RODATA(name) found from sparc64/include/asm.h,
instead of the useless _C_LABEL() to use in armv7 md .S assembly, and
just because of the weird name i didn't find intuitive enough, i didn't
even suggest the arm64 EENTRY()..
Don't get me wrong, i'm more than happy to drop all the labeling
macros out of my diffs, and choose my self what is global and what is not,
while it's against the minimalism i'm now aiming at to even get the diffs
read when freetime does meet:)

-Artturi

> > currently used _C_LABEL() is nothing, and i find it's usage
> > directly rather pointless/weird, this does atleast make x .globl,
> > so there is benefit to the added characters in written code
> > be it _C_LABEL() or C_OBJECT() instead of just the label, that
> > i personally prefer, but given wide use of _C_LABEL() i thought
> > this might want fixing so i wont get told to use _C_LABEL() or
> > anything for my diffs...
> > 
> > here is define for _C_LABEL:
> > #define _C_LABEL(x) x
> > 
> > _C_LABEL() is used over 60 times in sys/arch/arm/arm/*.S atm.
> > (C_OBJECT() only 4 times under the #define.
> > if this goes in, there miight be easy cleanup to be done for
> > anyone who does care. there might be even unused variables around:)
> > 
> > -Artturi
> > 
> > 
> > diff --git a/sys/arch/arm/arm/cpufunc_asm_armv7.S 
> > b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> > index 05679df15fa..7b71652c4dc 100644
> > --- a/sys/arch/arm/arm/cpufunc_asm_armv7.S
> > +++ b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> > @@ -234,10 +234,6 @@ ENTRY(armv7_context_switch)
> > isb sy
> > mov pc, lr
> >  
> > -/* XXX The following macros should probably be moved to asm.h */
> > -#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> > -#define C_OBJECT(x)_DATA_OBJECT(_C_LABEL(x))
> > -
> > .align 2
> >  C_OBJECT(armv7_dcache_sets_max)
> > .word   0
> > diff --git a/sys/arch/arm/include/asm.h b/sys/arch/arm/include/asm.h
> > index e1e5bbc4dd2..7f9af03c7a5 100644
> > --- a/sys/arch/arm/include/asm.h
> > +++ b/sys/arch/arm/include/asm.h
> > @@ -64,6 +64,9 @@
> >  #define _ENTRY(x) \
> > .text; _ALIGN_TEXT; .globl x; .type x,_ASM_TYPE_FUNCTION; x:
> >  
> > +#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> > +#define C_OBJECT(x)_DATA_OBJECT(_C_LABEL(x))
> > +
> >  #if defined(PROF) || defined(GPROF)
> >  # define _PROF_PROLOGUE\
> > mov ip, lr; bl __mcount
> > 
> > 

Re: urndis issues

[Artturi Alm]

On Tue, Jul 11, 2017 at 12:32:30PM +0200, Mike Belopuhov wrote:
> On Sun, Jul 09, 2017 at 09:57 +0300, Artturi Alm wrote:
> > Hi,
> > 
> > anyone else having issues w/urndis(android)?
> > victim of circumstances, i have to rely on it at times during the summer.
> > When i plug phone into usb, and enable usb tethering or w/e it is called,
> > i never get ip on first try, i have nothing but "dhcp"
> > in /etc/hostname.urndis0, so i just ^C on the first i"ksh /etc/netstart"
> > and get ip pretty much as expected in seconds on the successive run
> > right after ^C, the first dhclient would end up sleeping if not ^C'ed..
> > 
> > this is what i see in dmesg:
> > urndis0 at uhub0 port 1 configuration 1 interface 0 "SAMSUNG 
> > SAMSUNG_Android" rev 2.00/ff.ff addr 2
> > urndis0: using RNDIS, address 02:56:66:63:30:3c
> > urndis0: ctrl message error: unknown event 0x7
> > 
> > no dmesg, as i've ran into this issue on every installation of OpenBSD
> > i have tried w/.
> > unrelated issue is this spam i get which i haven't noticed to affect
> > anything:
> > urndis0: urndis_decap invalid buffer len 1 < minimum header 44
> > 
> > for which i ended up w/diff below.
> > -Artturi
> > 
> 
> What happens if you apply the diff below w/o your modifications?
> 
> diff --git sys/dev/usb/if_urndis.c sys/dev/usb/if_urndis.c
> index 4af6b55cf05..bdca361713d 100644
> --- sys/dev/usb/if_urndis.c
> +++ sys/dev/usb/if_urndis.c
> @@ -88,10 +88,12 @@ u_int32_t urndis_ctrl_handle_init(struct urndis_softc *,
>  const struct rndis_comp_hdr *);
>  u_int32_t urndis_ctrl_handle_query(struct urndis_softc *,
>  const struct rndis_comp_hdr *, void **, size_t *);
>  u_int32_t urndis_ctrl_handle_reset(struct urndis_softc *,
>  const struct rndis_comp_hdr *);
> +u_int32_t urndis_ctrl_handle_status(struct urndis_softc *,
> +const struct rndis_comp_hdr *);
>  
>  u_int32_t urndis_ctrl_init(struct urndis_softc *);
>  u_int32_t urndis_ctrl_halt(struct urndis_softc *);
>  u_int32_t urndis_ctrl_query(struct urndis_softc *, u_int32_t, void *, size_t,
>  void **, size_t *);
> @@ -233,10 +235,14 @@ urndis_ctrl_handle(struct urndis_softc *sc, struct 
> rndis_comp_hdr *hdr,
>   case REMOTE_NDIS_KEEPALIVE_CMPLT:
>   case REMOTE_NDIS_SET_CMPLT:
>   rval = letoh32(hdr->rm_status);
>   break;
>  
> + case REMOTE_NDIS_INDICATE_STATUS_MSG:
> + rval = urndis_ctrl_handle_status(sc, hdr);
> + break;
> +
>   default:
>   printf("%s: ctrl message error: unknown event 0x%x\n",
>   DEVNAME(sc), letoh32(hdr->rm_type));
>   rval = RNDIS_STATUS_FAILURE;
>   }
> @@ -400,10 +406,48 @@ urndis_ctrl_handle_reset(struct urndis_softc *sc,
>  
>   return rval;
>  }
>  
>  u_int32_t
> +urndis_ctrl_handle_status(struct urndis_softc *sc,
> +const struct rndis_comp_hdr *hdr)
> +{
> + const struct rndis_status_msg   *msg;
> + u_int32_trval;
> +
> + msg = (struct rndis_status_msg *)hdr;
> +
> + rval = letoh32(msg->rm_status);
> +
> + DPRINTF(("%s: urndis_ctrl_handle_status: len %u status 0x%x "
> + "stbuflen %u\n",
> + DEVNAME(sc),
> + letoh32(msg->rm_len),
> + rval,
> + letoh32(msg->rm_stbuflen)));
> +
> + switch (rval) {
> + case RNDIS_STATUS_MEDIA_CONNECT:
> + printf("%s: link up\n", DEVNAME(sc));
> + break;
> +
> + case RNDIS_STATUS_MEDIA_DISCONNECT:
> + printf("%s: link down\n", DEVNAME(sc));
> + break;
> +
> + /* Ignore these */
> + case RNDIS_STATUS_OFFLOAD_CURRENT_CONFIG:
> + break;
> +
> + default:
> + printf("%s: unknown status 0x%x\n", DEVNAME(sc), rval);
> + }
> +
> + return RNDIS_STATUS_SUCCESS;
> +}
> +
> +u_int32_t
>  urndis_ctrl_init(struct urndis_softc *sc)
>  {
>   struct rndis_init_req   *msg;
>   u_int32_trval;
>   struct rndis_comp_hdr   *hdr;

Hi,

your diff solved the problem w/ dhclient ending up sleeping upon
"ctrl message error: unknown event 0x7", and instead does print:

urndis0 at uhub1 port 2 configuration 1 interface 0 "SAMSUNG SAMSUNG_Android" 
rev 2.00/ff.ff addr 3
urndis0: using RNDIS, address 02:56:66:63:30:3c
urndis0: link up

the ": link up" does come after i run netstart urndis0, so it
seems to work as i guess you intended, thanks:)

i just connected w/the diff to reply, and have not yet seen
those _decap invalid buffer len 1 msgs, but i haven't caused
any pressure on the connection yet either so i might just not have
triggered those, and so don't know if your diff does help w/that
spam yet.

-Artturi

Re: relayd ipv6 ttl check_icmp / check_tcp

[Jeremie Courreges-Anglas]

Jeremie Courreges-Anglas  writes:

> Kapetanakis Giannis  writes:
>
>> On 10/07/17 17:22, Jeremie Courreges-Anglas wrote:
>>> Using -1 for IPV6_UNICAST_HOPS is correct.
>>> 
>>> Note that you can also use -1 for IP_TTL on OpenBSD, sadly some systems
>>> out there don't support it.
>>> 
 comments?
>>> 
>>> ok jca@ with the nits below.
>>> 
>>> It would be nice to factor this out in a helper function and use it
>>> elsewhere in relayd.
>>
>> Thanks for the comments.
>>
>> My guess is that the helper function should go outside of relayd so it can 
>> be used by others as well?
>> I leave that to a more competent programmer.
>>
>> Would you like me to set -1 to IP_TTL as well and drop the call to 
>> getsockopt(2)?
>
> I'm not sure about this.  Maybe it should be discussed separately?
>
>> updated diff bellow (in case not) with jca@ recommendations.
>
> I have tweaks, but this already looks fine to me.  ok jca@

The tweak I had in mind: consistently use "ttl" for all the
get/setsockopt calls.

ok?


Index: check_icmp.c
===
RCS file: /d/cvs/src/usr.sbin/relayd/check_icmp.c,v
retrieving revision 1.46
diff -u -p -p -u -r1.46 check_icmp.c
--- check_icmp.c11 Jul 2017 19:41:30 -  1.46
+++ check_icmp.c12 Jul 2017 18:57:52 -
@@ -220,11 +220,12 @@ send_icmp(int s, short event, void *arg)
sizeof(packet));
}
 
+   ttl = host->conf.ttl;
switch(cie->af) {
case AF_INET:
-   if ((ttl = host->conf.ttl) > 0) {
+   if (ttl > 0) {
if (setsockopt(s, IPPROTO_IP, IP_TTL,
-   >conf.ttl, sizeof(int)) == -1)
+   , sizeof(ttl)) == -1)
log_warn("%s: setsockopt",
__func__);
} else {
@@ -243,10 +244,10 @@ send_icmp(int s, short event, void *arg)
}
break;
case AF_INET6:
-   if ((ttl = host->conf.ttl) > 0) {
+   if (ttl > 0) {
if (setsockopt(s, IPPROTO_IPV6,
-   IPV6_UNICAST_HOPS, >conf.ttl,
-   sizeof(int)) == -1)
+   IPV6_UNICAST_HOPS, ,
+   sizeof(ttl)) == -1)
log_warn("%s: setsockopt",
__func__);
} else {


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

Re: SSHFP with EDNS0/DNSSEC

[Jeremie Courreges-Anglas]

Eric Faurot  writes:

> On Wed, Jul 12, 2017 at 07:45:36AM +0200, Christian Barthel wrote:
>> Hi, 
>> 
>> earlier this year, jca@ worked on support for DNSSEC and the EDNS0
>> extension [1] and committed this work at [2] (thanks!).  I tried this
>> with SSHFP records to check authenticity of hosts with DNSSEC; but ssh
>> reported that the hostkey fingerprints were insecure.   
>> 
>> I am using this configuration file: 
>> 
>> # cat /etc/resolv.conf
>> nameserver 8.8.8.8
>> options edns0
>> 
>> And ssh reports the following: 
>> 
>> $ ssh -o VerifyHostKeyDNS=yes - doamin_with_sshpf_dnssec
>>   ...
>> debug3: verify_host_key_dns
>> debug1: found 8 insecure fingerprints in DNS
>> debug1: matching host key fingerprint found in DNS
>> The authenticity of host 'xxx ()' can't be established.
>> ECDSA key fingerprint is 
>> Matching host key fingerprint found in DNS.
>> Are you sure you want to continue connecting (yes/no)? 
>>   ...
>> 
>> I tried to find out why and after going through the asr code, I found
>> the following: 
>> 
>> Index: lib/libc/asr/res_send_async.c
>> ===
>> RCS file: /cvs/src/lib/libc/asr/res_send_async.c,v
>> retrieving revision 1.36
>> diff -u -p -r1.36 res_send_async.c
>> --- lib/libc/asr/res_send_async.c15 Mar 2017 15:54:41 -  1.36
>> +++ lib/libc/asr/res_send_async.c11 Jul 2017 20:09:59 -
>> @@ -385,7 +385,7 @@ setup_query(struct asr_query *as, const 
>>  _asr_pack_query(, type, class, dname);
>>  if (as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC))
>>  _asr_pack_edns0(, MAXPACKETSZ,
>> -as->as_ctx->ac_options & RES_USE_DNSSEC);
>> +as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC));
>>  if (p.err) {
>>  DPRINT("error packing query");
>>  errno = EINVAL;
>
> The current code is correct, RES_USE_EDNS0 does not imply RES_USE_DNSSEC.
> The real problem is that there is no resolv.conf option for RES_USE_DNSSEC.

RES_USE_DNSSEC is set by applications that *do* care about the AD bit.
There's no point in setting globally RES_USE_DNSSEC, so I don't think we
need a resolv.conf option.

> It can only be set in user code by tweaking _res.options.

ssh(1) uses getrrsetbyname(3) to look at SSHFP records, so the fix is to
teach getrrsetbyname to request DNSSEC processing.  Eric and I have
already discussed this and need to settle on the implementation.

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

Re: Use modesetting driver on i965 and up

[Mark Kettenis]

> From: "Ted Unangst" 
> Date: Mon, 10 Jul 2017 17:36:00 -0400
> 
> Mark Kettenis wrote:
> > This diff has been in snaps for a while now.  This matches what
> > several major Linux distros do.
> > 
> > ok?
> 
> for those of us who haven't memorized hex PCI IDs, can we add a comment
> indicating the codename of the cutoff? broadwell or skylake or whatever?
> everything becomes "older" at some point. :)

Luckily the older ones remain "older" as well.

It's hard to use a codename here as there is considerable confusion
among them around the cutoff point.  I'll mention that the cutoff
point is after the third generation Intel graphics hardware.

Re: armv7 small XXX fix

[Mark Kettenis]

> Date: Mon, 10 Jul 2017 23:18:59 +0300
> From: Artturi Alm 
> 
> Hi,
> 
> this does clutter my diffs, and the XXX comment is correct,

It probably isn't. None of the other architectures have those macros
in their .

> currently used _C_LABEL() is nothing, and i find it's usage
> directly rather pointless/weird, this does atleast make x .globl,
> so there is benefit to the added characters in written code
> be it _C_LABEL() or C_OBJECT() instead of just the label, that
> i personally prefer, but given wide use of _C_LABEL() i thought
> this might want fixing so i wont get told to use _C_LABEL() or
> anything for my diffs...
> 
> here is define for _C_LABEL:
> #define _C_LABEL(x)   x
> 
> _C_LABEL() is used over 60 times in sys/arch/arm/arm/*.S atm.
> (C_OBJECT() only 4 times under the #define.
> if this goes in, there miight be easy cleanup to be done for
> anyone who does care. there might be even unused variables around:)
> 
> -Artturi
> 
> 
> diff --git a/sys/arch/arm/arm/cpufunc_asm_armv7.S 
> b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> index 05679df15fa..7b71652c4dc 100644
> --- a/sys/arch/arm/arm/cpufunc_asm_armv7.S
> +++ b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> @@ -234,10 +234,6 @@ ENTRY(armv7_context_switch)
>   isb sy
>   mov pc, lr
>  
> -/* XXX The following macros should probably be moved to asm.h */
> -#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> -#define C_OBJECT(x)  _DATA_OBJECT(_C_LABEL(x))
> -
>   .align 2
>  C_OBJECT(armv7_dcache_sets_max)
>   .word   0
> diff --git a/sys/arch/arm/include/asm.h b/sys/arch/arm/include/asm.h
> index e1e5bbc4dd2..7f9af03c7a5 100644
> --- a/sys/arch/arm/include/asm.h
> +++ b/sys/arch/arm/include/asm.h
> @@ -64,6 +64,9 @@
>  #define _ENTRY(x) \
>   .text; _ALIGN_TEXT; .globl x; .type x,_ASM_TYPE_FUNCTION; x:
>  
> +#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> +#define C_OBJECT(x)  _DATA_OBJECT(_C_LABEL(x))
> +
>  #if defined(PROF) || defined(GPROF)
>  # define _PROF_PROLOGUE  \
>   mov ip, lr; bl __mcount
> 
> 

Re: Potential DoS attack on PF due to infinite loop

[Alexander Bluhm]

On Tue, Jul 11, 2017 at 12:29:33PM -0700, Jingmin Zhou wrote:
> Recently we discovered a potential bug in pf_lb.c.

I have commited a fix.  Thanks for the report and analysis.

bluhm

Re: Potential DoS attack on PF due to infinite loop

[Alexandr Nedvedicky]

On Wed, Jul 12, 2017 at 02:40:58PM +0200, Alexander Bluhm wrote:
> On Tue, Jul 11, 2017 at 12:29:33PM -0700, Jingmin Zhou wrote:
> > The problem is at line 224. When a LB rule is configured to have 65535 as
> > the high port, and uint16 variable tmp reaches it, ++(tmp) will wrap around
> > and hence potentially enter into an infinite loop. Of course, it only
> > happens when high is configured to 65535.
> 
> I think this should detect all overflow problems.
> 
> ok?

looks good to me.

OK sashan
> 
> bluhm
> 
> Index: net/pf_lb.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_lb.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 pf_lb.c
> --- net/pf_lb.c   23 Apr 2017 11:37:11 -  1.60
> +++ net/pf_lb.c   12 Jul 2017 12:11:35 -
> @@ -211,7 +211,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
>   return (0);
>   }
>   } else {
> - u_int16_t tmp;
> + u_int32_t tmp;
>  
>   if (low > high) {
>   tmp = low;
> @@ -221,7 +221,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
>   /* low < high */
>   cut = arc4random_uniform(1 + high - low) + low;
>   /* low <= cut <= high */
> - for (tmp = cut; tmp <= high; ++(tmp)) {
> + for (tmp = cut; tmp <= high && tmp <= 0x; ++tmp) {
>   key.port[sidx] = htons(tmp);
>   if (pf_find_state_all(, dir, NULL) ==
>   NULL && !in_baddynamic(tmp, pd->proto)) {
> @@ -229,7 +229,8 @@ pf_get_sport(struct pf_pdesc *pd, struct
>   return (0);
>   }
>   }
> - for (tmp = cut - 1; tmp >= low; --(tmp)) {
> + tmp = cut;
> + for (tmp -= 1; tmp >= low && tmp <= 0x; --tmp) {
>   key.port[sidx] = htons(tmp);
>   if (pf_find_state_all(, dir, NULL) ==
>   NULL && !in_baddynamic(tmp, pd->proto)) {
> 

Re: Potential DoS attack on PF due to infinite loop

[Alexander Bluhm]

On Tue, Jul 11, 2017 at 12:29:33PM -0700, Jingmin Zhou wrote:
> The problem is at line 224. When a LB rule is configured to have 65535 as
> the high port, and uint16 variable tmp reaches it, ++(tmp) will wrap around
> and hence potentially enter into an infinite loop. Of course, it only
> happens when high is configured to 65535.

I think this should detect all overflow problems.

ok?

bluhm

Index: net/pf_lb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.60
diff -u -p -r1.60 pf_lb.c
--- net/pf_lb.c 23 Apr 2017 11:37:11 -  1.60
+++ net/pf_lb.c 12 Jul 2017 12:11:35 -
@@ -211,7 +211,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
return (0);
}
} else {
-   u_int16_t tmp;
+   u_int32_t tmp;
 
if (low > high) {
tmp = low;
@@ -221,7 +221,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
/* low < high */
cut = arc4random_uniform(1 + high - low) + low;
/* low <= cut <= high */
-   for (tmp = cut; tmp <= high; ++(tmp)) {
+   for (tmp = cut; tmp <= high && tmp <= 0x; ++tmp) {
key.port[sidx] = htons(tmp);
if (pf_find_state_all(, dir, NULL) ==
NULL && !in_baddynamic(tmp, pd->proto)) {
@@ -229,7 +229,8 @@ pf_get_sport(struct pf_pdesc *pd, struct
return (0);
}
}
-   for (tmp = cut - 1; tmp >= low; --(tmp)) {
+   tmp = cut;
+   for (tmp -= 1; tmp >= low && tmp <= 0x; --tmp) {
key.port[sidx] = htons(tmp);
if (pf_find_state_all(, dir, NULL) ==
NULL && !in_baddynamic(tmp, pd->proto)) {

Re: first stab at RFC 7217: random but stable link local addresses

[Florian Obser]

On Wed, Jul 12, 2017 at 01:45:36PM +1000, David Gwynne wrote:
> On Tue, Jul 11, 2017 at 11:48:47AM +, Florian Obser wrote:
> > On Tue, Jul 11, 2017 at 11:08:23AM +0100, Stuart Henderson wrote:
> > > On 2017/07/11 07:45, Florian Obser wrote:
> > > > The way I want to move forward with this is:
> > > > 
> > > > 1) generate a random key at boot if it's not present yet (like we do
> > > > for ssh host keys and ipsec)
> > > > 2) if /etc/netstart brings up an interface set the key there by
> > > > enabling the feature per default like we do with privacy addresses.
> > > > NOTE that this does NOT enable v6.
> > > > If you do not want this you have to put -soiikey into the hostname.if
> > > > file (like -autoconfprivacy)
> > > 
> > > It's slightly different to autoconfprivacy as that is enabled by
> > > default in the kernel for new interfaces, whereas with this it depends
> > > on the key being configured. So there are implications for dynamically
> > > created interfaces (USB, vlan etc) which may not always be handled by
> > > netstart.
> > > 
> > > I'm wondering if we should have a "system soiikey" (e.g. in a sysctl)
> > > that is automatically applied to newly created interfaces? (Does the
> > > key even need to be configurable per-interface or would a per-system
> > > one be enough by itself?)
> > 
> > I was also considering this. A per system key is probably enough.  I
> > went with the per-interface key because that made it easier to disable
> > the feature on an interface. If we go with per-system we need a
> > interface flag to disable the feature (like -autoconfprivacy).
> > 
> > The rfc seems to assume a per system secret.
> > 
> > hmm... are there sysctls that are only readable as root?
> 
> yeah. see below for example code for a net.inet6.ip6.soiikey sysctl.
> it looks big cos it teaches sysctl how to handle binary strings
> represented by hex.

Awesome, thank you very much, I will adapt my diff to this.

I don't think that we need to check securelevel (re: your comment in
the diff).
I don't really believe in securelevel, also it seems that even at 2
you are still allowed to change ip addresses on interfaces. And this
is all this key influences. Additionally I think it is valid to change
the key during runtime.

> 
> are we tied to sha512 for this? siphash makes a nice 8 byte output...

Nope, the rfc intentionally does not specify the hash function.

> 
> > 
> > > 
> > > > 3) if v6 is enabled in hostname.if you get a random but stable link
> > > > local address according to RFC 7217.
> > > > 4) if autoconf6 is enabled on the interface slaacd will generate a
> > > > random but stable global address according to RFC 7217
> > > > 
> > > > Also note that RFC 8064 is a thing and we are kinda supposed to do this.
> > > > Also I think it's a good idea[tm].
> > > > 
> > > > Comments, OKs?
> > > 
> > > A couple of other comments:
> > > 
> > > - system administrators should know in advance of updating that their
> > > IP address may change in case they need to prepare for it (adjust
> > > firewalls/nat-pt/proxies).
> > 
> > sure, /me glances at current.html
> > 
> > > 
> > > - ramdisk upgrades may need to load the key from the installed system
> > > (e.g. in presence of restrictive firewall policies).
> > > 
> > 
> > good point
> > 
> > > - a half-related idea came up while pondering this,
> > > "block in inet6 to autoconfprivacy" (similar UI to urpf-failed)
> > > (I'm meant to be doing something else and likely to forget so I'd
> > > like to at least plant the idea :)
> > 
> > heh, interesting
> > 
> > -- 
> > I'm not entirely sure you are real.
> 
> 
> Index: sbin/sysctl/sysctl.c
> ===
> RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.226
> diff -u -p -r1.226 sysctl.c
> --- sbin/sysctl/sysctl.c  25 Apr 2017 17:33:16 -  1.226
> +++ sbin/sysctl/sysctl.c  12 Jul 2017 03:39:33 -
> @@ -212,7 +212,7 @@ int sysctl_chipset(char *, char **, int 
>  #endif
>  void vfsinit(void);
>  
> -char *equ = "=";
> +const char *equ = "=";
>  
>  int
>  main(int argc, char *argv[])
> @@ -286,6 +286,52 @@ listall(char *prefix, struct list *lp)
>   }
>  }
>  
> +int
> +parse_hex_char(char ch)
> +{
> + if (ch >= '0' && ch <= '9')
> + return (ch - '0');
> + if (ch >= 'a' && ch <= 'f')
> + return (ch - 'a');
> + if (ch >= 'A' && ch <= 'F')
> + return (ch - 'A');
> +
> + return (-1);
> +}
> +
> +ssize_t
> +parse_hex_string(unsigned char *dst, size_t dstlen, const char *src)
> +{
> + ssize_t len = 0;
> + int digit;
> +
> + while (len < dstlen) {
> + if (*src == '\0')
> + return (len);
> +
> + digit = parse_hex_char(*src++);
> + if (digit == -1)
> + return (-1);
> + dst[len] = digit << 4;
> +
> + digit = parse_hex_char(*src++);
> + if (digit == -1)
> +  

Re: SSHFP with EDNS0/DNSSEC

[Eric Faurot]

On Wed, Jul 12, 2017 at 07:45:36AM +0200, Christian Barthel wrote:
> Hi, 
> 
> earlier this year, jca@ worked on support for DNSSEC and the EDNS0
> extension [1] and committed this work at [2] (thanks!).  I tried this
> with SSHFP records to check authenticity of hosts with DNSSEC; but ssh
> reported that the hostkey fingerprints were insecure.   
> 
> I am using this configuration file: 
> 
> # cat /etc/resolv.conf
> nameserver 8.8.8.8
> options edns0
> 
> And ssh reports the following: 
> 
> $ ssh -o VerifyHostKeyDNS=yes - doamin_with_sshpf_dnssec
>   ...
> debug3: verify_host_key_dns
> debug1: found 8 insecure fingerprints in DNS
> debug1: matching host key fingerprint found in DNS
> The authenticity of host 'xxx ()' can't be established.
> ECDSA key fingerprint is 
> Matching host key fingerprint found in DNS.
> Are you sure you want to continue connecting (yes/no)? 
>   ...
> 
> I tried to find out why and after going through the asr code, I found
> the following: 
> 
> Index: lib/libc/asr/res_send_async.c
> ===
> RCS file: /cvs/src/lib/libc/asr/res_send_async.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 res_send_async.c
> --- lib/libc/asr/res_send_async.c 15 Mar 2017 15:54:41 -  1.36
> +++ lib/libc/asr/res_send_async.c 11 Jul 2017 20:09:59 -
> @@ -385,7 +385,7 @@ setup_query(struct asr_query *as, const 
>   _asr_pack_query(, type, class, dname);
>   if (as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC))
>   _asr_pack_edns0(, MAXPACKETSZ,
> - as->as_ctx->ac_options & RES_USE_DNSSEC);
> + as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC));
>   if (p.err) {
>   DPRINT("error packing query");
>   errno = EINVAL;

The current code is correct, RES_USE_EDNS0 does not imply RES_USE_DNSSEC.
The real problem is that there is no resolv.conf option for RES_USE_DNSSEC.
It can only be set in user code by tweaking _res.options.

Eric.

Re: Remove accents from fortunes

[Theo Buehler]

On Wed, Jul 12, 2017 at 12:07:47AM -0600, Anthony J. Bentley wrote:
> Ingo Schwarze writes:
> > So here is a patch that makes putting UTF-8 characters into
> > fortune/datfiles safe.
> 
> And here is a patch that retains the accents in the datfiles as UTF-8.

ok

one comment:

> Index: datfiles/fortunes

> @@ -4066,7 +4066,7 @@ Five is a sufficiently close approximati
>   -- Robert Firth
>  %
>  Flappity, floppity, flip
> -The mouse on the m"obius strip;
> +The mouse on the möbius strip;

Shouldn't this be Möbius?
https://en.wikipedia.org/wiki/M%C3%B6bius_strip

>   The strip revolved,
>   The mouse dissolved
>  In a chronodimensional skip.

Re: Remove accents from fortunes

[Stefan Sperling]

On Wed, Jul 12, 2017 at 12:07:47AM -0600, Anthony J. Bentley wrote:
> Ingo Schwarze writes:
> > So here is a patch that makes putting UTF-8 characters into
> > fortune/datfiles safe.
> 
> And here is a patch that retains the accents in the datfiles as UTF-8.

ok

Re: remove CPU_LIDSUSPEND/machdep.lidsuspend

[Martin Natano]

On Tue, Jul 11, 2017 at 02:40:16PM -0700, Mike Larkin wrote:
> On Tue, Jul 11, 2017 at 11:19:17PM +0200, Martin Natano wrote:
> > Go ahead replacing machdep.lidsuspend with cpu.lidaction. OpenBSD 6.1
> 
> cpu.lidaction or machdep.lidaction?

machdep.lidaction; I got confused by the #define name.

Re: Remove accents from fortunes

[Anthony J. Bentley]

Ingo Schwarze writes:
> So here is a patch that makes putting UTF-8 characters into
> fortune/datfiles safe.

And here is a patch that retains the accents in the datfiles as UTF-8.

Index: Notes
===
RCS file: /cvs/src/games/fortune/Notes,v
retrieving revision 1.3
diff -u -p -r1.3 Notes
--- Notes   28 Sep 2004 20:40:01 -  1.3
+++ Notes   12 Jul 2017 05:57:23 -
@@ -107,8 +107,7 @@ Limericks are indented as follows, all l
No more than debris,
If the input was short of exact.
 
-Accents precede the letter they are over, e.g., "`^He" for e with a grave
-accent.  Underlining is done on a word-by-word basis, with the underlines
+Underlining is done on a word-by-word basis, with the underlines
 preceding the word, e.g., "__^H^Hhi ^H^H^H^Hthere".
 
 No fortune should run beyond 72 characters on a single line without good
Index: datfiles/fortunes
===
RCS file: /cvs/src/games/fortune/datfiles/fortunes,v
retrieving revision 1.49
diff -u -p -r1.49 fortunes
--- datfiles/fortunes   28 Dec 2015 12:32:12 -  1.49
+++ datfiles/fortunes   12 Jul 2017 05:57:24 -
@@ -232,7 +232,7 @@ test", said the outsider, "because I wan
Drescher took the paper that was offered him and put it into
 the toaster -- "I wish the toaster to be happy too."
 %
-A diva who specializes in risqu'e arias is an off-coloratura soprano ...
+A diva who specializes in risqué arias is an off-coloratura soprano ...
 %
A doctor, an architect, and a computer scientist were arguing
 about whose profession was the oldest.  In the course of their
@@ -3620,8 +3620,8 @@ Error in operator: add beer
 %
 Es brilig war.  Die schlichte Toven
Wirrten und wimmelten in Waben;
-Und aller-m"umsige Burggoven
-   Dir mohmen R"ath ausgraben.
+Und aller-mümsige Burggoven
+   Dir mohmen Räth ausgraben.
-- Lewis Carroll, "Through the Looking Glass"
 %
 Eternal nothingness is fine if you happen to be dressed for it.
@@ -4066,7 +4066,7 @@ Five is a sufficiently close approximati
-- Robert Firth
 %
 Flappity, floppity, flip
-The mouse on the m"obius strip;
+The mouse on the möbius strip;
The strip revolved,
The mouse dissolved
 In a chronodimensional skip.
@@ -5245,12 +5245,12 @@ I need your help, I say 'tween sobs,
 %
 Here in my heart, I am Helen;
I'm Aspasia and Hero, at least.
-I'm Judith, and Jael, and Madame de Sta"el;
+I'm Judith, and Jael, and Madame de Staël;
I'm Salome, moon of the East.
 
 Here in my soul I am Sappho;
Lady Hamilton am I, as well.
-In me R'ecamier vies with Kitty O'Shea,
+In me Récamier vies with Kitty O'Shea,
With Dido, and Eve, and poor nell.
 
 I'm all of the glamorous ladies
@@ -5820,7 +5820,7 @@ streets and frighten the horses.
 %
 "I don't object to sex before marriage, but two minutes before?!?"
 %
-"I don't think so," said Ren'e Descartes.  Just then, he vanished.
+"I don't think so," said René Descartes.  Just then, he vanished.
 %
 I don't think they could put him in a mental hospital.  On the other
 hand, if he were already in, I don't think they'd let him out.
@@ -7056,10 +7056,10 @@ Ignisecond, n.:
door even as the brain is saying, "my keys are in there!"
-- Rich Hall, "Sniglets"
 %
-Il brilgue: les t^oves libricilleux
+Il brilgue: les tôves libricilleux
Se gyrent et frillant dans le guave,
-Enm^im'es sont les gougebosquex,
-   Et le m^omerade horgrave.
+Enmîmés sont les gougebosquex,
+   Et le mômerade horgrave.
-- Lewis Carroll, "Through the Looking Glass"
 %
 Iles's Law:
@@ -10423,7 +10423,7 @@ laughter, singing
 The square was finally cleared by armed carabineri with tears of
 laughter streaming down their faces.  The event set a record for
 hilarious civic functions, smashing the previous record set when Baron
-Hans Neizant B"ompzidaize was elected Landburgher of K"oln in 1653.
+Hans Neizant Bömpzidaize was elected Landburgher of Köln in 1653.
-- Mike Harding, "The Armchair Anarchist's Almanac"
 %
 Portable, adj.:
@@ -11211,7 +11211,7 @@ pencil.
 Science is facts; just as houses are made of stones, so is science made
 of facts; but a pile of stones is not a house and a collection of facts
 is not necessarily science.
-   -- Henri Poincair'e
+   -- Henri Poincairé
 %
 Science is what happens when preconception meets verification.
 %
@@ -11760,7 +11760,7 @@ Spouse, n.:
wouldn't have had if you'd stayed single.
 %
 Star Wars is adolescent nonsense; Close Encounters is obscurantist
-drivel; Star Trek can turn your brains to pur'ee of bat guano; and the
+drivel; Star Trek can turn your brains to purée of bat guano; and the
 greatest science fiction series of all time is Doctor Who!  And I'll
 take you all on, one-by-one or all in a