Re: Towards tcp_input() w/o KERNEL_LOCK()

[Alexander Bluhm]

On Tue, Jun 06, 2017 at 05:15:40PM +0200, Martin Pieuchot wrote:
> TCP/UDP are almost ready to run without KERNEL_LOCK() because accesses
> to their sockets are serialized via the NET_LOCK().  On the other hand
> pfkey and routing sockets accesses still rely on the KERNEL_LOCK().
> 
> Since we're going to work at the socket layer, first to remove the
> KERNEL_LOCK() from routing/pfkey sockets then to split the NET_LOCK(),
> we need some tooling to move faster and avoid mistakes.
> 
> Currently all operations on socket buffers are protected by these
> locks.  I'd like to assert that, at least for all functions used in
> TCP/UDP layers.
> 
> The idea is to later change the lock asserted in soassertlocked(). 
> 
> Comments, ok?

Good idea, mostly OK.

> @@ -1058,7 +1058,9 @@ sorflush(struct socket *so)
>   }
>   if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose)
>   (*pr->pr_domain->dom_dispose)(asb.sb_mb);
> - sbrelease();
> + *sb = asb;
> + sbrelease(so, sb);
> + memset(sb, 0, sizeof (*sb));
>  }

A few lines above we have
/* XXX - the memset stomps all over so_rcv */
if (asb.sb_flags & SB_KNOTE) {
sb->sb_sel.si_note = asb.sb_sel.si_note;
sb->sb_flags = SB_KNOTE;
}
I think this has to be redone after your memset() as the socket is
still used after a soshutdown().

> @@ -1270,7 +1272,7 @@ somove(struct socket *so, int wait)
>   maxreached = 1;
>   }
>   }
> - space = sbspace(>so_snd);
> + space = sbspace(so, >so_snd);
>   if (so->so_oobmark && so->so_oobmark < len &&
>   so->so_oobmark < space + 1024)
>   space += 1024;

This must be sosp.
+   space = sbspace(sosp, >so_snd);

> -sbappendstream(struct sockbuf *sb, struct mbuf *m)
> +sbappendstream(struct socket *so, struct sockbuf *sb, struct mbuf *m)
> -sbappendaddr(struct sockbuf *sb, struct sockaddr *asa, struct mbuf *m0,
> +sbappendaddr(struct socket *so, struct sockbuf *sb, struct sockaddr *asa,
> -sbappendcontrol(struct sockbuf *sb, struct mbuf *m0, struct mbuf *control)
> +sbappendcontrol(struct socket *so, struct sockbuf *sb, struct mbuf *m0,

Should we also change sbappendrecord() and sbappend() ?

>  /* can we write something to so? */
>  #define  sowriteable(so) \
> -((sbspace(&(so)->so_snd) >= (so)->so_snd.sb_lowat && \
> +((sbspace(so, &(so)->so_snd) >= (so)->so_snd.sb_lowat && \
>   (((so)->so_state & SS_ISCONNECTED) || \
> ((so)->so_proto->pr_flags & PR_CONNREQUIRED)==0)) || \
>  ((so)->so_state & SS_CANTSENDMORE) || (so)->so_error)

You need () around marco arguments.
+   ((sbspace((so), &(so)->so_snd) >= (so)->so_snd.sb_lowat && \

bluhm

Re: tcpdump: drop atalk support

[Claudio Jeker]

On Thu, Jun 08, 2017 at 09:42:44PM +0200, Michal Mazurek wrote:
> Let's start by ignoring the existence of AppleTalk in the manpage,
> reducing it by 10%. This leaves mention of atalk in the syntax of libpcap.
> 
> A second diff will remove /etc/atalk.names support reducing the amount
> of appletalk code significantly.
> 
> Comments? OK?

OK claudio@
 
> Index: usr.sbin/tcpdump/tcpdump.8
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
> retrieving revision 1.92
> diff -u -p -r1.92 tcpdump.8
> --- usr.sbin/tcpdump/tcpdump.819 Apr 2017 05:36:13 -  1.92
> +++ usr.sbin/tcpdump/tcpdump.88 Jun 2017 19:36:14 -
> @@ -1604,142 +1604,6 @@ requests, and matches them to the replie
>  .Pq transaction ID .
>  If a reply does not closely follow the corresponding request,
>  it might not be parsable.
> -.Ss KIP AppleTalk (DDP in UDP)
> -AppleTalk DDP packets encapsulated in UDP datagrams
> -are de-encapsulated and dumped as DDP packets
> -.Pq i.e., all the UDP header information is discarded .
> -The file
> -.Pa /etc/atalk.names
> -is used to translate AppleTalk net and node numbers to names.
> -Lines in this file have the form
> -.Bl -column "number" "name" -offset indent
> -.It Sy "number" Ta Ta Sy "name"
> -.It "1.254" Ta Ta "ether"
> -.It "16.1" Ta Ta "icsd-net"
> -.It "1.254.110" Ta Ta "ace"
> -.El
> -.Pp
> -The first two lines give the names of AppleTalk networks.
> -The third line gives the name of a particular host
> -(a host is distinguished from a net by the 3rd octet in the number;
> -a net number
> -.Em must
> -have two octets and a host number
> -.Em must
> -have three octets).
> -The number and name should be separated by whitespace (blanks or tabs).
> -The
> -.Pa /etc/atalk.names
> -file may contain blank lines or comment lines
> -(lines starting with a
> -.Ql # ) .
> -.Pp
> -AppleTalk addresses are printed in the form
> -.Pp
> -.D1 Ar net . Ns Ar host . Ns Ar port
> -.Pp
> -For example:
> -.Bd -unfilled -offset indent
> -144.1.209.2 > icsd-net.112.220
> -office.2 > icsd-net.112.220
> -jssmag.149.235 > icsd-net.2
> -.Ed
> -.Pp
> -If
> -.Pa /etc/atalk.names
> -doesn't exist or doesn't contain an entry for some AppleTalk
> -host/net number, addresses are printed in numeric form.
> -In the first example, NBP
> -.Pq DDP port 2
> -on net 144.1 node 209
> -is sending to whatever is listening on port 220 of net icsd-net node 112.
> -The second line is the same except the full name of the source node is known
> -.Pq Dq office .
> -The third line is a send from port 235 on
> -net jssmag node 149 to broadcast on the icsd-net NBP port.
> -The broadcast address
> -.Pq 255
> -is indicated by a net name with no host number;
> -for this reason it is a good idea to keep node names and net names distinct 
> in
> -.Pa /etc/atalk.names .
> -.Pp
> -NBP
> -.Pq name binding protocol
> -and ATP
> -.Pq AppleTalk transaction protocol
> -packets have their contents interpreted.
> -Other protocols just dump the protocol name
> -.Po
> -or number if no name is registered for the protocol
> -.Pc
> -and packet size.
> -.Pp
> -NBP packets are formatted like the following examples:
> -.Bd -unfilled
> -icsd-net.112.220 > jssmag.2: nbp-lkup 190: "=:LaserWriter@*"
> -jssmag.209.2 > icsd-net.112.220: nbp-reply 190: "RM1140:LaserWriter@*" 250
> -techpit.2 > icsd-net.112.220: nbp-reply 190: "techpit:LaserWriter@*" 186
> -.Ed
> -.Pp
> -The first line is a name lookup request for laserwriters sent by
> -net icsdi-net host
> -112 and broadcast on net jssmag.
> -The nbp ID for the lookup is 190.
> -The second line shows a reply for this request
> -.Pq note that it has the same ID
> -from host jssmag.209 saying that it has a laserwriter
> -resource named RM1140 registered on port 250.
> -The third line is another reply to the same request
> -saying host techpit has laserwriter techpit registered on port 186.
> -.Pp
> -ATP packet formatting is demonstrated by the following example:
> -.Bd -unfilled -offset indent
> -jssmag.209.165 > helios.132: atp-req  12266<0-7> 0xae030001
> -helios.132 > jssmag.209.165: atp-resp 12266:0 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:1 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:2 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:3 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:4 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:5 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:6 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp*12266:7 (512) 0xae04
> -jssmag.209.165 > helios.132: atp-req  12266<3,5> 0xae030001
> -helios.132 > jssmag.209.165: atp-resp 12266:3 (512) 0xae04
> -helios.132 > jssmag.209.165: atp-resp 12266:5 (512) 0xae04
> -jssmag.209.165 > helios.132: atp-rel  12266<0-7> 0xae030001
> -jssmag.209.133 > helios.132: atp-req* 12267<0-7> 0xae030002
> -.Ed
> -.Pp
> -Jssmag.209 initiates 

Re: radix lookup w/o KERNEL_LOCK()

[Alexander Bluhm]

On Tue, Jun 06, 2017 at 03:36:12PM +0200, Martin Pieuchot wrote:
> +#define SALEN(sa)(*(u_char *)sa)

Put () around macro arguments.
#define SALEN(sa)   (*(u_char *)(sa))

> -int
> +static int
>  rn_refines(void *m_arg, void *n_arg)

> -int
> +static int
>  rn_inithead0(struct radix_node_head *rnh, int offset)

Could you keep these kernel funtions global for debugging?

> @@ -414,8 +422,9 @@ rn_addmask(void *n_arg, int search, int 
>   static int last_zeroed = 0;
> + char addmask_key[KEYLEN_LIMIT];

The static variable last_zeroed does not look MP safe.  If I get
this code correctly it is only an optimization to avoid multiple
zeroing in addmask_key.  This does not work anyway with addmask_key
on the stack.

>   *addmask_key = last_zeroed = mlen;

Should this be SALEN(addmask_key)?

bluhm

monop(6): correct number of players in the man page

[Frederic Cambus]

Hi tech@,

The man page says monop monitors a game between 1 to 9 users, but the
program enforces a range from 2 to 9.

Comments? OK?

Index: games/monop/monop.6
===
RCS file: /cvs/src/games/monop/monop.6,v
retrieving revision 1.15
diff -u -p -r1.15 monop.6
--- games/monop/monop.6 13 Mar 2015 19:58:40 -  1.15
+++ games/monop/monop.6 8 Jun 2017 21:15:46 -
@@ -29,7 +29,7 @@
 .\"
 .\"@(#)monop.6 6.5 (Berkeley) 3/25/93
 .\"
-.Dd $Mdocdate: March 13 2015 $
+.Dd $Mdocdate: June 8 2017 $
 .Dt MONOP 6
 .Os
 .Sh NAME
@@ -41,7 +41,7 @@
 .Sh DESCRIPTION
 .Nm
 is reminiscent of the Parker Brother's game Monopoly, and
-monitors a game between 1 to 9 users.
+monitors a game between 2 to 9 users.
 It is assumed that the rules of Monopoly are known.
 The game follows the standard rules, with the exception that,
 if a property goes up for auction and there are only two solvent players,

Re: doas: add confirm to prompt the user on what is to be executed

[lists]

Thu, 8 Jun 2017 20:17:02 +0200 Adam Wolk 
> This email is a request for comment, roughly I want to know if others
> see this feature as valuable.

Hi Adam,

Simple shell wrapper functions can replicate the are you sure thing.
Only a reminder to focus on implementing just features that must be:

https://en.wikiquote.org/wiki/Tony_Hoare#The_Emperor.27s_Old_Clothes

Kind regards,
Anton Lazarov

Re: tcpdump: drop atalk support

[Michal Mazurek]

Let's start by ignoring the existence of AppleTalk in the manpage,
reducing it by 10%. This leaves mention of atalk in the syntax of libpcap.

A second diff will remove /etc/atalk.names support reducing the amount
of appletalk code significantly.

Comments? OK?

Index: usr.sbin/tcpdump/tcpdump.8
===
RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
retrieving revision 1.92
diff -u -p -r1.92 tcpdump.8
--- usr.sbin/tcpdump/tcpdump.8  19 Apr 2017 05:36:13 -  1.92
+++ usr.sbin/tcpdump/tcpdump.8  8 Jun 2017 19:36:14 -
@@ -1604,142 +1604,6 @@ requests, and matches them to the replie
 .Pq transaction ID .
 If a reply does not closely follow the corresponding request,
 it might not be parsable.
-.Ss KIP AppleTalk (DDP in UDP)
-AppleTalk DDP packets encapsulated in UDP datagrams
-are de-encapsulated and dumped as DDP packets
-.Pq i.e., all the UDP header information is discarded .
-The file
-.Pa /etc/atalk.names
-is used to translate AppleTalk net and node numbers to names.
-Lines in this file have the form
-.Bl -column "number" "name" -offset indent
-.It Sy "number" Ta Ta Sy "name"
-.It "1.254" Ta Ta "ether"
-.It "16.1" Ta Ta "icsd-net"
-.It "1.254.110" Ta Ta "ace"
-.El
-.Pp
-The first two lines give the names of AppleTalk networks.
-The third line gives the name of a particular host
-(a host is distinguished from a net by the 3rd octet in the number;
-a net number
-.Em must
-have two octets and a host number
-.Em must
-have three octets).
-The number and name should be separated by whitespace (blanks or tabs).
-The
-.Pa /etc/atalk.names
-file may contain blank lines or comment lines
-(lines starting with a
-.Ql # ) .
-.Pp
-AppleTalk addresses are printed in the form
-.Pp
-.D1 Ar net . Ns Ar host . Ns Ar port
-.Pp
-For example:
-.Bd -unfilled -offset indent
-144.1.209.2 > icsd-net.112.220
-office.2 > icsd-net.112.220
-jssmag.149.235 > icsd-net.2
-.Ed
-.Pp
-If
-.Pa /etc/atalk.names
-doesn't exist or doesn't contain an entry for some AppleTalk
-host/net number, addresses are printed in numeric form.
-In the first example, NBP
-.Pq DDP port 2
-on net 144.1 node 209
-is sending to whatever is listening on port 220 of net icsd-net node 112.
-The second line is the same except the full name of the source node is known
-.Pq Dq office .
-The third line is a send from port 235 on
-net jssmag node 149 to broadcast on the icsd-net NBP port.
-The broadcast address
-.Pq 255
-is indicated by a net name with no host number;
-for this reason it is a good idea to keep node names and net names distinct in
-.Pa /etc/atalk.names .
-.Pp
-NBP
-.Pq name binding protocol
-and ATP
-.Pq AppleTalk transaction protocol
-packets have their contents interpreted.
-Other protocols just dump the protocol name
-.Po
-or number if no name is registered for the protocol
-.Pc
-and packet size.
-.Pp
-NBP packets are formatted like the following examples:
-.Bd -unfilled
-icsd-net.112.220 > jssmag.2: nbp-lkup 190: "=:LaserWriter@*"
-jssmag.209.2 > icsd-net.112.220: nbp-reply 190: "RM1140:LaserWriter@*" 250
-techpit.2 > icsd-net.112.220: nbp-reply 190: "techpit:LaserWriter@*" 186
-.Ed
-.Pp
-The first line is a name lookup request for laserwriters sent by
-net icsdi-net host
-112 and broadcast on net jssmag.
-The nbp ID for the lookup is 190.
-The second line shows a reply for this request
-.Pq note that it has the same ID
-from host jssmag.209 saying that it has a laserwriter
-resource named RM1140 registered on port 250.
-The third line is another reply to the same request
-saying host techpit has laserwriter techpit registered on port 186.
-.Pp
-ATP packet formatting is demonstrated by the following example:
-.Bd -unfilled -offset indent
-jssmag.209.165 > helios.132: atp-req  12266<0-7> 0xae030001
-helios.132 > jssmag.209.165: atp-resp 12266:0 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:1 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:2 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:3 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:4 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:5 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:6 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp*12266:7 (512) 0xae04
-jssmag.209.165 > helios.132: atp-req  12266<3,5> 0xae030001
-helios.132 > jssmag.209.165: atp-resp 12266:3 (512) 0xae04
-helios.132 > jssmag.209.165: atp-resp 12266:5 (512) 0xae04
-jssmag.209.165 > helios.132: atp-rel  12266<0-7> 0xae030001
-jssmag.209.133 > helios.132: atp-req* 12267<0-7> 0xae030002
-.Ed
-.Pp
-Jssmag.209 initiates transaction ID 12266 with host helios by requesting
-up to 8 packets
-.Sm off
-.Pq the Dq <0\-7> .
-.Sm on
-The hex number at the end of the line is the value of the
-.Ar userdata
-field in the request.
-.Pp
-Helios responds with 8 512-byte packets.
-The
-.Dq : Ns Ar n
-following the
-transaction ID gives the packet sequence number in the 

Re: ifconfig: Fix/improve settimeslot(), simplify get_ts_map() out

[Stuart Henderson]

On 2017/06/08 19:17, Stuart Henderson wrote:
> On 2017/06/08 13:54, Ted Unangst wrote:
> > Klemens Nanni wrote:
> > > This fixes the primitive parsing route of settimeslot() to allow any
> > > possible list of slots and/or ranges, see the update manual section.
> > 
> > There doesn't appear to be any use of timeslot code, is there?
> > 
> > Better to delete it entirely.
> 
> ok.
> 
> nsh (in ports) will need changes for the kernel counterpart to this.
> Probably nothing else but I'll check.

SIOC[SG]IFTIMESLOT shows up about 17 times in the vendored crap in a
load of go ports as usual, but nothing besides nsh that actually uses it.

Re: doas: add confirm to prompt the user on what is to be executed

[Ted Unangst]

Adam Wolk wrote:
> This email is a request for comment, roughly I want to know if others see this
> feature as valuable. The diff currently lacks manpage changes, I will work on
> those if the general decision is to include this feature.
> 
> I won't cry if we decide to drop this. I would have implemented it
> anyways for fun :)

I'll let you keep this in your private collection. :)

Re: ifconfig: Fix/improve settimeslot(), simplify get_ts_map() out

[Stuart Henderson]

On 2017/06/08 13:54, Ted Unangst wrote:
> Klemens Nanni wrote:
> > This fixes the primitive parsing route of settimeslot() to allow any
> > possible list of slots and/or ranges, see the update manual section.
> 
> There doesn't appear to be any use of timeslot code, is there?
> 
> Better to delete it entirely.

ok.

nsh (in ports) will need changes for the kernel counterpart to this.
Probably nothing else but I'll check.

doas: add confirm to prompt the user on what is to be executed

[Adam Wolk]

Hi tech@

This is a feture that came up in a chat I had with Kurt Mosiejczuk. I have been
recently reading source daily as a learning experience and decided that
implementing the feature we discussed would be a nice exercise.

The attached diff extends the configuration syntax with a new option 'confirm'
which when present on a rule triggers doas to print what command is about to
run, by whom and as what user with a question asking if the execution should
continue.

Rationale for adding this are scripts that shell out doas commands showing just
a prompt with no way for the user to tell what command he is authenticating.

with the following rule
permit confirm persist mulander as root

running a script that calls doas results in the following behavior:

$ sh test.sh
mulander wants to run 'whoami' as root. Continue? [yN]
doas: aborted by user

allowing the user to bail out or proceed. This re-uses variables already there
for syslog logging (except we don't use pwd as that would require moving getcwd
calls early and we had to introduce `first` for getchar checking).

brynet@ pointed out -n (non interactive mode), when -n is present we bail out
with the following message:

$ doas.new -n whoami
doas: Confirmation required

This email is a request for comment, roughly I want to know if others see this
feature as valuable. The diff currently lacks manpage changes, I will work on
those if the general decision is to include this feature.

I won't cry if we decide to drop this. I would have implemented it
anyways for fun :)

Regards,
Adam
Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.72
diff -u -p -r1.72 doas.c
--- doas.c  27 May 2017 09:51:07 -  1.72
+++ doas.c  8 Jun 2017 17:37:20 -
@@ -256,7 +256,7 @@ main(int argc, char **argv)
uid_t target = 0;
gid_t groups[NGROUPS_MAX + 1];
int ngroups;
-   int i, ch;
+   int i, ch, first;
int sflag = 0;
int nflag = 0;
char cwdpath[PATH_MAX];
@@ -355,6 +355,20 @@ main(int argc, char **argv)
syslog(LOG_AUTHPRIV | LOG_NOTICE,
"failed command for %s: %s", myname, cmdline);
errc(1, EPERM, NULL);
+   }
+
+   if (rule->options & CONFIRM) {
+   if (nflag)
+   errx(1, "Confirmation required");
+
+   printf("%s wants to run '%s' as %s. Continue? [yN] ",
+   myname, cmdline, pw->pw_name);
+   fflush(stdout);
+   first = ch = getchar();
+   while (ch != '\n' && ch != EOF)
+   ch = getchar();
+   if (first != 'y' && first != 'Y')
+   errx(1, "aborted by user");
}
 
if (!(rule->options & NOPASS)) {
Index: doas.h
===
RCS file: /cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.13
diff -u -p -r1.13 doas.h
--- doas.h  6 Apr 2017 21:12:06 -   1.13
+++ doas.h  8 Jun 2017 17:37:20 -
@@ -37,3 +37,5 @@ char **prepenv(const struct rule *);
 #define NOPASS 0x1
 #define KEEPENV0x2
 #define PERSIST0x4
+#define CONFIRM0x8
+
Index: parse.y
===
RCS file: /cvs/src/usr.bin/doas/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- parse.y 2 Jan 2017 01:40:20 -   1.26
+++ parse.y 8 Jun 2017 17:37:20 -
@@ -69,7 +69,7 @@ arraylen(const char **arr)
 
 %}
 
-%token TPERMIT TDENY TAS TCMD TARGS
+%token TPERMIT TDENY TAS TCMD TCONFIRM TARGS
 %token TNOPASS TPERSIST TKEEPENV TSETENV
 %token TSTRING
 
@@ -136,6 +136,9 @@ options:/* none */ {
 option:TNOPASS {
$$.options = NOPASS;
$$.envlist = NULL;
+   } | TCONFIRM {
+   $$.options = CONFIRM;
+   $$.envlist = NULL;
} | TPERSIST {
$$.options = PERSIST;
$$.envlist = NULL;
@@ -209,6 +212,7 @@ static struct keyword {
{ "cmd", TCMD },
{ "args", TARGS },
{ "nopass", TNOPASS },
+   { "confirm", TCONFIRM },
{ "persist", TPERSIST },
{ "keepenv", TKEEPENV },
{ "setenv", TSETENV },

Re: ifconfig: Fix/improve settimeslot(), simplify get_ts_map() out

[Ted Unangst]

Klemens Nanni wrote:
> This fixes the primitive parsing route of settimeslot() to allow any
> possible list of slots and/or ranges, see the update manual section.

There doesn't appear to be any use of timeslot code, is there?

Better to delete it entirely.


Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.285
diff -u -p -r1.285 ifconfig.8
--- ifconfig.8  8 Jun 2017 00:46:42 -   1.285
+++ ifconfig.8  8 Jun 2017 17:52:33 -
@@ -450,9 +450,6 @@ and
 .Xr pf.conf 5 .
 .It Cm -rtlabel
 Clear the route label.
-.It Cm timeslot Ar timeslot_range
-Set the timeslot range map, which is used to control which channels
-an interface device uses.
 .It Cm up
 Mark an interface
 .Dq up .
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.343
diff -u -p -r1.343 ifconfig.c
--- ifconfig.c  6 Jun 2017 04:52:40 -   1.343
+++ ifconfig.c  8 Jun 2017 17:52:33 -
@@ -208,8 +208,6 @@ voidclone_create(const char *, int);
 void   clone_destroy(const char *, int);
 void   unsetmediaopt(const char *, int);
 void   setmediainst(const char *, int);
-void   settimeslot(const char *, int);
-void   timeslot_status(void);
 void   setmpelabel(const char *, int);
 void   process_mpw_commands(void);
 void   setmpwencap(const char *, int);
@@ -437,7 +435,6 @@ const structcmd {
{ "-pppoesvc",  1,  0,  setpppoe_svc },
{ "pppoeac",NEXTARG,0,  setpppoe_ac },
{ "-pppoeac",   1,  0,  setpppoe_ac },
-   { "timeslot",   NEXTARG,0,  settimeslot },
{ "authproto",  NEXTARG,0,  setsroto },
{ "authname",   NEXTARG,0,  setspppname },
{ "authkey",NEXTARG,0,  setspppkey },
@@ -2612,110 +2609,6 @@ setmediainst(const char *val, int d)
/* Media will be set after other processing is complete. */
 }
 
-/*
- * Note: 
- * bits:   0   1   2   3   4   5      24   25   ...   30   31
- * T1 mode:   N/A ch1 ch2 ch3 ch4 ch5ch24  N/AN/A  N/A
- * E1 mode:   ts0 ts1 ts2 ts3 ts4 ts5ts24  ts25   ts30 ts31
- */
-#ifndef SMALL
-/* ARGSUSED */
-void
-settimeslot(const char *val, int d)
-{
-#define SINGLE_CHANNEL 0x1
-#define RANGE_CHANNEL  0x2
-#define ALL_CHANNELS   0x
-   unsigned long   ts_map = 0;
-   char*ptr = (char *)val;
-   int ts_flag = 0;
-   int ts = 0, ts_start = 0;
-
-   if (strcmp(val,"all") == 0) {
-   ts_map = ALL_CHANNELS;
-   } else {
-   while (*ptr != '\0') {
-   if (isdigit((unsigned char)*ptr)) {
-   ts = strtoul(ptr, , 10);
-   ts_flag |= SINGLE_CHANNEL;
-   } else {
-   if (*ptr == '-') {
-   ts_flag |= RANGE_CHANNEL;
-   ts_start = ts;
-   } else {
-   ts_map |= get_ts_map(ts_flag,
-   ts_start, ts);
-   ts_flag = 0;
-   }
-   ptr++;
-   }
-   }
-   if (ts_flag)
-   ts_map |= get_ts_map(ts_flag, ts_start, ts);
-
-   }
-   (void) strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
-   ifr.ifr_data = (caddr_t)_map;
-
-   if (ioctl(s, SIOCSIFTIMESLOT, (caddr_t)) < 0)
-   err(1, "SIOCSIFTIMESLOT");
-}
-
-unsigned long
-get_ts_map(int ts_flag, int ts_start, int ts_stop)
-{
-   int i = 0;
-   unsigned long   map = 0, mask = 0;
-
-   if ((ts_flag & (SINGLE_CHANNEL | RANGE_CHANNEL)) == 0)
-   return 0;
-   if (ts_flag & RANGE_CHANNEL) { /* Range of channels */
-   for (i = ts_start; i <= ts_stop; i++) {
-   mask = 1 << i;
-   map |=mask;
-   }
-   } else { /* Single channel */
-   mask = 1 << ts_stop;
-   map |= mask;
-   }
-   return map;
-}
-
-void
-timeslot_status(void)
-{
-   char*sep = " ";
-   unsigned longts_map = 0;
-   int  i, start = -1;
-
-   ifr.ifr_data = (caddr_t)_map;
-
-   if (ioctl(s, SIOCGIFTIMESLOT, (caddr_t)) == -1)
-   return;
-
-   printf("\ttimeslot:");
-   for (i = 0; i < sizeof(ts_map) * 8; i++) {
-   if (start == -1 && ts_map & (1 << i))
-   start = i;
-   else if (start != -1 && !(ts_map & (1 << i))) {
-   if 

ifconfig: Fix/improve settimeslot(), simplify get_ts_map() out

[Klemens Nanni]

This fixes the primitive parsing route of settimeslot() to allow any
possible list of slots and/or ranges, see the update manual section.

The old code would happily mask "1,2-" into 0b11, settimeslot() now fails
on such broken ranges and also checks for inconsistencies like "4-1",
etc.

get_ts_map() has been heavily simplified to fit into a small macro
TS_RANGE_MASK().

strsep(3) is used instead of the deprecated strtok(3).

While here, remove trailing spaces and align function prototypes for
better readability.


Feedback/OK?

Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.285
diff -u -p -r1.285 ifconfig.8
--- ifconfig.8  8 Jun 2017 00:46:42 -   1.285
+++ ifconfig.8  8 Jun 2017 15:23:54 -
@@ -450,9 +450,18 @@ and
.Xr pf.conf 5 .
.It Cm -rtlabel
Clear the route label.
-.It Cm timeslot Ar timeslot_range
+.It Cm timeslot Ar n-m,n-m,...
Set the timeslot range map, which is used to control which channels
an interface device uses.
+.Ar n
+and
+.Ar m
+must range from 0 to 31. Multiple ranges may overlap.
+.Ar m
+must be
+greater than n if given.
+.Dq all
+can be used as alias to denote all timeslots.
.It Cm up
Mark an interface
.Dq up .
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.343
diff -u -p -r1.343 ifconfig.c
--- ifconfig.c  6 Jun 2017 04:52:40 -   1.343
+++ ifconfig.c  8 Jun 2017 15:23:54 -
@@ -326,13 +326,13 @@ int   actions;/* Actions 
performed */
#define NEXTARG20xfd

const structcmd {
-   char*c_name;
-   int c_parameter;/* NEXTARG means next argv */
-   int c_action;   /* defered action */
+   char *c_name;
+   int   c_parameter;  /* NEXTARG means next argv */
+   int   c_action; /* defered action */
void(*c_func)(const char *, int);
void(*c_func2)(const char *, const char *);
} cmds[] = {
-   { "up",   IFF_UP, 0,  setifflags } ,
+   { "up",   IFF_UP, 0,  setifflags },
{ "down", -IFF_UP,0,  setifflags },
{ "arp",  -IFF_NOARP, 0,  setifflags },
{ "-arp", IFF_NOARP,  0,  setifflags },
@@ -427,11 +427,11 @@ const struct  cmd {
{ "defer",1,  0,  setpfsync_defer },
{ "-defer",   0,  0,  setpfsync_defer },
/* giftunnel is for backward compat */
-   { "giftunnel",  NEXTARG2, 0,  NULL, settunnel } ,
-   { "tunnel",   NEXTARG2,   0,  NULL, settunnel } ,
-   { "deletetunnel",  0, 0,  deletetunnel } ,
-   { "tunneldomain", NEXTARG,0,  settunnelinst } ,
-   { "tunnelttl",NEXTARG,0,  settunnelttl } ,
+   { "giftunnel",  NEXTARG2, 0,  NULL, settunnel },
+   { "tunnel",   NEXTARG2,   0,  NULL, settunnel },
+   { "deletetunnel",  0, 0,  deletetunnel },
+   { "tunneldomain", NEXTARG,0,  settunnelinst },
+   { "tunnelttl",NEXTARG,0,  settunnelttl },
{ "pppoedev", NEXTARG,0,  setpppoe_dev },
{ "pppoesvc", NEXTARG,0,  setpppoe_svc },
{ "-pppoesvc",1,  0,  setpppoe_svc },
@@ -537,15 +537,15 @@ const struct  cmd {
#endif /* SMALL */
#if 0
/* XXX `create' special-cased below */
-   { "create",   0,  0,  clone_create } ,
+   { "create",   0,  0,  clone_create },
#endif
-   { "destroy",  0,  0,  clone_destroy } ,
-   { "link0",IFF_LINK0,  0,  setifflags } ,
-   { "-link0",   -IFF_LINK0, 0,  setifflags } ,
-   { "link1",IFF_LINK1,  0,  setifflags } ,
-   { "-link1",   -IFF_LINK1, 0,  setifflags } ,
-   { "link2",IFF_LINK2,  0,  setifflags } ,
-   { "-link2",   -IFF_LINK2, 0,  setifflags } ,
+   { "destroy",  0,  0,  clone_destroy },
+   { "link0",IFF_LINK0,  0,  setifflags },
+   { "-link0",   -IFF_LINK0, 0,  setifflags },
+   { "link1",IFF_LINK1,  0,  setifflags },
+   { "-link1",   -IFF_LINK1, 0,  setifflags },
+   { "link2",IFF_LINK2,  0,  setifflags },
+   { "-link2",   -IFF_LINK2, 0,  setifflags },
{ "media",NEXTARG0,   A_MEDIA,setmedia },
{ "mediaopt", 

Re: amd64: EFI boot over network try to load kernel from hd0

[Patrick Wildt]

On Thu, Jun 08, 2017 at 11:42:44PM +0900, YASUOKA Masahiko wrote:
> Hi,
> 
> Tested the diff.  It works fine.
> 
> On Wed, 7 Jun 2017 20:23:43 +0200
> Patrick Wildt  wrote:
> > Do you want it to run diskless with root on NFS?  Well, I haven't
> > implemented that, but here's a diff that should allow you to load
> > the kernel from a TFTP server.
> > 
> > I have a similar diff on arm64 (which probably doesn't work with
> > u-boot's EFI API, but works on EDK2 machines) which I use for testing
> > kernels more easily.
> > 
> > I'm not sure how helpful this feature is for most people.  It's probably
> > nice for developers who want to boot different kernels over network, but
> > so far it does not support root on NFS, or only accidentally.
> 
> What the diff is missing is passing the mac address of booted NIC as
> "bootmac" parameter?
> 
> --yasuoka
> 

Does this do what you want it to do?

Patrick

diff --git a/sys/arch/amd64/stand/efiboot/Makefile.common 
b/sys/arch/amd64/stand/efiboot/Makefile.common
index d821e0bc39a..7742e201683 100644
--- a/sys/arch/amd64/stand/efiboot/Makefile.common
+++ b/sys/arch/amd64/stand/efiboot/Makefile.common
@@ -24,7 +24,7 @@ AFLAGS+=  -pipe -fPIC
 
 .PATH: ${.CURDIR}/..
 SRCS+= self_reloc.c
-SRCS+= efiboot.c efidev.c
+SRCS+= efiboot.c efidev.c efipxe.c
 SRCS+= conf.c
 
 .PATH: ${S}/stand/boot
diff --git a/sys/arch/amd64/stand/efiboot/conf.c 
b/sys/arch/amd64/stand/efiboot/conf.c
index 3b2059e414f..55ab425fab2 100644
--- a/sys/arch/amd64/stand/efiboot/conf.c
+++ b/sys/arch/amd64/stand/efiboot/conf.c
@@ -31,12 +31,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include "disk.h"
 #include "efiboot.h"
 #include "efidev.h"
+#include "efipxe.h"
 
 const char version[] = "3.33";
 
@@ -51,7 +53,7 @@ void (*i386_probe1[])(void) = {
cninit, efi_memprobe
 };
 void (*i386_probe2[])(void) = {
-   efi_diskprobe, diskprobe
+   efi_pxeprobe, efi_diskprobe, diskprobe
 };
 
 struct i386_boot_probes probe_list[] = {
@@ -62,6 +64,8 @@ int nibprobes = nitems(probe_list);
 
 
 struct fs_ops file_system[] = {
+   { tftp_open,   tftp_close,   tftp_read,   tftp_write,   tftp_seek,
+ tftp_stat,   tftp_readdir   },
{ ufs_open,ufs_close,ufs_read,ufs_write,ufs_seek,
  ufs_stat,ufs_readdir},
{ cd9660_open, cd9660_close, cd9660_read, cd9660_write, cd9660_seek,
@@ -76,10 +80,8 @@ struct fs_ops file_system[] = {
 int nfsys = nitems(file_system);
 
 struct devsw   devsw[] = {
-   { "EFI", efistrategy, efiopen, eficlose, efiioctl },
-#if 0
{ "TFTP", tftpstrategy, tftpopen, tftpclose, tftpioctl },
-#endif
+   { "EFI", efistrategy, efiopen, eficlose, efiioctl },
 };
 int ndevs = nitems(devsw);
 
diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
b/sys/arch/amd64/stand/efiboot/efiboot.c
index 9b6d5fc00fd..d753ffbf919 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -52,9 +52,8 @@ static EFI_GUIDblkio_guid = BLOCK_IO_PROTOCOL;
 static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
 u_long  efi_loadaddr;
 
-static int  efi_device_path_depth(EFI_DEVICE_PATH *dp, int);
-static int  efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *,
-   int);
+int efi_device_path_depth(EFI_DEVICE_PATH *dp, int);
+int efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
 static void efi_heap_init(void);
 static void efi_memprobe_internal(void);
 static void efi_video_init(void);
@@ -96,6 +95,11 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab)
bios_bootdev = 0x80;
efi_bootdp = dp0;
break;
+   } else if (DevicePathType(dp) == MESSAGING_DEVICE_PATH&&
+   DevicePathSubType(dp) == MSG_MAC_ADDR_DP) {
+   bios_bootdev = 0x0;
+   efi_bootdp = dp0;
+   break;
}
}
}
@@ -215,7 +219,7 @@ next:
free(handles, sz);
 }
 
-static int
+int
 efi_device_path_depth(EFI_DEVICE_PATH *dp, int dptype)
 {
int i;
@@ -228,7 +232,7 @@ efi_device_path_depth(EFI_DEVICE_PATH *dp, int dptype)
return (-1);
 }
 
-static int
+int
 efi_device_path_ncmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int deptn)
 {
int  i, cmp;
diff --git a/sys/arch/amd64/stand/efiboot/efiboot.h 
b/sys/arch/amd64/stand/efiboot/efiboot.h
index 8a07d5b46b3..ac50556748c 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.h
+++ b/sys/arch/amd64/stand/efiboot/efiboot.h
@@ -21,6 +21,7 @@ void   efi_cons_probe(struct consdev *);
 voidefi_memprobe(void);
 voidefi_hardprobe(void);
 voidefi_diskprobe(void);
+voidefi_pxeprobe(void);
 voidefi_cons_init(struct consdev *);
 int   

Re: amd64: EFI boot over network try to load kernel from hd0

[YASUOKA Masahiko]

Hi,

Tested the diff.  It works fine.

On Wed, 7 Jun 2017 20:23:43 +0200
Patrick Wildt  wrote:
> Do you want it to run diskless with root on NFS?  Well, I haven't
> implemented that, but here's a diff that should allow you to load
> the kernel from a TFTP server.
> 
> I have a similar diff on arm64 (which probably doesn't work with
> u-boot's EFI API, but works on EDK2 machines) which I use for testing
> kernels more easily.
> 
> I'm not sure how helpful this feature is for most people.  It's probably
> nice for developers who want to boot different kernels over network, but
> so far it does not support root on NFS, or only accidentally.

What the diff is missing is passing the mac address of booted NIC as
"bootmac" parameter?

--yasuoka

Re: patch(1) is fucked up

[Todd C. Miller]

On Thu, 08 Jun 2017 15:33:11 +0200, Marc Espie wrote:

> There is zero option that says "assume all those patches are correct and
> error out if something untowards happen".
> 
> This is really annoying for ports.
> 
> - prompting for non-existent filenames breaks  automated builds... dpb avoids
> that by explicitly zapping stdin.
> 
> - the new one. In the absence of input, patch automatically assumes -R when
> needed... just ran into that one with a patch that didn't get removed.

You can use the -N flag to prevent patch from switching to reversed
mode.  There is not currently a flag to error out when patch would
otherwise prompt the user.

 - todd

patch(1) is fucked up

[Marc Espie]

There is zero option that says "assume all those patches are correct and
error out if something untowards happen".

This is really annoying for ports.

- prompting for non-existent filenames breaks  automated builds... dpb avoids
that by explicitly zapping stdin.

- the new one. In the absence of input, patch automatically assumes -R when
needed... just ran into that one with a patch that didn't get removed.

There are some batch options, but they just continue without erroring out
if something weird happens.

What's missing is some kind of --strict option, like when a patch doesn't
apply, error out, don't prompt for anything, and never assert -R...

Does this exist elsewhere ?

Otherwise, parsing the very chatty output of patchand erroring out
is what I'm going to have to do... :(

isakmpd(8) use-after-free

[Martin Pieuchot]

Michał Koc reported a crash on misc@, turns out it's a use-after-free:
http://marc.info/?l=openbsd-misc=149597472223216=2

The trace indicates that argument given to pf_key_v2_stayalive() is no
longer valid:

  #0  conf_get_str (section=0xa8735b03f80 '  , tag=0xa8459272809 "Phase") at 
/usr/src/sbin/isakmpd/conf.c:94
  #1  0x0a84590293b4 in pf_key_v2_remove_conf (section=0xa8735b03f80 ' 
 ) at 
/usr/src/sbin/isakmpd/pf_key_v2.c:1905
  #2  0x0a845902956a in pf_key_v2_stayalive (exchange=0xa86a6f44200, 
vconn=0xa8735b03f80, fail=1) at /usr/src/sbin/isakmpd/pf_key_v2.c:2131


In r1.58 of pf_key_v2.c angelos@ move the argument given to
pf_key_v2_connection_check(), the one used after free, from
the stack to the heap:

   Dynamically allocate conn, as this is given to the exchange; cleanup
   conf space on failure to establish dynamic SA. ok niklas@

I don't understand the whole magic of function pointers in exchange.c
but what's interesting is that in his diff he stopped dereferencing
'exchange->name'.

But in pf_key_v2_connection_check() the 'conn' argument is passed as
'name' and as 'arg'...  So the diff below fixes Michał's problem.  I'd
appreciate if more people could test it and check if isakmpd(8) do not
leaking more memory than it already does.

Note that this diff do not fix the 'conn' leak introduced in the above
mentioned commit when a connection exist and exchange_establish() is
not called.

Comments, oks?

Index: pf_key_v2.c
===
RCS file: /cvs/src/sbin/isakmpd/pf_key_v2.c,v
retrieving revision 1.198
diff -u -p -r1.198 pf_key_v2.c
--- pf_key_v2.c 28 Feb 2017 16:46:27 -  1.198
+++ pf_key_v2.c 8 Jun 2017 11:00:47 -
@@ -2131,16 +2131,25 @@ pf_key_v2_stayalive(struct exchange *exc
pf_key_v2_remove_conf(conn);
pf_key_v2_remove_conf(conn);
}
+   free(conn);
 }
 
 /* Check if a connection CONN exists, otherwise establish it.  */
 void
 pf_key_v2_connection_check(char *conn)
 {
+   char*conn2 = NULL;
+
if (!sa_lookup_by_name(conn, 2)) {
+   conn2 = strdup(conn); /* will be freed in pf_key_v2_stayalive */
+   if (!conn2) {
+   LOG_DBG((LOG_SYSDEP, 70, "pf_key_v2_connection_check: "
+   "strdup(%s) failed", conn));
+   return;
+   }
LOG_DBG((LOG_SYSDEP, 70,
-   "pf_key_v2_connection_check: SA for %s missing", conn));
-   exchange_establish(conn, pf_key_v2_stayalive, conn, 0);
+   "pf_key_v2_connection_check: SA for %s missing", conn2));
+   exchange_establish(conn, pf_key_v2_stayalive, conn2, 0);
} else
LOG_DBG((LOG_SYSDEP, 70, "pf_key_v2_connection_check: "
"SA for %s exists", conn));

Re: [patch] Avoid system(3) in ikectl

[Jonathan Gray]

On Fri, May 19, 2017 at 12:32:16AM -0500, Matthew Martin wrote:
> ikectl errors in a number of situations where shell special characters
> are used. For example:
> 
> % doas ikectl ca test create password \'
> [...]
> subject=/C=DE/ST=Lower Saxony/L=Hanover/O=OpenBSD/OU=iked/CN=VPN 
> CA/emailAddress=r...@openbsd.org
> Getting Private key
> sh: no closing quote
> 
> This is because it uses system(3) in various places to run openssl, tar,
> and zip. Take the hint from the system(3) man page, and write a small
> function that does the fork and exec bypassing sh.
> 
> Keep in mind while reviewing ca->batch was either "" or "-batch " and is
> now either "" or "-batch".
> 
> - Matthew Martin

This would be simpler if the 'run' style function just took a NULL
terminated array.  Closer to how other things work and could then
be passed directly to an exec call.

> 
> 
> 
> diff --git ikeca.c ikeca.c
> index cee6623a30f..69ca076407b 100644
> --- ikeca.c
> +++ ikeca.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -68,7 +69,7 @@ struct ca {
>   char serial[PATH_MAX];
>   char sslcnf[PATH_MAX];
>   char extcnf[PATH_MAX];
> - char batch[PATH_MAX];
> + char batch[sizeof("-batch")];
>   char*caname;
>  };
>  
> @@ -117,6 +118,7 @@ void   ca_setenv(const char *, const char *);
>  void  ca_clrenv(void);
>  void  ca_setcnf(struct ca *, const char *);
>  void  ca_create_index(struct ca *);
> +int staticrun(int, ...);
>  
>  /* util.c */
>  int   expand_string(char *, size_t, const char *, const char *);
> @@ -131,7 +133,6 @@ int
>  ca_key_create(struct ca *ca, char *keyname)
>  {
>   struct stat  st;
> - char cmd[PATH_MAX * 2];
>   char path[PATH_MAX];
>  
>   snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -141,10 +142,7 @@ ca_key_create(struct ca *ca, char *keyname)
>   return (0);
>   }
>  
> - snprintf(cmd, sizeof(cmd),
> - "%s genrsa -out %s 2048",
> - PATH_OPENSSL, path);
> - system(cmd);
> + run(5, PATH_OPENSSL, "genrsa", "-out", path, "2048");
>   chmod(path, 0600);
>  
>   return (0);
> @@ -201,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
>  int
>  ca_request(struct ca *ca, char *keyname, int type)
>  {
> - charcmd[PATH_MAX * 2];
>   charhostname[HOST_NAME_MAX+1];
>   charname[128];
> + charkey[PATH_MAX];
>   charpath[PATH_MAX];
>  
>   ca_setenv("$ENV::CERT_CN", keyname);
> @@ -227,13 +225,11 @@ ca_request(struct ca *ca, char *keyname, int type)
>  
>   ca_setcnf(ca, keyname);
>  
> + snprintf(key, sizeof(key), "%s/private/%s.key",  ca->sslpath, keyname);
>   snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
> - snprintf(cmd, sizeof(cmd), "%s req %s-new"
> - " -key %s/private/%s.key -out %s -config %s",
> - PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
> - path, ca->sslcnf);
>  
> - system(cmd);
> + run(10, PATH_OPENSSL, "req", ca->batch, "-new", "-key", key,
> + "-out", path, "-config", ca->sslcnf);
>   chmod(path, 0600);
>  
>   return (0);
> @@ -242,7 +238,11 @@ ca_request(struct ca *ca, char *keyname, int type)
>  int
>  ca_sign(struct ca *ca, char *keyname, int type)
>  {
> - charcmd[PATH_MAX * 2];
> + charkeyfile[PATH_MAX];
> + charcert[PATH_MAX];
> + charout[PATH_MAX];
> + charin[PATH_MAX];
> + charpassfile[PATH_MAX+5];
>   const char  *extensions = NULL;
>  
>   if (type == HOST_IPADDR) {
> @@ -259,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, int type)
>   ca_setenv("$ENV::CASERIAL", ca->serial);
>   ca_setcnf(ca, keyname);
>  
> - snprintf(cmd, sizeof(cmd),
> - "%s ca -config %s -keyfile %s/private/ca.key"
> - " -cert %s/ca.crt"
> - " -extfile %s -extensions %s -out %s/%s.crt"
> - " -in %s/private/%s.csr"
> - " -passin file:%s -outdir %s -batch",
> - PATH_OPENSSL, ca->sslcnf, ca->sslpath,
> - ca->sslpath,
> - ca->extcnf, extensions, ca->sslpath, keyname,
> - ca->sslpath, keyname,
> - ca->passfile, ca->sslpath);
> + snprintf(keyfile, sizeof(keyfile), "%s/private/ca.key",  ca->sslpath);
> + snprintf(cert, sizeof(cert), "%s/ca.crt", ca->sslpath);
> + snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
> + snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);
> + snprintf(passfile, sizeof(passfile), "file:%s", ca->passfile);
>  
> - system(cmd);
> + run(21, PATH_OPENSSL, "ca", "-config", ca->sslcnf, "-keyfile", keyfile,

Re: [patch] Use readpassphrase in ikectl

[Jonathan Gray]

On Fri, May 19, 2017 at 12:35:44AM -0500, Matthew Martin wrote:
> While making the last patch, I noticed ikectl uses getpass. Use
> readpassphrase instead and explicit_bzero the buffers.
> 
> - Matthew Martin

What is the goal here?  It can't be to use a different buffer size as
the same size as getpass is used.

getpass is implemented in terms of readpassphrase.  Looking at the
implementation the flags argument should be RPP_ECHO_OFF (0) rather
than just 0.

char *
getpass(const char *prompt)
{
static char buf[_PASSWORD_LEN + 1];

return(readpassphrase(prompt, buf, sizeof(buf), RPP_ECHO_OFF));
}

> 
> 
> 
> diff --git ikeca.c ikeca.c
> index 69ca076407b..2ec010a5831 100644
> --- ikeca.c
> +++ ikeca.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -636,7 +637,7 @@ ca_export(struct ca *ca, char *keyname, char *myname, 
> char *password)
>   DIR *dexp;
>   struct dirent   *de;
>   struct stat  st;
> - char*pass;
> + char pass[_PASSWORD_LEN + 1];
>   char prev[_PASSWORD_LEN + 1];
>   char passenv[_PASSWORD_LEN + 8];
>   char oname[PATH_MAX];
> @@ -667,16 +668,21 @@ ca_export(struct ca *ca, char *keyname, char *myname, 
> char *password)
>   if (password != NULL)
>   snprintf(passenv, sizeof(passenv), "EXPASS=%s", password);
>   else {
> - pass = getpass("Export passphrase:");
> - if (pass == NULL || *pass == '\0')
> - err(1, "password not set");
> -
> - strlcpy(prev, pass, sizeof(prev));
> - pass = getpass("Retype export passphrase:");
> - if (pass == NULL || strcmp(prev, pass) != 0)
> + if (readpassphrase("Export passphrase:", prev, sizeof(prev), 0)
> + == NULL)
> + errx(1, "unable to read passphrase");
> + if (*prev == '\0')
> + errx(1, "password not set");
> +
> + if (readpassphrase("Retype export passphrase:", pass,
> + sizeof(pass), 0) == NULL)
> + errx(1, "unable to read passphrase");
> + if (strcmp(prev, pass) != 0)
>   errx(1, "passphrase does not match!");
>  
>   snprintf(passenv, sizeof(passenv), "EXPASS=%s", pass);
> + explicit_bzero(pass, sizeof(pass));
> + explicit_bzero(prev, sizeof(prev));
>   }
>  
>   snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
> 

Re: Better handling of short reads

[Mike Belopuhov]

On Wed, Jun 07, 2017 at 23:04 -0500, Amit Kulkarni wrote:
> On Wed, 7 Jun 2017 21:27:27 -0500
> Amit Kulkarni  wrote:
> 
> > On Thu, 8 Jun 2017 01:57:25 +0200
> > Mike Belopuhov  wrote:
> > 
> > > On Wed, Jun 07, 2017 at 18:35 -0500, Amit Kulkarni wrote:
> > > > Wow, please get this in!!!
> > > > 
> > > > This fixes cvs update on hard disks, to go much much faster. When I am
> > > > updating the entire set of cvs trees: www, src, xenocara, ports, I can
> > > > still use firefox and have it perfectly usable. There's a night and
> > > > day improvement, before and after. Thanks for debugging and fixing
> > > > this.
> > > >
> > > 
> > > What kind of broken hardware do you have that this diff helps you?
> > > Can you show us your dmesg?
> > > 
> 
> Please ignore previous dmesg, it was incomplete.
> 

Are you 100% sure this diff changes anything for you?
Can you please try the one below.  It adds a printf.

diff --git sys/kern/vfs_bio.c sys/kern/vfs_bio.c
index 95bc80bc0e6..9316e6e0eb2 100644
--- sys/kern/vfs_bio.c
+++ sys/kern/vfs_bio.c
@@ -534,10 +534,27 @@ bread_cluster_callback(struct buf *bp)
 */
buf_fix_mapping(bp, newsize);
bp->b_bcount = newsize;
}
 
+   /* Invalidate read-ahead buffers if read short */
+   if (bp->b_resid > 0) {
+   printf("read %ld resid %ld\n", bp->b_bcount, bp->b_resid);
+   for (i = 0; xbpp[i] != NULL; i++)
+   continue;
+   for (i = i - 1; i != 0; i--) {
+   if (xbpp[i]->b_bufsize <= bp->b_resid) {
+   bp->b_resid -= xbpp[i]->b_bufsize;
+   SET(xbpp[i]->b_flags, B_INVAL);
+   } else if (bp->b_resid > 0) {
+   bp->b_resid = 0;
+   SET(xbpp[i]->b_flags, B_INVAL);
+   } else
+   break;
+   }
+   }
+
for (i = 1; xbpp[i] != 0; i++) {
if (ISSET(bp->b_flags, B_ERROR))
SET(xbpp[i]->b_flags, B_INVAL | B_ERROR);
biodone(xbpp[i]);
}

Re: diff: add missing rtm_send to nd6

[Jan Klemkow]

Hi Martin,
 
On Thu, Jun 08, 2017 at 09:44:37AM +0200, Martin Pieuchot wrote:
> On 08/06/17(Thu) 00:47, Jan Klemkow wrote:
> > This diff adds a missing routing message to the neighbor discovery code.
> > The message informs the userland about new reachable IPv6 nodes on the
> > network.  The IPv6 network stack acts more like the IPv4 port by this
> > diff.  Now, the behavior is like arpcache() in netinet/if_ether.c on
> > line 653.
> 
> The call should be under KERNEL_LOCK()/KERNEL_UNLOCK().  Writing to
> routing sockets isn't protected by the NET_LOCK().
> 
> Yes this doesn't matter right now, but since we're working on removing
> the KERNEL_LOCK() from the protocol layer, let's have this ready ;)

Here is a fixed version of this diff.

Thanks,
Jan

Index: nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.116
diff -u -p -r1.116 nd6_nbr.c
--- nd6_nbr.c   16 May 2017 12:24:04 -  1.116
+++ nd6_nbr.c   8 Jun 2017 08:44:28 -
@@ -715,6 +715,10 @@ nd6_na_input(struct mbuf *m, int off, in
if (is_solicited) {
ln->ln_state = ND6_LLINFO_REACHABLE;
ln->ln_byhint = 0;
+   /* Notify userland that a new ND entry is reachable. */
+   KERNEL_LOCK();
+   rtm_send(rt, RTM_RESOLVE, ifp->if_rdomain);
+   KERNEL_UNLOCK();
if (!ND6_LLINFO_PERMANENT(ln)) {
nd6_llinfo_settimer(ln,
ND_IFINFO(ifp)->reachable);

Re: ifconfig.8 doco for vnetid and parent options

[Sebastian Benoit]

Claudio Jeker(cje...@diehard.n-r-g.com) on 2017.06.07 16:05:15 +0200:
> Be careful, AFAIK the capitalisation of IEEE standards does matter.
> You're right 802.1Q is the correct spelling but not for 802.1ad (where the
> lowercase version is the offical standard). IIRC the status of the
> standard is what causes the letters to become uppercase.

Lower case letters denote amendments to an existing standard.

So for example there is "802.1Q - Virtual LANs" and "802.1u - 802.1Q
Maintenance".

/B

Re: diff: add missing rtm_send to nd6

[Martin Pieuchot]

On 08/06/17(Thu) 00:47, Jan Klemkow wrote:
> Hi,
> 
> This diff adds a missing routing message to the neighbor discovery code.
> The message informs the userland about new reachable IPv6 nodes on the
> network.  The IPv6 network stack acts more like the IPv4 port by this
> diff.  Now, the behavior is like arpcache() in netinet/if_ether.c on
> line 653.

The call should be under KERNEL_LOCK()/KERNEL_UNLOCK().  Writing to
routing sockets isn't protected by the NET_LOCK().

Yes this doesn't matter right now, but since we're working on removing
the KERNEL_LOCK() from the protocol layer, let's have this ready ;)

> Index: nd6_nbr.c
> ===
> RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 nd6_nbr.c
> --- nd6_nbr.c 16 May 2017 12:24:04 -  1.116
> +++ nd6_nbr.c 7 Jun 2017 22:11:31 -
> @@ -715,6 +715,8 @@ nd6_na_input(struct mbuf *m, int off, in
>   if (is_solicited) {
>   ln->ln_state = ND6_LLINFO_REACHABLE;
>   ln->ln_byhint = 0;
> + /* Notify userland that a new ND entry is reachable. */
> + rtm_send(rt, RTM_RESOLVE, ifp->if_rdomain);
>   if (!ND6_LLINFO_PERMANENT(ln)) {
>   nd6_llinfo_settimer(ln,
>   ND_IFINFO(ifp)->reachable);
>