Re: RFC 7217: random but stable addresses (take 2)

2017-07-13 Thread Ted Unangst
David Gwynne wrote:
> secondly, im always wary of truncating hash output in case it throws away 
> some of the guarantees it's supposed to provide. if you cut sha512 output 
> down to an 8th of its size, is it 8 times easier to calculate a collision, or 
> more than 8 times easier? sha384 being a truncation of sha512 kind of argues 
> against this though.

on this point, all the bits are supposed to be equally good. you can take as
many as you want, and just like that, you have an X bit hash.



Re: pledge ifstated

2017-07-13 Thread Rob Pierce
On Mon, Jul 10, 2017 at 01:21:58PM -0400, Rob Pierce wrote:
> The following diff is loosely based on the approach that was taken for
> pledging mountd. Other code/approaches leveraged from various networking
> daemons.
> 
> This first step moves the ioctl with SIOCGIFDATA call to a privileged
> child so we can at least pledge "stdio rpath dns inet proc exec" without
> (intentionally) changing existing behaviour.
> 
> Comments and suggestions welcome.
> 
> Thanks!
> 
> Rob

An unnecessary call to log_info snuck in. Here is a clean diff.

Rob

Index: Makefile
===
RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- Makefile20 Jul 2010 02:05:15 -  1.10
+++ Makefile14 Jul 2017 01:12:58 -
@@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations
 CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
 YFLAGS=
 MAN= ifstated.8 ifstated.conf.5
-LDADD+=-levent
+LDADD+=-levent -lutil
 DPADD+= ${LIBEVENT}
 
 .include 
Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.50
diff -u -p -r1.50 ifstated.c
--- ifstated.c  4 Jul 2017 21:09:52 -   1.50
+++ ifstated.c  14 Jul 2017 01:12:58 -
@@ -25,13 +25,16 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 
 #include 
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -39,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -47,7 +51,9 @@
 #include "log.h"
 
 struct  ifsd_config *conf = NULL, *newconf = NULL;
+struct  imsgbuf ibuf;
 
+pid_t   privchild_pid;
 int opts = 0;
 int opt_inhibit = 0;
 char   *configfile = "/etc/ifstated.conf";
@@ -74,6 +80,7 @@ void  do_action(struct ifsd_action *);
 void   remove_action(struct ifsd_action *, struct ifsd_state *);
 void   remove_expression(struct ifsd_expression *,
struct ifsd_state *);
+void   privchild(int);
 
 __dead void
 usage(void)
@@ -89,6 +96,7 @@ int
 main(int argc, char *argv[])
 {
struct timeval tv;
+   int socks[2];
int ch, rt_fd;
int debug = 0;
unsigned int rtfilter;
@@ -143,6 +151,22 @@ main(int argc, char *argv[])
if (!debug)
daemon(1, 0);
 
+   if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) {
+   fatal("%s: socketpair", __func__);
+   }
+
+   switch (privchild_pid = fork()) {
+   case -1:
+   fatal("%s: fork", __func__);
+   case 0:
+   close(socks[0]);
+   privchild(socks[1]);
+   }
+   close(socks[1]);
+
+   imsg_init(, socks[0]);
+   setproctitle("parent");
+
event_init();
log_init(debug, LOG_DAEMON);
log_setverbose(opts & IFSD_OPT_VERBOSE);
@@ -160,6 +184,9 @@ main(int argc, char *argv[])
, sizeof(rtfilter)) == -1) /* not fatal */
log_warn("%s: setsockopt tablefilter", __func__);
 
+   if (pledge("stdio rpath dns inet proc exec", NULL) == -1)
+   fatal("pledge");
+
signal_set(_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(_ev, NULL);
 
@@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void
 void
 sigchld_handler(int fd, short event, void *arg)
 {
+   pid_t pid;
+   int status;
+
+   do {
+   pid = waitpid(privchild_pid, , WNOHANG);
+   if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status)))
+   fatal("privchild %i exited due to receipt of signal %i",
+   privchild_pid, WTERMSIG(status));
+   } while (pid == -1 && errno == EINTR);
+
check_external_status(>initstate);
if (conf->curstate != NULL)
check_external_status(conf->curstate);
@@ -599,29 +636,74 @@ do_action(struct ifsd_action *action)
 void
 fetch_ifstate(void)
 {
+   struct imsg imsg;
struct ifaddrs *ifap, *ifa;
+   struct pollfd pfd[1];
char *oname = NULL;
int sock = socket(AF_INET, SOCK_DGRAM, 0);
+   int done = 0, state, rv;
+   ssize_t n;
 
if (getifaddrs() != 0)
err(1, "getifaddrs");
 
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
struct ifreq ifr;
-   struct if_data ifrdat;
 
if (oname && !strcmp(oname, ifa->ifa_name))
continue;
oname = ifa->ifa_name;
 
strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name));
-   ifr.ifr_data = (caddr_t)
-
-   if (ioctl(sock, SIOCGIFDATA, (caddr_t)) == -1)
-   continue;
 
-   scan_ifstate(if_nametoindex(ifa->ifa_name),
-   ifrdat.ifi_link_state, 0);
+   /*
+* synchronous imsg request/response to privchild

Re: RFC 7217: random but stable addresses (take 2)

2017-07-13 Thread David Gwynne

> On 14 Jul 2017, at 06:30, Christian Weisgerber  wrote:
> 
> On 2017-07-13, Florian Obser  wrote:
> 
>> It switches the hash function to SipHash24 from sha512 as suggested by dlg
> 
> It's for from clear to me whether SipHash is suitable for crypto
> operations, and which ones, other than the hash tables it was
> designed for.

the abstract on the siphash paper says:

"SipHash is a family of pseudorandom functions optimized for short inputs. 
Target applications include network traffic authentication and hash-table 
lookups protected against hash-flooding denial-of-service attacks."

i somehow have the impression that it's use for hash tables was a convenient 
bonus rather than a specific design target of the algorithm. it was handy that 
it is good at spreading bits from a small input into its output.

> 
> We went with SHA-512 for RFC 1948 TCP initial sequence number
> generation, which is arguably performance-sensitive, while RFC 7217
> addresses are certainly not.  I think we should use a SHA-2-family
> function such as SHA-512 here.

i dont mind which way we go, but i would like to make two (possibly invalid) 
arguments. 

firstly, we're generating public information. if anyone wants to "collide" with 
us, the can just copy and use the same ip, regardless of how we got there. as 
long as the key is strong enough we should be able to prevent prediction of 
other soii based addresses. if we're trying to protect against recovery of 
information, specifically the mac address, we could just not feed that into the 
hash. use the name of the interface instead, which is what netstart uses to 
assign addresses too.

secondly, im always wary of truncating hash output in case it throws away some 
of the guarantees it's supposed to provide. if you cut sha512 output down to an 
8th of its size, is it 8 times easier to calculate a collision, or more than 8 
times easier? sha384 being a truncation of sha512 kind of argues against this 
though.

with regard to the code, siphash implied a length for soiikey which makes the 
sysctl handling a lot easier. if (when?) we go back to sha512 id like to see a 
fixed key length remain. we can make it bigger, but just keep it one size.

> 
>> +# Apply soiikey.conf settings.
>> +soiikey_conf() {
>> +stripcom /etc/soiikey.conf |
>> +while read _line; do
>> +sysctl -q "net.inet6.ip6.soiikey=$_line"
>> +done
>> +}
> 
> I think .conf is a strange choice of name for what is not a
> configuration file but effectively a private key, cf.
> 
> /etc/{iked,isakmpd}/private/local.key
> /etc/ssh/ssh_host__key

id also like to see key in the name, not conf.

> 
>> +SOOIs use the whole 64 bit of the host part while SLAAC addresses are
>> +formed from MAC addresses and have 48 bits of entropy at most.
> 
> 46 bits.
> (The first bit of a MAC address is 0 for unicast addresses, the
> second is 0 for "universally administered" addresses, i.e., those
> that are uniquely assigned to a device by its manufacturer.)
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 



time(1): kill some lint-era voids, switch to getprogname(3)

2017-07-13 Thread Scott Cheloha
The (void) casts are going out of style.  While here,
switch from __progname to getprogname(3)

--
Scott Cheloha

Index: usr.bin/time/time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.22
diff -u -p -r1.22 time.c
--- usr.bin/time/time.c 13 Jul 2017 06:39:54 -  1.22
+++ usr.bin/time/time.c 14 Jul 2017 00:34:33 -
@@ -93,8 +93,8 @@ main(int argc, char *argv[])
}
 
/* parent */
-   (void)signal(SIGINT, SIG_IGN);
-   (void)signal(SIGQUIT, SIG_IGN);
+   signal(SIGINT, SIG_IGN);
+   signal(SIGQUIT, SIG_IGN);
while (wait3(, 0, ) != pid)
;
clock_gettime(CLOCK_MONOTONIC, );
@@ -180,9 +180,7 @@ main(int argc, char *argv[])
 __dead void
 usage(void)
 {
-   extern char *__progname;
-
-   (void)fprintf(stderr, "usage: %s [-lp] utility [argument ...]\n",
-   __progname);
+   fprintf(stderr, "usage: %s [-lp] utility [argument ...]\n",
+   getprogname());
exit(1);
 }



Re: armv7 _dcache_wbinv_all, or _dcache_wb_all ?

2017-07-13 Thread Artturi Alm
On Fri, Jul 14, 2017 at 03:28:25AM +0300, Artturi Alm wrote:
> Hi,
> 
> i'm having hard time choosing, between this diff, and the one that
> renames it to for what it does, which would likely incur cleanup
> elsewhere. current is just wrong.. but i'm open to be taught on this
> matter anyway.
> 
> -Artturi
> 

Wonders of manually recreating a quick diff by hand above.. fixed:

diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
index c91108e7066..fd5385043fc 100644
--- a/sys/arch/arm/arm/cpufunc.c
+++ b/sys/arch/arm/arm/cpufunc.c
@@ -291,8 +291,8 @@ armv7_dcache_wbinv_all()
for (ways = 0; ways < nways; ways++) {
word = wayval | setval | lvl;
 
-   /* Clean D cache SE with Set/Index */
-   __asm volatile("mcr p15, 0, %0, c7, c10, 2"
+   /* Data cache clean and invalidate by set/way */
+   __asm volatile("mcr p15, 0, %0, c7, c14, 2\n"
: : "r" (word));
wayval += wayincr;
}



time(1): perror(3) -> err(3) and friends

2017-07-13 Thread Scott Cheloha
We currently use a mix of perror(3) and err(3).

In one case you can merge perror + exit into err, which is nice.

The warns, though, are not equivalent (you get a "time: " prefix), so
maybe this is too risky.

Putting it out here anyway.

--
Scott Cheloha

Index: usr.bin/time/time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.22
diff -u -p -r1.22 time.c
--- usr.bin/time/time.c 13 Jul 2017 06:39:54 -  1.22
+++ usr.bin/time/time.c 14 Jul 2017 00:24:59 -
@@ -82,12 +82,11 @@ main(int argc, char *argv[])
clock_gettime(CLOCK_MONOTONIC, );
switch(pid = vfork()) {
case -1:/* error */
-   perror("time");
-   exit(1);
+   err(1, NULL);
/* NOTREACHED */
case 0: /* child */
execvp(*argv, argv);
-   perror(*argv);
+   warn("%s", *argv);
_exit((errno == ENOENT) ? 127 : 126);
/* NOTREACHED */
}
@@ -170,7 +169,7 @@ main(int argc, char *argv[])
 
if (exitonsig) {
if (signal(exitonsig, SIG_DFL) == SIG_ERR)
-   perror("signal");
+   warn("signal");
else
kill(getpid(), exitonsig);
}



armv7 _dcache_wbinv_all, or _dcache_wb_all ?

2017-07-13 Thread Artturi Alm
Hi,

i'm having hard time choosing, between this diff, and the one that
renames it to for what it does, which would likely incur cleanup
elsewhere. current is just wrong.. but i'm open to be taught on this
matter anyway.

-Artturi


diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
index c91108e7066..0631ba8fa78 100644
--- a/sys/arch/arm/arm/cpufunc.c
+++ b/sys/arch/arm/arm/cpufunc.c
@@ -291,8 +291,8 @@ armv7_dcache_wbinv_all()
for (ways = 0; ways < nways; ways++) {
word = wayval | setval | lvl;
 
-   /* Clean D cache SE with Set/Index */
-   __asm volatile("mcr p15, 0, %0, c7, c10, 2"
+   /* Data cache clean and invalidate by set/way */
+   __asm volatile("mcr p15, 0, rr, c7, c14, 2\n"
: : "r" (word));
wayval += wayincr;
}



time(1): kill some NOTREACHEDs

2017-07-13 Thread Scott Cheloha
style(9) says these can go.

--
Scott Cheloha

Index: usr.bin/time/time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.22
diff -u -p -r1.22 time.c
--- usr.bin/time/time.c 13 Jul 2017 06:39:54 -  1.22
+++ usr.bin/time/time.c 14 Jul 2017 00:16:39 -
@@ -69,10 +69,8 @@ main(int argc, char *argv[])
break;
default:
usage();
-   /* NOTREACHED */
}
}
-
argc -= optind;
argv += optind;
 
@@ -84,12 +82,10 @@ main(int argc, char *argv[])
case -1:/* error */
perror("time");
exit(1);
-   /* NOTREACHED */
case 0: /* child */
execvp(*argv, argv);
perror(*argv);
_exit((errno == ENOENT) ? 127 : 126);
-   /* NOTREACHED */
}
 
/* parent */



time(1): make global flags local

2017-07-13 Thread Scott Cheloha
The flags don't need to be global, and there are more obvious
ways to zero a variable.

While here, order the stack structures and variables by size.

--
Scott Cheloha

Index: usr.bin/time/time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.22
diff -u -p -r1.22 time.c
--- usr.bin/time/time.c 13 Jul 2017 06:39:54 -  1.22
+++ usr.bin/time/time.c 14 Jul 2017 00:10:09 -
@@ -42,19 +42,17 @@
 #include 
 #include 
 
-int lflag;
-int portableflag;
-
 __dead void usage(void);
 
 int
 main(int argc, char *argv[])
 {
-   pid_t pid;
-   int ch, status;
-   struct timespec before, after, during;
struct rusage ru;
-   int exitonsig = 0;
+   struct timespec before, after, during;
+   pid_t pid;
+   int ch, exitonsig, lflag, portableflag, status;
+
+   exitonsig = lflag = portableflag = 0;
 
if (pledge("stdio proc exec", NULL) == -1)
err(1, "pledge");



Re: add simple ifstated regression test script

2017-07-13 Thread Rob Pierce
Sure, no problem. Thank you. 

Rob 




From: "Sebastian Benoit"  
To: "Rob Pierce"  
Cc: "tech"  
Sent: Thursday, July 13, 2017 6:12:14 PM 
Subject: Re: add simple ifstated regression test script 




BQ_BEGIN
Hi, 

i wanted to commit this, but saw that it does not have a licence yet. 

Can i add /usr/share/misc/license.template with your name and email-Adress? 

/Benno 

Rob Pierce(r...@2keys.ca) on 2017.07.06 13:12:26 -0400: 
> On Sun, Jul 02, 2017 at 06:29:07PM +0200, Sebastian Benoit wrote: 
> > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400: 
> > > I am currently using this regression script for basic ifstated sanity 
> > > testing. 
> > > 
> > > Still a work in progress. Requesting commit for safe keeping. 
> > 
> > 
> > Hi, 
> > 
> > this should go into /usr/src/regress/usr.sbin/ifstated 
> > (which does not esist yet). 
> > 
> > Also, it should hook into the regress framework (bsd.regress.mk(5)). 
> > 
> > As it needs some network configuration, maybe it should be similar to 
> > relayd regress tests. 
> > 
> > Happy to work with you on that. 
> > 
> > /B. 
> 
> I have updated the ifstated regression scripts based on your feedback. 
> 
> I also added a script to test drive the state machine. 
> 
> Both should be more systematic in coverage, but hopefully it is a good start. 
> 
> Regards, 
> 
> Rob 
> 
> Index: regress/usr.sbin/ifstated/Makefile 
> === 
> RCS file: regress/usr.sbin/ifstated/Makefile 
> diff -N regress/usr.sbin/ifstated/Makefile 
> --- /dev/null 1 Jan 1970 00:00:00 - 
> +++ regress/usr.sbin/ifstated/Makefile 6 Jul 2017 16:55:57 - 
> @@ -0,0 +1,13 @@ 
> +# $OpenBSD$ 
> + 
> +# Regress tests for ifstated 
> + 
> +REGRESS_TARGETS = run-regress-statemachine run-regress-ifstated 
> + 
> +run-regress-statemachine: 
> + sh ${.CURDIR}/statemachine 
> + 
> +run-regress-ifstated: 
> + sh ${.CURDIR}/ifstated 
> + 
> +.include  
> 
> Index: regress/usr.sbin/ifstated/ifstated 
> === 
> RCS file: regress/usr.sbin/ifstated/ifstated 
> diff -N regress/usr.sbin/ifstated/ifstated 
> --- /dev/null 1 Jan 1970 00:00:00 - 
> +++ regress/usr.sbin/ifstated/ifstated 6 Jul 2017 16:55:57 - 
> @@ -0,0 +1,148 @@ 
> +# $OpenBSD$ 
> + 
> +# Basic ifstated regression script to test interface changes. 
> + 
> +# Golbal variables 
> +VHIDA=252 
> +VHIDB=253 
> +PREFIX=172.16.0 
> +DEMOTE=ifconfig 
> +PROMOTE=ifconfig 
> +EVERY=5 
> +SLEEP=10 
> + 
> +cleanup() { 
> + ifconfig carp${VHIDA} destroy > /dev/null 2>&1 
> + ifconfig carp${VHIDB} destroy > /dev/null 2>&1 
> + rm working/ifstated.conf >/dev/null 2>&1 
> + rm working/ifstated.log >/dev/null 2>&1 
> + rm working/output.test >/dev/null 2>&1 
> + rm working/output.new >/dev/null 2>&1 
> + rm working/nohup.out >/dev/null 2>&1 
> + rmdir working >/dev/null 2>&1 
> +} 
> + 
> +fail() { 
> + echo FAILED 
> + cleanup 
> + exit 1 
> +} 
> + 
> +skip() { 
> + echo SKIPPED 
> + cleanup 
> + exit 0 
> +} 
> + 
> +trap 'skip' INT 
> + 
> +# look for a suitable physical interface for carp 
> +NIC="$(netstat -rn -finet | grep ^default | awk '{ print $8 }')" 
> +STATUS="$(ifconfig | grep -A5 ^${NIC} | grep status: | awk '{ print $2 }')" 
> + 
> +if [ "$STATUS" != "active" ] 
> +then 
> + echo "No suitable physical interface found." 
> + echo SKIPPED 
> + exit 0 
> +fi 
> + 
> +if [ "$(pgrep ifstated)" ] 
> +then 
> + echo "The ifstated daemon is already running." 
> + echo SKIPPED 
> + exit 0 
> +fi 
> + 
> +for interface in carp${VHIDA} carp${VHIDB} 
> +do 
> + ifconfig ${interface} > /dev/null 2>&1 
> + if [ $? -eq 0 ] 
> + then 
> + echo "Interface $interface already exists." 
> + echo SKIPPED 
> + exit 0 
> + fi 
> +done 
> + 
> +mkdir -p working 
> + 
> +cat > working/ifstated.conf < +# This is a config template for ifstated regression testing 
> +carp = "carp${VHIDA}.link.up" 
> +init-state primary 
> +net = '( "ping -q -c 1 -w 1 ${PREFIX}.${VHIDB} > /dev/null" every ${EVERY})' 
> +state primary { 
> + init { 
> + run "ifconfig" 
> + } 
> + if ! \$net 
> + set-state demoted 
> + if ! \$carp 
> + set-state demoted 
> +} 
> +state demoted { 
> + init { 
> + run "ifconfig" 
> + } 
> + if \$net && \$carp 
> + set-state primary 
> +} 
> +EOF 
> + 
> +ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 
> broadcast \ 
> + ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC} 
> +ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 
> broadcast \ 
> + ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} 
> + 
> +# give the carp interface time to come up as MASTER 
> +sleep 5 
> + 
> +cat > working/output.test < +changing state to primary 
> +changing state to demoted 
> +changing state to primary 
> +changing state to demoted 
> +changing state to primary 
> +changing state to demoted 
> +changing state to primary 
> +changing state to primary 
> +EOF 
> + 
> 

Re: add simple ifstated regression test script

2017-07-13 Thread Sebastian Benoit
Hi,

i wanted to commit this, but saw that it does not have a licence yet.

Can i add /usr/share/misc/license.template with your name and email-Adress?

/Benno

Rob Pierce(r...@2keys.ca) on 2017.07.06 13:12:26 -0400:
> On Sun, Jul 02, 2017 at 06:29:07PM +0200, Sebastian Benoit wrote:
> > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400:
> > > I am currently using this regression script for basic ifstated sanity 
> > > testing.
> > > 
> > > Still a work in progress. Requesting commit for safe keeping.
> > 
> > 
> > Hi,
> > 
> > this should go into /usr/src/regress/usr.sbin/ifstated
> > (which does not esist yet).
> > 
> > Also, it should hook into the regress framework (bsd.regress.mk(5)).
> > 
> > As it needs some network configuration, maybe it should be similar to
> > relayd regress tests.
> > 
> > Happy to work with you on that.
> > 
> > /B.
> 
> I have updated the ifstated regression scripts based on your feedback.
> 
> I also added a script to test drive the state machine.
> 
> Both should be more systematic in coverage, but hopefully it is a good start.
> 
> Regards,
> 
> Rob
> 
> Index: regress/usr.sbin/ifstated/Makefile
> ===
> RCS file: regress/usr.sbin/ifstated/Makefile
> diff -N regress/usr.sbin/ifstated/Makefile
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/usr.sbin/ifstated/Makefile6 Jul 2017 16:55:57 -
> @@ -0,0 +1,13 @@
> +# $OpenBSD$
> +
> +# Regress tests for ifstated
> +
> +REGRESS_TARGETS =run-regress-statemachine run-regress-ifstated
> +
> +run-regress-statemachine:
> + sh ${.CURDIR}/statemachine
> +
> +run-regress-ifstated:
> + sh ${.CURDIR}/ifstated
> +
> +.include 
> 
> Index: regress/usr.sbin/ifstated/ifstated
> ===
> RCS file: regress/usr.sbin/ifstated/ifstated
> diff -N regress/usr.sbin/ifstated/ifstated
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/usr.sbin/ifstated/ifstated6 Jul 2017 16:55:57 -
> @@ -0,0 +1,148 @@
> +# $OpenBSD$
> +
> +# Basic ifstated regression script to test interface changes.
> +
> +# Golbal variables
> +VHIDA=252
> +VHIDB=253
> +PREFIX=172.16.0
> +DEMOTE=ifconfig
> +PROMOTE=ifconfig
> +EVERY=5
> +SLEEP=10
> +
> +cleanup() {
> + ifconfig carp${VHIDA} destroy > /dev/null 2>&1
> + ifconfig carp${VHIDB} destroy > /dev/null 2>&1
> + rm working/ifstated.conf >/dev/null 2>&1
> + rm working/ifstated.log >/dev/null 2>&1
> + rm working/output.test >/dev/null 2>&1
> + rm working/output.new >/dev/null 2>&1
> + rm working/nohup.out >/dev/null 2>&1
> + rmdir working >/dev/null 2>&1
> +}
> +
> +fail() {
> + echo FAILED
> + cleanup
> + exit 1
> +}
> +
> +skip() {
> + echo SKIPPED
> + cleanup
> + exit 0
> +}
> +
> +trap 'skip' INT
> +
> +# look for a suitable physical interface for carp
> +NIC="$(netstat -rn -finet | grep ^default | awk '{ print $8 }')"
> +STATUS="$(ifconfig | grep -A5 ^${NIC} | grep status: | awk '{ print $2 }')"
> +
> +if [ "$STATUS" != "active" ]
> +then
> + echo "No suitable physical interface found."
> + echo SKIPPED
> + exit 0
> +fi
> +
> +if [ "$(pgrep ifstated)" ]
> +then
> + echo "The ifstated daemon is already running."
> + echo SKIPPED
> + exit 0
> +fi
> +
> +for interface in carp${VHIDA} carp${VHIDB}
> +do
> + ifconfig ${interface} > /dev/null 2>&1
> + if [ $? -eq 0 ]
> + then
> + echo "Interface $interface already exists."
> + echo SKIPPED
> + exit 0
> + fi
> +done
> +
> +mkdir -p working
> +
> +cat > working/ifstated.conf < +# This is a config template for ifstated regression testing
> +carp = "carp${VHIDA}.link.up"
> +init-state primary
> +net = '( "ping -q -c 1 -w 1 ${PREFIX}.${VHIDB} > /dev/null" every ${EVERY})'
> +state primary {
> + init {
> + run "ifconfig"
> + }
> + if ! \$net
> + set-state demoted
> + if ! \$carp
> + set-state demoted
> +}
> +state demoted {
> + init {
> + run "ifconfig"
> + }
> + if \$net && \$carp
> + set-state primary
> +}
> +EOF
> +
> +ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 
> broadcast \
> +   ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC}
> +ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 
> broadcast \
> +   ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC}
> +
> +# give the carp interface time to come up as MASTER
> +sleep 5
> +
> +cat > working/output.test < +changing state to primary
> +changing state to demoted
> +changing state to primary
> +changing state to demoted
> +changing state to primary
> +changing state to demoted
> +changing state to primary
> +changing state to primary
> +EOF
> +
> +(cd working && nohup ifstated -dvf ./ifstated.conf > ifstated.log 2>&1) &
> +
> +sleep ${SLEEP}
> +ifconfig carp${VHIDA} down
> +sleep ${SLEEP}
> +ifconfig carp${VHIDA} up
> 

Re: Add Diffie-Hellman group negotiation to iked

2017-07-13 Thread viq
On 17-06-25 21:44:24, Tim Stewart wrote:
> Hi,
> 
> In this message I've tried to encode everything I've done to allow
> strongSwan on Android to connect with iked, including the latest patch.
> I have also verified that it breaks neither initial negotiation nor
> Child SA rekeying for OpenBSD, Windows, and strongSwan (on Android)
> clients.

 This patch gets my android phone much closer to being able to negotiate
 a connection, but there are still issues. Paraphrasing analysis mikeb
 performed on IRC:
 android sends incorrect (for us) group, and with this patch we now send
 a failure message and android retries. But, we don't increment msgid
 "because we did sa_free and restarted, so we can assume that android
 thinks that negotiation continues, that's why it re-sends the
 IKE_SA_INIT"
 
> Stuart Henderson  writes:
> 
> > On 2017/05/22 01:52, Tim Stewart wrote:
> >> Hello again,
> >>
> >> Tim Stewart  writes:
> >>
> >> > Tim Stewart  writes:
> >> >
> >> >> This patch teaches iked to reject a KE with a Notify payload of type
> >> >> INVALID_KE_PAYLOAD when the KE uses a different Diffie-Hellman group
> >> >> than is configured locally.  The rejection indicates the desired
> >> >> group.
> >> >>
> >> >> In my environment, this patch allows stock strongSwan on Android from
> >> >> the Google Play store to interop with iked.  strongSwan's logs show
> >> >> the following once iked is patched:
> >> >>
> >> >>   [IKE] initiating IKE_SA android[7] to 192.0.2.1
> >> >>   [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) 
> >> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ]
> >> >>   [ENC] parsed IKE_SA_INIT response 0 [ N(INVAL_KE) ]
> >> >>   [IKE] peer didn't accept DH group ECP_256, it requested MODP_2048
> >> >>   [IKE] initiating IKE_SA android[7] to 192.0.2.1
> >> >>   [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) 
> >> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ]
> >> >>   [ENC] parsed IKE_SA_INIT response 0 [ SA KE No N(NATD_S_IP) 
> >> >> N(NATD_D_IP) CERTREQ N(HASH_ALG) ]
> >> >>
> >> >> I'm happy to iterate on this patch to get it into proper shape for
> >> >> inclusion.
> >> >
> >> > I discovered a bug in the previous patch that broke renegotiation of
> >> > CHILD SAs.  I was ignoring "other than NONE" in the following sentence
> >> > from RFC 5996 section 3.4:
> >> >
> >> >   If the selected proposal uses a different Diffie-Hellman group
> >> >   (other than NONE), the message MUST be rejected with a Notify
> >> >   payload of type INVALID_KE_PAYLOAD.
> >> >
> >> > The new patch below repairs the flaw.
> >>
> >> After re-reading relevant parts of the RFC I'm not convinced that my fix
> >> (rejecting with INVALID_KE_PAYLOAD unless msg->msg_dhgroup is
> >> IKEV2_XFORMDH_NONE) is correct.  It happens to resolve my local issue
> >> but I think it may accidentally work due to a side effect of the code
> >> path for rekeying a child SA.
> >>
> >> I will look at it more closely this week.
> 
> My first patch did, in fact, break Child SAs rekeying.  I have a new
> patch at the end of this message that simply restricts DH group
> negotiation to IKE SAs (I *think* that DH group guessing only applies to
> IKE SAs, and perhaps only the IKE_SA_INIT exchange, but I'm still
> working through the RFC).  This may not be the ultimate solution, but it
> does allow us to move forward.
> 
> > Hi, I'm interested in this, but wasn't able to get strongswan to connect
> > with either of your patches (and had iked exiting on one attempt, though
> > I haven't been able to repeat that).
> 
> After making changes I usually rebuild all of /usr/src/sbin/iked/ (make
> clean && make).  I started doing this after experiencing similar exits
> and the problem went away.  That could just be a coincidence, though!
> 
> > If you have any updates please do send them here, it can be a bit slow
> > getting feedback on iked diffs at times but it definitely is worth sending
> > them out.
> 
> I'll summarize what I know about strongSwan (on Android) and iked
> interop.  strongSwan chooses ECP_256 for its DH group guess when it
> starts the IKE_SA_INIT exchange.  The negotiation gets pretty far if I
> specify `group ecp256' in the ikesa directive, but there there is some
> incompatibility between iked and strongSwan that causes the following:
> 
> ...
> ikev2_recv: IKE_AUTH request from initiator 5.6.7.8:49317 to 1.2.3.4:4500 
> policy 'default' id 1, 2040 bytes
> ikev2_recv: ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8
> ikev2_recv: updated SA to peer 5.6.7.8:49317 local 1.2.3.4:4500
> ikev2_pld_parse: header ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8 
> nextpayload SK version 0x20 exchange IKE_AUTH flags 0x08 msgid 1 length 2040 
> response 0
> ikev2_pld_payloads: payload SK nextpayload IDi critical 0x00 length 2012
> ikev2_msg_decrypt: IV length 16
> ikev2_msg_decrypt: encrypted payload length 1968
> ikev2_msg_decrypt: integrity checksum 

Re: RFC 7217: random but stable addresses (take 2)

2017-07-13 Thread Christian Weisgerber
On 2017-07-13, Florian Obser  wrote:

> It switches the hash function to SipHash24 from sha512 as suggested by dlg

It's for from clear to me whether SipHash is suitable for crypto
operations, and which ones, other than the hash tables it was
designed for.

We went with SHA-512 for RFC 1948 TCP initial sequence number
generation, which is arguably performance-sensitive, while RFC 7217
addresses are certainly not.  I think we should use a SHA-2-family
function such as SHA-512 here.

> +# Apply soiikey.conf settings.
> +soiikey_conf() {
> + stripcom /etc/soiikey.conf |
> + while read _line; do
> + sysctl -q "net.inet6.ip6.soiikey=$_line"
> + done
> +}

I think .conf is a strange choice of name for what is not a
configuration file but effectively a private key, cf.

/etc/{iked,isakmpd}/private/local.key
/etc/ssh/ssh_host__key

> +SOOIs use the whole 64 bit of the host part while SLAAC addresses are
> +formed from MAC addresses and have 48 bits of entropy at most.

46 bits.
(The first bit of a MAC address is 0 for unicast addresses, the
second is 0 for "universally administered" addresses, i.e., those
that are uniquely assigned to a device by its manufacturer.)

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: RFC 7217: random but stable addresses (take 2)

2017-07-13 Thread Florian Obser
On Thu, Jul 13, 2017 at 05:59:24PM +0200, Alexander Bluhm wrote:
> On Thu, Jul 13, 2017 at 03:43:50PM +, Florian Obser wrote:
> > It switches the hash function to SipHash24 from sha512 as suggested by dlg
> 
> Is is performance critical?  Then siphash would be better.

no

> 
> Is is a security concern?  Is is a problem that someone could try

maybe..

> to calculate our secret when he knows a bunch of our IP addresses?
> Then sha512 would be better.

if you know the key and the mac you can track a host when it moves to
a different prefix.

> 
> I don't know wether the algorithm is relevant here.  So I would
> have chosen sha512.

sha512 is certainly the conservative choice. note that we are only use
64 bit so the digest is a wee bit to big ;)

I'm happy to bikeshed this for a bit since it kinda defeats the
purpose if we need to change the hash function later.

dlg?

> 
> bluhm
> 

-- 
I'm not entirely sure you are real.



dhcpd - pf table handler child not cleaned up

2017-07-13 Thread Adam Wolk
Hi tech@,

sthen@ pointed out to me that dhcpd doesn't properly terminate the pf table
handler. 

I reproduced the issue both on 6.1 and -current.

Minimal config I used on my server:
/etc/dhcpd.conf
 subnet 45.63.9.186 netmask 255.255.255.224 {
   range 45.63.9.186 45.63.9.186;
 }

enabled dhcpd and used the -A flag to trigger the pf handler being spawned:
 rcctl enable dhcpd
 rcctl set dhcpd flags -A test

and performed the test

# rcctl start dhcpd; rcctl stop dhcpd; pgrep -lf dhcpd
dhcpd(ok)
dhcpd(ok)
96584 dhcpd: pf table handler

which demonstrates that indeed the pf table handler is left running.

Both the main program and the child spin in a for(;;) loop, there is no
code anticipating the need to signal a child to quit, there is also
no attempt to detect the parent dying by the child.

The child also uses a notreached exit(3) after the loop.

When the parent is not around the unattended child proceeds to puke all over
the logfiles:

Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe

brynet@ noticed the exit(3) instead of _exit(2) and he also pointed
out that the pf table handler is a bit optimistic on warnings instead of bailing
out with an error and terminating.

Examples:
55if ((fd = open(_PATH_DEV_PF, O_RDWR|O_NOFOLLOW, 0660)) == -1)
56log_warn("can't open pf device");

this child is useless if it can't pf

57if (chroot(_PATH_VAREMPTY) == -1)
58log_warn("chroot %s", _PATH_VAREMPTY);
59if (chdir("/") == -1)
60log_warn("chdir(\"/\")");
61if (setgroups(1, >pw_gid) ||
62setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
63setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
64log_warn("can't drop privileges");

not able to chroot and drop privileges seems like good enough reason to die.

76if (nfds > 0 && (pfd[0].revents & POLLHUP))
77log_warnx("pf pipe closed");

we won't get any work to do as the pipe is closed, should we die?

Now, I did try a quick diff changing log_warn on those to fatal()
https://junk.tintagel.pl/dhcpd-pf-handler.diff

This of course results in the child dying when the parent is gone
but we believe it's not commitable as the following needs to be
decided:

 - if the handler fails (ie. not able to open pf) should it signal
   the main process to die or just stop working (like it did now)?
 - fatal() can't be used as it uses exit(3) instead of _exit(2)
 - anything else that we might have missed

looking for pointers/suggestions on how to move forward with this.

regards,
Adam



fsck_ext2fs: Remove always false comparison

2017-07-13 Thread Matthew Martin
src/sbin/fsck_ext2fs/dir.c: In function 'dircheck':
src/sbin/fsck_ext2fs/dir.c:241: warning: comparison is always false due to 
limited range of data type

And indeed in /usr/include/ufs/ext2fs/ext2fs_dir.h e2d_namelen is
a u_int8_t and a few lines above #define EXT2FS_MAXNAMLEN255

diff --git dir.c dir.c
index 9864bacd0d3..105f5153997 100644
--- dir.c
+++ dir.c
@@ -237,8 +237,7 @@ dircheck(struct inodesc *idesc, struct ext2fs_direct *dp)
return (1);
size = EXT2FS_DIRSIZ(dp->e2d_namlen);
if (reclen < size ||
-   idesc->id_filesize < size ||
-   dp->e2d_namlen > EXT2FS_MAXNAMLEN)
+   idesc->id_filesize < size)
return (0);
for (cp = dp->e2d_name, size = 0; size < dp->e2d_namlen; size++)
if (*cp == '\0' || (*cp++ == '/'))



Re: [patch] Remove binc from vi(1)

2017-07-13 Thread Martijn van Duren
No one?

On 07/02/17 19:58, Martijn van Duren wrote:
> Any takers?
> 
> On 06/22/17 21:32, Martijn van Duren wrote:
>> Hello tech@,
>>
>> Attached a patch to remove the binc function from vi and replace it with
>> recallocarray. The functions effectively do the same thing since
>> BINC_{GOTO,RET} already do the nlen > llen comparison. I've run
>> this without any issues, but since recallocarray does extra checks and
>> binc ALWAYS allocates A LOT more than requested there might be some
>> bugs lurking.
>>
>> I haven't changed the name since the size component is always one and
>> hence they don't expose a similar interface.
>>
>> OK?
>>
>> martijn@
>>
>> Index: common/mem.h
>> ===
>> RCS file: /cvs/src/usr.bin/vi/common/mem.h,v
>> retrieving revision 1.9
>> diff -u -p -r1.9 mem.h
>> --- common/mem.h 7 May 2016 14:03:01 -   1.9
>> +++ common/mem.h 22 Jun 2017 19:02:28 -
>> @@ -14,30 +14,30 @@
>>  /* Increase the size of a malloc'd buffer.  Two versions, one that
>>   * returns, one that jumps to an error label.
>>   */
>> -#define BINC_GOTO(sp, lp, llen, nlen) { 
>> \
>> -void *L__bincp; \
>> +#define BINC_GOTO(sp, p, llen, nlen) {  
>> \
>> +void *tmpp; \
>>  if ((nlen) > (llen)) {  \
>> -if ((L__bincp = binc((sp), (lp), &(llen), (nlen)))  \
>> -== NULL)\
>> +if (((tmpp) = recallocarray((p), (llen), (nlen), 1))\
>> +== NULL) {  \
>> +msgq((sp), M_SYSERR, NULL); \
>> +free(p);\
>>  goto alloc_err; \
>> -/*  \
>> - * !!!  \
>> - * Possible pointer conversion. \
>> - */ \
>> -(lp) = L__bincp;\
>> +}   \
>> +llen = nlen;\
>> +(p) = tmpp; \
>>  }   \
>>  }
>> -#define BINC_RET(sp, lp, llen, nlen) {  
>> \
>> -void *L__bincp; \
>> +#define BINC_RET(sp, p, llen, nlen) {   
>> \
>> +void *tmpp; \
>>  if ((nlen) > (llen)) {  \
>> -if ((L__bincp = binc((sp), (lp), &(llen), (nlen)))  \
>> -== NULL)\
>> +if (((tmpp) = recallocarray((p), (llen), (nlen), 1))\
>> +== NULL) {  \
>> +msgq((sp), M_SYSERR, NULL); \
>> +free(p);\
>>  return (1); \
>> -/*  \
>> - * !!!  \
>> - * Possible pointer conversion. \
>> - */ \
>> -(lp) = L__bincp;\
>> +}   \
>> +llen = nlen;\
>> +(p) = tmpp; \
>>  }   \
>>  }
>>  
>> Index: common/util.c
>> ===
>> RCS file: /cvs/src/usr.bin/vi/common/util.c,v
>> retrieving revision 1.15
>> diff -u -p -r1.15 util.c
>> --- common/util.c27 May 2016 09:18:11 -  1.15
>> +++ common/util.c22 Jun 2017 19:02:28 -
>> @@ -24,43 +24,6 @@
>>  
>>  #include "common.h"
>>  
>> -#define MAXIMUM(a, b)   (((a) > (b)) ? (a) : (b))
>> -
>> -/*
>> - * binc --
>> - *  Increase the size of a buffer.
>> - *
>> - * PUBLIC: void *binc(SCR *, void *, size_t *, size_t);
>> - */
>> -void *
>> -binc(SCR *sp, void *bp, size_t *bsizep, size_t min)
>> -{
>> -size_t csize;
>> -
>> -/* If already larger than the 

pledge ifstated

2017-07-13 Thread Rob Pierce
The following diff is loosely based on the approach that was taken for
pledging mountd. Other code/approaches leveraged from various networking
daemons.

This first step moves the ioctl with SIOCGIFDATA call to a privileged
child so we can at least pledge "stdio rpath dns inet proc exec" without
(intentionally) changing existing behaviour.

Comments and suggestions welcome.

Thanks!

Rob

Index: Makefile
===
RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- Makefile20 Jul 2010 02:05:15 -  1.10
+++ Makefile13 Jul 2017 16:17:20 -
@@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations
 CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
 YFLAGS=
 MAN= ifstated.8 ifstated.conf.5
-LDADD+=-levent
+LDADD+=-levent -lutil
 DPADD+= ${LIBEVENT}
 
 .include 
Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.50
diff -u -p -r1.50 ifstated.c
--- ifstated.c  4 Jul 2017 21:09:52 -   1.50
+++ ifstated.c  13 Jul 2017 16:17:20 -
@@ -25,13 +25,16 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 
 #include 
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -39,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -47,7 +51,9 @@
 #include "log.h"
 
 struct  ifsd_config *conf = NULL, *newconf = NULL;
+struct  imsgbuf ibuf;
 
+pid_t   privchild_pid;
 int opts = 0;
 int opt_inhibit = 0;
 char   *configfile = "/etc/ifstated.conf";
@@ -74,6 +80,7 @@ void  do_action(struct ifsd_action *);
 void   remove_action(struct ifsd_action *, struct ifsd_state *);
 void   remove_expression(struct ifsd_expression *,
struct ifsd_state *);
+void   privchild(int);
 
 __dead void
 usage(void)
@@ -89,6 +96,7 @@ int
 main(int argc, char *argv[])
 {
struct timeval tv;
+   int socks[2];
int ch, rt_fd;
int debug = 0;
unsigned int rtfilter;
@@ -143,6 +151,22 @@ main(int argc, char *argv[])
if (!debug)
daemon(1, 0);
 
+   if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) {
+   fatal("%s: socketpair", __func__);
+   }
+
+   switch (privchild_pid = fork()) {
+   case -1:
+   fatal("%s: fork", __func__);
+   case 0:
+   close(socks[0]);
+   privchild(socks[1]);
+   }
+   close(socks[1]);
+
+   imsg_init(, socks[0]);
+   setproctitle("parent");
+
event_init();
log_init(debug, LOG_DAEMON);
log_setverbose(opts & IFSD_OPT_VERBOSE);
@@ -160,6 +184,9 @@ main(int argc, char *argv[])
, sizeof(rtfilter)) == -1) /* not fatal */
log_warn("%s: setsockopt tablefilter", __func__);
 
+   if (pledge("stdio rpath dns inet proc exec", NULL) == -1)
+   fatal("pledge");
+
signal_set(_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(_ev, NULL);
 
@@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void
 void
 sigchld_handler(int fd, short event, void *arg)
 {
+   pid_t pid;
+   int status;
+
+   do {
+   pid = waitpid(privchild_pid, , WNOHANG);
+   if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status)))
+   fatal("privchild %i exited due to receipt of signal %i",
+   privchild_pid, WTERMSIG(status));
+   } while (pid == -1 && errno == EINTR);
+
check_external_status(>initstate);
if (conf->curstate != NULL)
check_external_status(conf->curstate);
@@ -599,29 +636,75 @@ do_action(struct ifsd_action *action)
 void
 fetch_ifstate(void)
 {
+   struct imsg imsg;
struct ifaddrs *ifap, *ifa;
+   struct pollfd pfd[1];
char *oname = NULL;
int sock = socket(AF_INET, SOCK_DGRAM, 0);
+   int done = 0, state, rv;
+   ssize_t n;
 
if (getifaddrs() != 0)
err(1, "getifaddrs");
 
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
struct ifreq ifr;
-   struct if_data ifrdat;
 
if (oname && !strcmp(oname, ifa->ifa_name))
continue;
oname = ifa->ifa_name;
 
strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name));
-   ifr.ifr_data = (caddr_t)
-
-   if (ioctl(sock, SIOCGIFDATA, (caddr_t)) == -1)
-   continue;
 
-   scan_ifstate(if_nametoindex(ifa->ifa_name),
-   ifrdat.ifi_link_state, 0);
+   /*
+* synchronous imsg request/response to privchild
+*/
+   if (imsg_compose(, IMSG_IFSTATE_REQ, 0, 0, -1, ,
+  sizeof(struct ifreq)) == -1)
+   

Re: sosend netlock assertion crash

2017-07-13 Thread Martin Pieuchot
On 13/07/17(Thu) 18:10, Alexander Bluhm wrote:
> On Thu, Jul 13, 2017 at 10:01:09AM -0600, Todd C. Miller wrote:
> > On Thu, 13 Jul 2017 17:41:19 +0200, Alexander Bluhm wrote:
> > 
> > > My laptop just crashed while running some php ports regress tests.
> > > The kernel complained that it did not hold the netlock in sounlock()
> > > after the out label in sosend().  The assert is correct, let's fix
> > > the obvious offender.
> > 
> > Alternately, we could just initialize s to -42.
> 
> I still hope that this -42 hack will go away eventually.

It will.  The first step is to run the diff I sent to get rid of the
splsotnet()/splx() in the NET_LOCK().



Re: remove net.inet6.ip6.maxifdefrouters and net.inet6.ip6.maxifprefixes sysctls

2017-07-13 Thread Stuart Henderson
On 2017/07/13 15:23, Florian Obser wrote:
> pointed out by jmc, these sysctls are useless now, too, the kernel no
> longer tracks prefixes or default routers from router advertisements.
> OK?

ports/shells/nsh will need patching, not sure about others yet, X crashed
when I tried to search unpacked ports source..trying that one again now!



Re: sosend netlock assertion crash

2017-07-13 Thread Alexander Bluhm
On Thu, Jul 13, 2017 at 10:01:09AM -0600, Todd C. Miller wrote:
> On Thu, 13 Jul 2017 17:41:19 +0200, Alexander Bluhm wrote:
> 
> > My laptop just crashed while running some php ports regress tests.
> > The kernel complained that it did not hold the netlock in sounlock()
> > after the out label in sosend().  The assert is correct, let's fix
> > the obvious offender.
> 
> Alternately, we could just initialize s to -42.

I still hope that this -42 hack will go away eventually.

bluhm



Re: sosend netlock assertion crash

2017-07-13 Thread Todd C. Miller
On Thu, 13 Jul 2017 17:41:19 +0200, Alexander Bluhm wrote:

> My laptop just crashed while running some php ports regress tests.
> The kernel complained that it did not hold the netlock in sounlock()
> after the out label in sosend().  The assert is correct, let's fix
> the obvious offender.

Alternately, we could just initialize s to -42.

 - todd



Re: RFC 7217: random but stable addresses (take 2)

2017-07-13 Thread Alexander Bluhm
On Thu, Jul 13, 2017 at 03:43:50PM +, Florian Obser wrote:
> It switches the hash function to SipHash24 from sha512 as suggested by dlg

Is is performance critical?  Then siphash would be better.

Is is a security concern?  Is is a problem that someone could try
to calculate our secret when he knows a bunch of our IP addresses?
Then sha512 would be better.

I don't know wether the algorithm is relevant here.  So I would
have chosen sha512.

bluhm



OpenNTPD 6.1p1, 6.2p1 released

2017-07-13 Thread Brent Cook
We have made two new portable OpenNTPD releases today. These should be
arriving soon in the OpenNTPD directory of an OpenBSD mirror near you.

OpenNTPD 6.1p1 represents the version shipped with OpenBSD 6.1. It
provides a number of new features and reliability improvements.

OpenNTPD 6.2p1 is the first development snapshot from what will become
OpenBSD 6.2.

Changes since OpenNTPD 6.0p1


* Quieted warnings about constraint connection retries.

* Implemented fork+exec for ntpd child processes.

* Added imsg inter-process reliability fixes.

* Fixed memory leaks and reduced heap memory usage.

* Numerous logging improvements and additions.

* Added macOS 10.12 getentropy support.

* Fixed arc4random blacklist use native implementations where
  possible.

Changes since OpenNTPD 6.1p1


* Added option "query from " to ntpd.conf, to specify a local IP
  address for outgoing NTP queries.

The libtls library, as shipped with LibreSSL 2.5.0 or later, is
required to use the HTTPS constraint feature, though it is not
required to use OpenNTPD.



OpenNTPD 6.1p1, 6.2p1 released

2017-07-13 Thread Brent Cook
announce at openbsd.org
 Thu, 13 Jul 2017 10:41:48 -0500



RFC 7217: random but stable addresses (take 2)

2017-07-13 Thread Florian Obser
this has all the bells and whistles notably the installer and
documentation for the net.inet6.ip6.soiikey sysctl are missing.

the sysctl implementation is from dlg, all the mistakes are probably
tweaks by me ;)

It switches the hash function to SipHash24 from sha512 as suggested by dlg

Comments, tests, improvments, OKs?

diff --git etc/rc etc/rc
index 48e5671335f..fda8a620bbb 100644
--- etc/rc
+++ etc/rc
@@ -47,6 +47,14 @@ update_limit() {
done
 }
 
+# Apply soiikey.conf settings.
+soiikey_conf() {
+   stripcom /etc/soiikey.conf |
+   while read _line; do
+   sysctl -q "net.inet6.ip6.soiikey=$_line"
+   done
+}
+
 # Apply sysctl.conf(5) settings.
 sysctl_conf() {
stripcom /etc/sysctl.conf |
@@ -60,6 +68,7 @@ sysctl_conf() {
update_limit -n openfiles;;
esac
done
+   soiikey_conf
 }
 
 # Apply mixerctl.conf(5) settings.
@@ -154,6 +163,11 @@ make_keys() {
fi
 
ssh-keygen -A
+
+   if [[ ! -f /etc/soiikey.conf ]]; then
+   openssl rand -hex 16 > /etc/soiikey.conf && \
+   chmod 600 /etc/soiikey.conf && soiikey_conf
+   fi
 }
 
 # Re-link libraries, placing the objects in a random order.
diff --git sbin/ifconfig/brconfig.h sbin/ifconfig/brconfig.h
index ee68feb411b..09c871b352b 100644
--- sbin/ifconfig/brconfig.h
+++ sbin/ifconfig/brconfig.h
@@ -73,7 +73,7 @@ void switch_portno(const char *, const char *);
"\024\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6NOTRAILERS" \
"\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\
"\15LINK0\16LINK1\17LINK2\20MULTICAST"  \
-   "\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6"
+   "\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6\27INET6_NOSOII"
 
 void printb(char *, unsigned int, unsigned char *);
 
diff --git sbin/ifconfig/ifconfig.8 sbin/ifconfig/ifconfig.8
index cac8eafc2cb..30721e41ad4 100644
--- sbin/ifconfig/ifconfig.8
+++ sbin/ifconfig/ifconfig.8
@@ -1070,6 +1070,7 @@ protocol when supported by the access point.
 .Op Oo Fl Oc Ns Cm autoconfprivacy
 .Op Cm eui64
 .Op Cm pltime Ar n
+.Op Oo Fl Oc Ns Cm soii
 .Op Oo Fl Oc Ns Cm tentative
 .Op Cm vltime Ar n
 .Ek
@@ -1127,6 +1128,21 @@ Fill the interface index
 automatically.
 .It Cm pltime Ar n
 Set preferred lifetime for the address.
+.It Cm soii
+Enable persistent Semantically Opaque Interface Identifiers (SOIIs),
+as per RFC 7217, for link local and SLAAC addresses on the interface.
+These identifiers are enabled by default.
+The purpose of these identifiers is to make discovery of hosts by
+scanning a whole prefix more difficult.
+SOOIs use the whole 64 bit of the host part while SLAAC addresses are
+formed from MAC addresses and have 48 bits of entropy at most.
+In reality entropy can be  as low as 24 bit.
+See RFC 8064 for details.
+.It Cm -soii
+Disable IPv6 persistent Semantically Opaque Interface Identifiers on the
+interface.
+Currently configured addresses will not be removed until they become
+invalid.
 .It Cm tentative
 Set the IPv6 tentative address bit.
 .It Cm -tentative
diff --git sbin/ifconfig/ifconfig.c sbin/ifconfig/ifconfig.c
index d99bcb34871..bd7ce506398 100644
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -388,6 +388,8 @@ const structcmd {
{ "eui64",  0,  0,  setia6eui64 },
{ "autoconfprivacy",-IFXF_INET6_NOPRIVACY,  0,  setifxflags },
{ "-autoconfprivacy",   IFXF_INET6_NOPRIVACY,   0,  setifxflags },
+   { "soii",   -IFXF_INET6_NOSOII, 0,  setifxflags },
+   { "-soii",  IFXF_INET6_NOSOII,  0,  setifxflags },
 #ifndef SMALL
{ "hwfeatures", NEXTARG0,   0,  printifhwfeatures },
{ "metric", NEXTARG,0,  setifmetric },
diff --git sbin/slaacd/control.c sbin/slaacd/control.c
index 76b0f3b15ea..4f38d26331c 100644
--- sbin/slaacd/control.c
+++ sbin/slaacd/control.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git sbin/slaacd/engine.c sbin/slaacd/engine.c
index e02cb6ec3d0..cc7abd68512 100644
--- sbin/slaacd/engine.c
+++ sbin/slaacd/engine.c
@@ -70,6 +70,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -180,6 +181,7 @@ struct address_proposal {
uint8_t  prefix_len;
uint32_t vltime;
uint32_t pltime;
+   SIPHASH_KEY  soiikey;
 };
 
 struct dfr_proposal {
@@ -205,8 +207,10 @@ struct slaacd_iface {
uint32_t if_index;
int  running;
int  autoconfprivacy;
+   int  soii;
struct ether_addrhw_address;
struct sockaddr_in6  ll_address;
+   SIPHASH_KEY  

sosend netlock assertion crash

2017-07-13 Thread Alexander Bluhm
Hi,

My laptop just crashed while running some php ports regress tests.
The kernel complained that it did not hold the netlock in sounlock()
after the out label in sosend().  The assert is correct, let's fix
the obvious offender.

ok?

bluhm

Index: kern/uipc_socket.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.193
diff -u -p -r1.193 uipc_socket.c
--- kern/uipc_socket.c  8 Jul 2017 09:19:02 -   1.193
+++ kern/uipc_socket.c  13 Jul 2017 15:34:52 -
@@ -396,8 +396,9 @@ sosend(struct socket *so, struct mbuf *a
resid = top->m_pkthdr.len;
/* MSG_EOR on a SOCK_STREAM socket is invalid. */
if (so->so_type == SOCK_STREAM && (flags & MSG_EOR)) {
-   error = EINVAL;
-   goto out;
+   m_freem(top);
+   m_freem(control);
+   return (EINVAL);
}
if (uio && uio->uio_procp)
uio->uio_procp->p_ru.ru_msgsnd++;



Re: CVS: cvs.openbsd.org: src

2017-07-13 Thread Martijn van Duren
On 07/03/17 08:30, Martijn van Duren wrote:
> On 07/01/17 18:14, Mark Kettenis wrote:
>> CVSROOT: /cvs
>> Module name: src
>> Changes by:  kette...@cvs.openbsd.org2017/07/01 10:14:10
>>
>> Modified files:
>>  sys/dev/pci/drm: drm_irq.c drm_linux.c drm_linux.h 
>>   drm_linux_list.h drm_mm.c drm_mm.h drm_mode.h 
>>   drm_modes.c drm_rect.c drm_rect.h 
>>   drm_vma_manager.c files.drm i915_drm.h 
>>   i915_pciids.h linux_hdmi.h 
>>  sys/dev/pci/drm/i915: dvo.h dvo_ch7017.c dvo_ch7xxx.c dvo_ivch.c 
>>dvo_ns2501.c dvo_sil164.c dvo_tfp410.c 
>>i915_dma.c i915_drv.c i915_drv.h 
>>i915_gem.c i915_gem_context.c 
>>i915_gem_evict.c i915_gem_execbuffer.c 
>>i915_gem_gtt.c i915_gem_stolen.c 
>>i915_gem_tiling.c i915_gpu_error.c 
>>i915_irq.c i915_reg.h i915_suspend.c 
>>i915_trace.h intel_bios.c intel_bios.h 
>>intel_crt.c intel_ddi.c intel_display.c 
>>intel_dp.c intel_drv.h intel_dvo.c 
>>intel_fbdev.c intel_hdmi.c intel_i2c.c 
>>intel_lvds.c intel_modes.c 
>>intel_opregion.c intel_overlay.c 
>>intel_panel.c intel_pm.c 
>>intel_ringbuffer.c intel_ringbuffer.h 
>>intel_sdvo.c intel_sdvo_regs.h 
>>intel_sideband.c intel_sprite.c intel_tv.c 
>>intel_uncore.c 
>>  sys/dev/pci/drm/radeon: atombios_crtc.c atombios_dp.c 
>>  atombios_i2c.c nid.h r100.c radeon.h 
>>  radeon_connectors.c radeon_device.c 
>>  radeon_display.c radeon_fb.c 
>>  radeon_i2c.c radeon_irq_kms.c 
>>  radeon_kms.c radeon_legacy_crtc.c 
>>  radeon_legacy_encoders.c radeon_mode.h 
>>  radeon_pm.c 
>>  sys/dev/pci/drm/ttm: ttm_bo_manager.c 
>> Added files:
>>  sys/dev/pci/drm: drm_linux_atomic.h drm_mipi_dsi.h drm_modes.h 
>>   drm_modeset_lock.c drm_modeset_lock.h 
>>   drm_panel.c drm_panel.h drm_plane_helper.c 
>>   drm_plane_helper.h drm_probe_helper.c 
>>   linux_list_sort.c linux_types.h 
>>   linux_ww_mutex.h 
>>  sys/dev/pci/drm/i915: i915_cmd_parser.c i915_gem_batch_pool.c 
>>i915_gem_batch_pool.h i915_gem_fence.c 
>>i915_gem_gtt.h i915_gem_render_state.c 
>>i915_gem_render_state.h i915_gem_userptr.c 
>>i915_guc_reg.h i915_guc_submission.c 
>>i915_params.c i915_vgpu.c i915_vgpu.h 
>>intel_atomic.c intel_atomic_plane.c 
>>intel_audio.c intel_csr.c intel_dp_mst.c 
>>intel_dsi.c intel_dsi.h 
>>intel_dsi_panel_vbt.c intel_dsi_pll.c 
>>intel_fbc.c intel_fifo_underrun.c 
>>intel_frontbuffer.c intel_gtt.c 
>>intel_guc.h intel_guc_fwif.h 
>>intel_guc_loader.c intel_hotplug.c 
>>intel_lrc.c intel_lrc.h intel_mocs.c 
>>intel_mocs.h intel_psr.c 
>>intel_renderstate.h 
>>intel_renderstate_gen6.c 
>>intel_renderstate_gen7.c 
>>intel_renderstate_gen8.c 
>>intel_renderstate_gen9.c 
>>intel_runtime_pm.c 
>>  sys/dev/pci/drm/radeon: radeon_dp_auxch.c 
>>
>> Log message:
>> Update inteldrm(4) to code based on Linux 4.4.70.  This brings us support for
>> Skylake and Cherryview and better support for Broadwell and Valleyview.  Also
>> adds MST support.  Some tweaks to the TTM code and radeondrm(4) to keep it
>> working with the updated generic DRM code needed for inteldrm(4).
>>
>> Tested by many.
>>
> This change *STILL* breaks my $DAYJOB machine.
> 
After a couple of updates where I had to revert back to an old kernel I
managed to lay claim to a "new" system. This one is intel only and
doesn't have any issues.

If there's still interest in fixing this issue, the hardware is still
available, but there's no direct need for me any more.

OpenBSD 6.1-current (GENERIC.MP) #95: Wed Jul 12 19:23:28 MDT 2017
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
RTC BIOS 

Re: arm/sysreg.h use in C

2017-07-13 Thread aa e30
13.7.2017 17.50 "Mark Kettenis"  kirjoitti:

> Date: Thu, 13 Jul 2017 15:49:03 +0300
> From: Artturi Alm 
>
> On Sat, Jul 01, 2017 at 10:53:14AM +0300, Artturi Alm wrote:
> > Hi,
> >
> > just in case i didn't make it clear what it is for, here's diff "fixing"
> > current uses below, compile-tested.
> >
> > -Artturi
> >
>
> Hi,
>
> ping?
> Noone up for bikeshedding, or seen useless/worse than handcrafting?
> I think this would alleviate from some of the complementary commenting,
> regarding the CP15 reg usage, that is currently somewhat of necessity.

I'm not sure myself if doing something like this is actually an
improvement.


Ok, i'll try to get some fbsd dev to
comment why they never went for
this, just for my own curiousity,
so not pushing the diff any further.

-Artturi


> > diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
> > index c91108e7066..fcb56627af7 100644
> > --- a/sys/arch/arm/arm/cpufunc.c
> > +++ b/sys/arch/arm/arm/cpufunc.c
> > @@ -55,6 +55,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #if defined(PERFCTRS)
> >  struct arm_pmc_funcs *arm_pmc;
> > @@ -176,8 +177,7 @@ arm_get_cachetype_cp15v7(void)
> > uint32_t sel, level;
> >
> > /* CTR - Cache Type Register */
> > -   __asm volatile("mrc p15, 0, %0, c0, c0, 1"
> > -   : "=r" (ctype));
> > +   __asm volatile("mrc " SR_STR(CP15_CTR(%0)) "\n" : "=r"(ctype));
> >
> > arm_dcache_min_line_size = 1 << (CPU_CT_DMINLINE(ctype) + 2);
> > arm_icache_min_line_size = 1 << (CPU_CT_IMINLINE(ctype) + 2);
> > @@ -185,8 +185,8 @@ arm_get_cachetype_cp15v7(void)
> > min(arm_icache_min_line_size, arm_dcache_min_line_size);
> >
> > /* CLIDR - Cache Level ID Register */
> > -   __asm volatile("mrc p15, 1, %0, c0, c0, 1"
> > -   : "=r" (cache_level_id) :);
> > +   __asm volatile("mrc " SR_STR(CP15_CLIDR(%0))
> > +   : "=r"(cache_level_id));
> > cpu_drain_writebuf();
> >
> > /* L1 Cache available. */
> > @@ -201,17 +201,18 @@ arm_get_cachetype_cp15v7(void)
> > cache_level_id & (0x2 << level)) {
> > sel = level << 1 | 0 << 0; /* L1 | unified/data
cache */
> > /* CSSELR - Cache Size Selection Register */
> > -   __asm volatile("mcr p15, 2, %0, c0, c0, 0"
> > -   :: "r" (sel));
> > +   __asm volatile("mcr " SR_STR(CP15_CSSELR(%0))
"\n"
> > +   :: "r"(sel));
> > cpu_drain_writebuf();
> > /* CCSIDR - Cache Size Identification Register */
> > -   __asm volatile("mrc p15, 1, %0, c0, c0, 0"
> > -   : "=r" (cachereg) :);
> > +   __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0))
"\n"
> > +   : "=r"(cachereg));
> > cpu_drain_writebuf();
> > sets = ((cachereg >> 13) & 0x7fff) + 1;
> > arm_pdcache_line_size = 1 << ((cachereg & 0x7) + 4);
> > arm_pdcache_ways = ((cachereg >> 3) & 0x3ff) + 1;
> > -   arm_pdcache_size = arm_pdcache_line_size *
arm_pdcache_ways * sets;
> > +   arm_pdcache_size =
> > +   arm_pdcache_line_size * arm_pdcache_ways * sets;
> > switch (cachereg & 0xc000) {
> > case 0x:
> > arm_pcache_type = 0;
> > @@ -230,24 +231,26 @@ arm_get_cachetype_cp15v7(void)
> > if (cache_level_id & (0x1 << level)) {
> > sel = level << 1 | 1 << 0; /* L1 | instruction
cache */
> > /* CSSELR - Cache Size Selection Register */
> > -   __asm volatile("mcr p15, 2, %0, c0, c0, 0"
> > -   :: "r" (sel));
> > +   __asm volatile("mcr " SR_STR(CP15_CSSELR(%0))
"\n"
> > +   :: "r"(sel));
> > cpu_drain_writebuf();
> > /* CCSIDR - Cache Size Identification Register */
> > -   __asm volatile("mrc p15, 1, %0, c0, c0, 0"
> > -   : "=r" (cachereg) :);
> > +   __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0))
"\n"
> > +   : "=r"(cachereg));
> > cpu_drain_writebuf();
> > sets = ((cachereg >> 13) & 0x7fff) + 1;
> > arm_picache_line_size = 1 << ((cachereg & 0x7) + 4);
> > arm_picache_ways = ((cachereg >> 3) & 0x3ff) + 1;
> > -   arm_picache_size = arm_picache_line_size *
arm_picache_ways * sets;
> > +   arm_picache_size =
> > +   arm_picache_line_size * arm_picache_ways * sets;
> > }
> > }
> >
> > arm_dcache_align = arm_pdcache_line_size;
> > arm_dcache_align_mask = arm_dcache_align - 1;
> >
> > - 

remove net.inet6.ip6.maxifdefrouters and net.inet6.ip6.maxifprefixes sysctls

2017-07-13 Thread Florian Obser
pointed out by jmc, these sysctls are useless now, too, the kernel no
longer tracks prefixes or default routers from router advertisements.
OK?

diff --git lib/libc/gen/sysctl.3 lib/libc/gen/sysctl.3
index 0ac92a5c079..e6d3d092b2f 100644
--- lib/libc/gen/sysctl.3
+++ lib/libc/gen/sysctl.3
@@ -1654,8 +1654,6 @@ The currently defined protocols and names are:
 .It ip6 Ta maxdynroutes Ta integer Ta yes
 .It ip6 Ta maxfragpackets Ta integer Ta yes
 .It ip6 Ta maxfrags Ta integer Ta yes
-.It ip6 Ta maxifprefixes Ta integer Ta yes
-.It ip6 Ta maxifdefrouters Ta integer Ta yes
 .It ip6 Ta mforwarding Ta integer Ta yes
 .It ip6 Ta mtudisctimeout Ta integer Ta yes
 .It ip6 Ta multicast_mtudisc Ta integer Ta yes
@@ -1814,17 +1812,6 @@ The maximum number of fragments the node will accept.
 \-1 means that the node will accept as many fragments as it receives.
 The flag is provided basically for avoiding possible DoS attacks.
 .Pp
-.It Li ip6.maxifprefixes Pq Va net.inet6.ip6.maxifprefixes
-Maximum number of prefixes created by route advertisements per interface.
-Set to negative to disable.
-The default value is 16.
-.Pp
-.It Li ip6.maxifdefrouters Pq Va net.inet6.ip6.maxifdefrouters
-Maximum number of default routers created by route advertisements per
-interface.
-Set to negative to disable.
-The default value is 16.
-.Pp
 .It Li ip6.mforwarding Pq Va net.inet6.ip6.mforwarding
 If set to 1, then multicast forwarding is enabled for the host.
 The default is 0.
diff --git sys/netinet6/in6.h sys/netinet6/in6.h
index 1aac6581916..2af7d987335 100644
--- sys/netinet6/in6.h
+++ sys/netinet6/in6.h
@@ -584,8 +584,6 @@ ifatoia6(struct ifaddr *ifa)
 #define IPV6CTL_MULTIPATH  43
 #define IPV6CTL_MCAST_PMTU 44  /* path MTU discovery for multicast */
 #define IPV6CTL_NEIGHBORGCTHRESH 45
-#define IPV6CTL_MAXIFPREFIXES  46
-#define IPV6CTL_MAXIFDEFROUTERS 47
 #define IPV6CTL_MAXDYNROUTES   48
 #define IPV6CTL_DAD_PENDING49
 #define IPV6CTL_MTUDISCTIMEOUT 50
@@ -644,8 +642,8 @@ ifatoia6(struct ifaddr *ifa)
{ "multipath", CTLTYPE_INT }, \
{ "multicast_mtudisc", CTLTYPE_INT }, \
{ "neighborgcthresh", CTLTYPE_INT }, \
-   { "maxifprefixes", CTLTYPE_INT }, \
-   { "maxifdefrouters", CTLTYPE_INT }, \
+   { 0, 0 }, \
+   { 0, 0 }, \
{ "maxdynroutes", CTLTYPE_INT }, \
{ "dad_pending", CTLTYPE_INT }, \
{ "mtudisctimeout", CTLTYPE_INT }, \
@@ -701,8 +699,8 @@ ifatoia6(struct ifaddr *ifa)
_multipath, \
_mcast_pmtu, \
_neighborgcthresh, \
-   _maxifprefixes, \
-   _maxifdefrouters, \
+   NULL, \
+   NULL, \
_maxdynroutes, \
NULL, \
NULL, \
diff --git sys/netinet6/in6_proto.c sys/netinet6/in6_proto.c
index e2294ed1cb4..2353571afd6 100644
--- sys/netinet6/in6_proto.c
+++ sys/netinet6/in6_proto.c
@@ -360,8 +360,6 @@ int ip6_auto_flowlabel = 1;
 intip6_use_deprecated = 1; /* allow deprecated addr (RFC2462 5.5.4) */
 intip6_mcast_pmtu = 0; /* enable pMTU discovery for multicast? */
 intip6_neighborgcthresh = 2048; /* Threshold # of NDP entries for GC */
-intip6_maxifprefixes = 16; /* Max acceptable prefixes via RA per IF */
-intip6_maxifdefrouters = 16; /* Max acceptable def routers via RA */
 intip6_maxdynroutes = 4096; /* Max # of routes created via redirect */
 time_t ip6_log_time = (time_t)0L;
 
diff --git sys/netinet6/ip6_var.h sys/netinet6/ip6_var.h
index 9b58d9ee0af..45225f272fa 100644
--- sys/netinet6/ip6_var.h
+++ sys/netinet6/ip6_var.h
@@ -279,8 +279,6 @@ extern int  ip6_sendredirect;   /* send ICMPv6 
redirect? */
 extern int ip6_use_deprecated; /* allow deprecated addr as source */
 extern int ip6_mcast_pmtu; /* path MTU discovery for multicast */
 extern int ip6_neighborgcthresh; /* Threshold # of NDP entries for GC */
-extern int ip6_maxifprefixes; /* Max acceptable prefixes via RA per IF */
-extern int ip6_maxifdefrouters; /* Max acceptable def routers via RA */
 extern int ip6_maxdynroutes; /* Max # of routes created via redirect */
 
 extern struct socket *ip6_mrouter[RT_TABLEID_MAX]; /* multicast routing daemon 
*/

-- 
I'm not entirely sure you are real.



Re: signal info code SEGV_ACCERR

2017-07-13 Thread Mark Kettenis
> Date: Thu, 13 Jul 2017 14:54:41 +0200
> From: Alexander Bluhm 
> 
> Hi,
> 
> The regress test src/regress/sys/kern/siginfo-fault checks wether
> the si_code is set to SEGV_ACCERR after memory access with wrong
> permissions has triggert a SIGSEGV.
> 
> Relevant commit message of the test is:
> According to POSIX, SIGSEGV should specify SEGV_ACCERR if the memory
> pages are mapped, but the protections don't match the user's access
> attempts, while SEGV_MAPERR should only be specified for pages that
> are unmapped.  Some platforms currently handle this correctly, but not
> all.
> 
> This diff changes the behavior of i366 and amd64.  hppa seems to
> do something similar.  Changing error form EACCES to EFAULT in amd64
> affects only a printf, so I removed the useless code.  Also change
> rv to errno in i386 to make it look like amd64.
> 
> ok?

I like this.  ok kettenis@

> Index: arch/amd64/amd64/trap.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 trap.c
> --- arch/amd64/amd64/trap.c   30 Apr 2017 13:04:49 -  1.54
> +++ arch/amd64/amd64/trap.c   13 Jul 2017 12:31:28 -
> @@ -362,9 +362,6 @@ faultcommon:
>   KERNEL_UNLOCK();
>   goto out;
>   }
> - if (error == EACCES) {
> - error = EFAULT;
> - }
>  
>   if (type == T_PAGEFLT) {
>   if (pcb->pcb_onfault != 0) {
> @@ -389,7 +386,8 @@ faultcommon:
>   frame_dump(frame);
>  #endif
>   sv.sival_ptr = (void *)fa;
> - trapsignal(p, SIGSEGV, T_PAGEFLT, SEGV_MAPERR, sv);
> + trapsignal(p, SIGSEGV, T_PAGEFLT,
> + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
>   }
>   KERNEL_UNLOCK();
>   break;
> Index: arch/i386/i386/trap.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/trap.c,v
> retrieving revision 1.130
> diff -u -p -r1.130 trap.c
> --- arch/i386/i386/trap.c 30 Apr 2017 13:04:49 -  1.130
> +++ arch/i386/i386/trap.c 13 Jul 2017 12:33:30 -
> @@ -373,7 +373,7 @@ trap(struct trapframe *frame)
>   vaddr_t va, fa;
>   struct vmspace *vm;
>   struct vm_map *map;
> - int rv;
> + int error;
>  
>   cr2 = rcr2();
>   KERNEL_LOCK();
> @@ -406,12 +406,12 @@ trap(struct trapframe *frame)
>   if (curcpu()->ci_inatomic == 0 || map == kernel_map) {
>   onfault = p->p_addr->u_pcb.pcb_onfault;
>   p->p_addr->u_pcb.pcb_onfault = NULL;
> - rv = uvm_fault(map, va, 0, ftype);
> + error = uvm_fault(map, va, 0, ftype);
>   p->p_addr->u_pcb.pcb_onfault = onfault;
>   } else
> - rv = EFAULT;
> + error = EFAULT;
>  
> - if (rv == 0) {
> + if (error == 0) {
>   if (map != kernel_map)
>   uvm_grow(p, va);
>   if (type == T_PAGEFLT) {
> @@ -428,11 +428,12 @@ trap(struct trapframe *frame)
>   goto copyfault;
>   }
>   printf("uvm_fault(%p, 0x%lx, 0, %d) -> %x\n",
> - map, va, ftype, rv);
> + map, va, ftype, error);
>   goto we_re_toast;
>   }
>   sv.sival_int = fa;
> - trapsignal(p, SIGSEGV, vftype, SEGV_MAPERR, sv);
> + trapsignal(p, SIGSEGV, vftype,
> + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
>   KERNEL_UNLOCK();
>   break;
>   }
> 
> 



Re: arm/sysreg.h use in C

2017-07-13 Thread Mark Kettenis
> Date: Thu, 13 Jul 2017 15:49:03 +0300
> From: Artturi Alm 
> 
> On Sat, Jul 01, 2017 at 10:53:14AM +0300, Artturi Alm wrote:
> > Hi,
> > 
> > just in case i didn't make it clear what it is for, here's diff "fixing"
> > current uses below, compile-tested.
> > 
> > -Artturi
> > 
> 
> Hi,
> 
> ping?
> Noone up for bikeshedding, or seen useless/worse than handcrafting?
> I think this would alleviate from some of the complementary commenting,
> regarding the CP15 reg usage, that is currently somewhat of necessity.

I'm not sure myself if doing something like this is actually an
improvement.

> > diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
> > index c91108e7066..fcb56627af7 100644
> > --- a/sys/arch/arm/arm/cpufunc.c
> > +++ b/sys/arch/arm/arm/cpufunc.c
> > @@ -55,6 +55,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #if defined(PERFCTRS)
> >  struct arm_pmc_funcs *arm_pmc;
> > @@ -176,8 +177,7 @@ arm_get_cachetype_cp15v7(void)
> > uint32_t sel, level;
> >  
> > /* CTR - Cache Type Register */
> > -   __asm volatile("mrc p15, 0, %0, c0, c0, 1"
> > -   : "=r" (ctype));
> > +   __asm volatile("mrc " SR_STR(CP15_CTR(%0)) "\n" : "=r"(ctype));
> >  
> > arm_dcache_min_line_size = 1 << (CPU_CT_DMINLINE(ctype) + 2);
> > arm_icache_min_line_size = 1 << (CPU_CT_IMINLINE(ctype) + 2);
> > @@ -185,8 +185,8 @@ arm_get_cachetype_cp15v7(void)
> > min(arm_icache_min_line_size, arm_dcache_min_line_size);
> >  
> > /* CLIDR - Cache Level ID Register */
> > -   __asm volatile("mrc p15, 1, %0, c0, c0, 1"
> > -   : "=r" (cache_level_id) :);
> > +   __asm volatile("mrc " SR_STR(CP15_CLIDR(%0))
> > +   : "=r"(cache_level_id));
> > cpu_drain_writebuf();
> >  
> > /* L1 Cache available. */
> > @@ -201,17 +201,18 @@ arm_get_cachetype_cp15v7(void)
> > cache_level_id & (0x2 << level)) {
> > sel = level << 1 | 0 << 0; /* L1 | unified/data cache */
> > /* CSSELR - Cache Size Selection Register */
> > -   __asm volatile("mcr p15, 2, %0, c0, c0, 0"
> > -   :: "r" (sel));
> > +   __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n"
> > +   :: "r"(sel));
> > cpu_drain_writebuf();
> > /* CCSIDR - Cache Size Identification Register */
> > -   __asm volatile("mrc p15, 1, %0, c0, c0, 0"
> > -   : "=r" (cachereg) :);
> > +   __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n"
> > +   : "=r"(cachereg));
> > cpu_drain_writebuf();
> > sets = ((cachereg >> 13) & 0x7fff) + 1;
> > arm_pdcache_line_size = 1 << ((cachereg & 0x7) + 4);
> > arm_pdcache_ways = ((cachereg >> 3) & 0x3ff) + 1;
> > -   arm_pdcache_size = arm_pdcache_line_size * 
> > arm_pdcache_ways * sets;
> > +   arm_pdcache_size =
> > +   arm_pdcache_line_size * arm_pdcache_ways * sets;
> > switch (cachereg & 0xc000) {
> > case 0x:
> > arm_pcache_type = 0;
> > @@ -230,24 +231,26 @@ arm_get_cachetype_cp15v7(void)
> > if (cache_level_id & (0x1 << level)) {
> > sel = level << 1 | 1 << 0; /* L1 | instruction cache */
> > /* CSSELR - Cache Size Selection Register */
> > -   __asm volatile("mcr p15, 2, %0, c0, c0, 0"
> > -   :: "r" (sel));
> > +   __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n"
> > +   :: "r"(sel));
> > cpu_drain_writebuf();
> > /* CCSIDR - Cache Size Identification Register */
> > -   __asm volatile("mrc p15, 1, %0, c0, c0, 0"
> > -   : "=r" (cachereg) :);
> > +   __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n"
> > +   : "=r"(cachereg));
> > cpu_drain_writebuf();
> > sets = ((cachereg >> 13) & 0x7fff) + 1;
> > arm_picache_line_size = 1 << ((cachereg & 0x7) + 4);
> > arm_picache_ways = ((cachereg >> 3) & 0x3ff) + 1;
> > -   arm_picache_size = arm_picache_line_size * 
> > arm_picache_ways * sets;
> > +   arm_picache_size =
> > +   arm_picache_line_size * arm_picache_ways * sets;
> > }
> > }
> >  
> > arm_dcache_align = arm_pdcache_line_size;
> > arm_dcache_align_mask = arm_dcache_align - 1;
> >  
> > -   arm_dcache_l2_nsets = 
> > arm_pdcache_size/arm_pdcache_ways/arm_pdcache_line_size;
> > +   arm_dcache_l2_nsets =
> > +   arm_pdcache_size / arm_pdcache_ways / arm_pdcache_line_size;
> > 

signal info code SEGV_ACCERR

2017-07-13 Thread Alexander Bluhm
Hi,

The regress test src/regress/sys/kern/siginfo-fault checks wether
the si_code is set to SEGV_ACCERR after memory access with wrong
permissions has triggert a SIGSEGV.

Relevant commit message of the test is:
According to POSIX, SIGSEGV should specify SEGV_ACCERR if the memory
pages are mapped, but the protections don't match the user's access
attempts, while SEGV_MAPERR should only be specified for pages that
are unmapped.  Some platforms currently handle this correctly, but not
all.

This diff changes the behavior of i366 and amd64.  hppa seems to
do something similar.  Changing error form EACCES to EFAULT in amd64
affects only a printf, so I removed the useless code.  Also change
rv to errno in i386 to make it look like amd64.

ok?

bluhm

Index: arch/amd64/amd64/trap.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v
retrieving revision 1.54
diff -u -p -r1.54 trap.c
--- arch/amd64/amd64/trap.c 30 Apr 2017 13:04:49 -  1.54
+++ arch/amd64/amd64/trap.c 13 Jul 2017 12:31:28 -
@@ -362,9 +362,6 @@ faultcommon:
KERNEL_UNLOCK();
goto out;
}
-   if (error == EACCES) {
-   error = EFAULT;
-   }
 
if (type == T_PAGEFLT) {
if (pcb->pcb_onfault != 0) {
@@ -389,7 +386,8 @@ faultcommon:
frame_dump(frame);
 #endif
sv.sival_ptr = (void *)fa;
-   trapsignal(p, SIGSEGV, T_PAGEFLT, SEGV_MAPERR, sv);
+   trapsignal(p, SIGSEGV, T_PAGEFLT,
+   error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
}
KERNEL_UNLOCK();
break;
Index: arch/i386/i386/trap.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/trap.c,v
retrieving revision 1.130
diff -u -p -r1.130 trap.c
--- arch/i386/i386/trap.c   30 Apr 2017 13:04:49 -  1.130
+++ arch/i386/i386/trap.c   13 Jul 2017 12:33:30 -
@@ -373,7 +373,7 @@ trap(struct trapframe *frame)
vaddr_t va, fa;
struct vmspace *vm;
struct vm_map *map;
-   int rv;
+   int error;
 
cr2 = rcr2();
KERNEL_LOCK();
@@ -406,12 +406,12 @@ trap(struct trapframe *frame)
if (curcpu()->ci_inatomic == 0 || map == kernel_map) {
onfault = p->p_addr->u_pcb.pcb_onfault;
p->p_addr->u_pcb.pcb_onfault = NULL;
-   rv = uvm_fault(map, va, 0, ftype);
+   error = uvm_fault(map, va, 0, ftype);
p->p_addr->u_pcb.pcb_onfault = onfault;
} else
-   rv = EFAULT;
+   error = EFAULT;
 
-   if (rv == 0) {
+   if (error == 0) {
if (map != kernel_map)
uvm_grow(p, va);
if (type == T_PAGEFLT) {
@@ -428,11 +428,12 @@ trap(struct trapframe *frame)
goto copyfault;
}
printf("uvm_fault(%p, 0x%lx, 0, %d) -> %x\n",
-   map, va, ftype, rv);
+   map, va, ftype, error);
goto we_re_toast;
}
sv.sival_int = fa;
-   trapsignal(p, SIGSEGV, vftype, SEGV_MAPERR, sv);
+   trapsignal(p, SIGSEGV, vftype,
+   error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv);
KERNEL_UNLOCK();
break;
}



Re: arm/sysreg.h use in C

2017-07-13 Thread Artturi Alm
On Sat, Jul 01, 2017 at 10:53:14AM +0300, Artturi Alm wrote:
> Hi,
> 
> just in case i didn't make it clear what it is for, here's diff "fixing"
> current uses below, compile-tested.
> 
> -Artturi
> 

Hi,

ping?
Noone up for bikeshedding, or seen useless/worse than handcrafting?
I think this would alleviate from some of the complementary commenting,
regarding the CP15 reg usage, that is currently somewhat of necessity.

-Artturi

> 
> diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
> index c91108e7066..fcb56627af7 100644
> --- a/sys/arch/arm/arm/cpufunc.c
> +++ b/sys/arch/arm/arm/cpufunc.c
> @@ -55,6 +55,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #if defined(PERFCTRS)
>  struct arm_pmc_funcs *arm_pmc;
> @@ -176,8 +177,7 @@ arm_get_cachetype_cp15v7(void)
>   uint32_t sel, level;
>  
>   /* CTR - Cache Type Register */
> - __asm volatile("mrc p15, 0, %0, c0, c0, 1"
> - : "=r" (ctype));
> + __asm volatile("mrc " SR_STR(CP15_CTR(%0)) "\n" : "=r"(ctype));
>  
>   arm_dcache_min_line_size = 1 << (CPU_CT_DMINLINE(ctype) + 2);
>   arm_icache_min_line_size = 1 << (CPU_CT_IMINLINE(ctype) + 2);
> @@ -185,8 +185,8 @@ arm_get_cachetype_cp15v7(void)
>   min(arm_icache_min_line_size, arm_dcache_min_line_size);
>  
>   /* CLIDR - Cache Level ID Register */
> - __asm volatile("mrc p15, 1, %0, c0, c0, 1"
> - : "=r" (cache_level_id) :);
> + __asm volatile("mrc " SR_STR(CP15_CLIDR(%0))
> + : "=r"(cache_level_id));
>   cpu_drain_writebuf();
>  
>   /* L1 Cache available. */
> @@ -201,17 +201,18 @@ arm_get_cachetype_cp15v7(void)
>   cache_level_id & (0x2 << level)) {
>   sel = level << 1 | 0 << 0; /* L1 | unified/data cache */
>   /* CSSELR - Cache Size Selection Register */
> - __asm volatile("mcr p15, 2, %0, c0, c0, 0"
> - :: "r" (sel));
> + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n"
> + :: "r"(sel));
>   cpu_drain_writebuf();
>   /* CCSIDR - Cache Size Identification Register */
> - __asm volatile("mrc p15, 1, %0, c0, c0, 0"
> - : "=r" (cachereg) :);
> + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n"
> + : "=r"(cachereg));
>   cpu_drain_writebuf();
>   sets = ((cachereg >> 13) & 0x7fff) + 1;
>   arm_pdcache_line_size = 1 << ((cachereg & 0x7) + 4);
>   arm_pdcache_ways = ((cachereg >> 3) & 0x3ff) + 1;
> - arm_pdcache_size = arm_pdcache_line_size * 
> arm_pdcache_ways * sets;
> + arm_pdcache_size =
> + arm_pdcache_line_size * arm_pdcache_ways * sets;
>   switch (cachereg & 0xc000) {
>   case 0x:
>   arm_pcache_type = 0;
> @@ -230,24 +231,26 @@ arm_get_cachetype_cp15v7(void)
>   if (cache_level_id & (0x1 << level)) {
>   sel = level << 1 | 1 << 0; /* L1 | instruction cache */
>   /* CSSELR - Cache Size Selection Register */
> - __asm volatile("mcr p15, 2, %0, c0, c0, 0"
> - :: "r" (sel));
> + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n"
> + :: "r"(sel));
>   cpu_drain_writebuf();
>   /* CCSIDR - Cache Size Identification Register */
> - __asm volatile("mrc p15, 1, %0, c0, c0, 0"
> - : "=r" (cachereg) :);
> + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n"
> + : "=r"(cachereg));
>   cpu_drain_writebuf();
>   sets = ((cachereg >> 13) & 0x7fff) + 1;
>   arm_picache_line_size = 1 << ((cachereg & 0x7) + 4);
>   arm_picache_ways = ((cachereg >> 3) & 0x3ff) + 1;
> - arm_picache_size = arm_picache_line_size * 
> arm_picache_ways * sets;
> + arm_picache_size =
> + arm_picache_line_size * arm_picache_ways * sets;
>   }
>   }
>  
>   arm_dcache_align = arm_pdcache_line_size;
>   arm_dcache_align_mask = arm_dcache_align - 1;
>  
> - arm_dcache_l2_nsets = 
> arm_pdcache_size/arm_pdcache_ways/arm_pdcache_line_size;
> + arm_dcache_l2_nsets =
> + arm_pdcache_size / arm_pdcache_ways / arm_pdcache_line_size;
>   arm_dcache_l2_assoc = log2(arm_pdcache_ways);
>   arm_dcache_l2_linesize = log2(arm_pdcache_line_size);
>  }
> @@ -255,17 +258,16 @@ arm_get_cachetype_cp15v7(void)
>  /* 
>   */
>  void
> -armv7_idcache_wbinv_all()
> 

Re: armv7 small XXX fix

2017-07-13 Thread Artturi Alm
On Wed, Jul 12, 2017 at 04:21:11PM -0400, Dale Rahn wrote:
> 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

Oh, that explains what i was missing about 'why?', thank you for the info/
details.
At this point, given how harmless it is, i think cleaning up all at once
would be just churn, so i will consider it deprecated myself, and go w/o
it in my future diffs, which would touch any of them for other reasons.

-Artturi



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

2017-07-13 Thread Mike Belopuhov
On Thu, Jul 13, 2017 at 13:44 +1000, David Gwynne wrote:
> 
> > 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.
> 

Same here. I'm surprised to learn time(1) is not using CLOCK_MONOTONIC.



Re: urndis issues

2017-07-13 Thread Mike Belopuhov
On Wed, Jul 12, 2017 at 21:04 +, Jonathan Armani wrote:
> Hi,
> 
> Thanks I was cooking the same diff.
> 
> Ok armani@
> 

Hi, thanks! I want to get rid of printfs though and
return errors (or unhandled status codes) so that we
don't paper over them. In theory, all error codes that
I've seen in the ndis.h from the DDK have the MSB set,
i.e. (rval & 0x8000) != 0 for those, but doing it
would be a bit too hackish IMO. However, if we go down
this road the rval check can be rewritten as:

/* Not an error */
if ((rval & 0x8000) == 0)
rval = RNDIS_STATUS_SUCCESS;
else
printf("%s: status 0x%x\n", DEVNAME(sc), rval);

Is this something we want to do?

I was also going to add sc_link toggled by those status
codes and check it in the urndis_start like other USB
Ethernet drivers do, but decided against it cause this
would break other devices that don't send this message.


diff --git sys/dev/usb/if_urndis.c sys/dev/usb/if_urndis.c
index 4af6b55cf05..956207f73a8 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,42 @@ 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:
+   case RNDIS_STATUS_MEDIA_DISCONNECT:
+   case RNDIS_STATUS_OFFLOAD_CURRENT_CONFIG:
+   rval = RNDIS_STATUS_SUCCESS;
+   break;
+
+   default:
+   printf("%s: status 0x%x\n", DEVNAME(sc), rval);
+   }
+
+   return rval;
+}
+
+u_int32_t
 urndis_ctrl_init(struct urndis_softc *sc)
 {
struct rndis_init_req   *msg;
u_int32_trval;
struct rndis_comp_hdr   *hdr;