fine tuning PF_LOCK in pfioctl()

2017-10-27 Thread Alexandr Nedvedicky 
Hello,

patch below fine tunes PF_LOCK in pfioctl() function. Currently pfioctl()
function grabs PF_LOCK() for all operations at line 1006, right before 'the big
switch' is entered. The PF_LOCK() gets released once we are done with 'the big
switch' at line 2477:

   905 int
   906 pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
   907 {
   908 int  error = 0;
   909 
   910 /* XXX keep in sync with switch() below */
   911 if (securelevel > 1)
   912 switch (cmd) {
  
  1004 
  1005 NET_LOCK();
  1006 PF_LOCK();
  1007 switch (cmd) {
  1008 
  1009 case DIOCSTART:
  
  2472 default:
  2473 error = ENODEV;
  2474 break;
  2475 }
  2476 fail:
  2477 PF_UNLOCK();
  2478 NET_UNLOCK();
  2479 return (error);
 
patch below move responsibility for PF_LOCK manipulation to particular ioctl
commands. The change is investment for future. It will allow us to gradually
move individual PF subsystems out of PF_LOCK scope.

OK?

thanks and
regards
sasha

8<---8<---8<--8<
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index de40e934112..2e954111d03 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1003,10 +1003,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
}
 
NET_LOCK();
-   PF_LOCK();
switch (cmd) {
 
case DIOCSTART:
+   PF_LOCK();
if (pf_status.running)
error = EEXIST;
else {
@@ -1020,9 +1020,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
pf_create_queues();
DPFPRINTF(LOG_NOTICE, "pf: started");
}
+   PF_UNLOCK();
break;
 
case DIOCSTOP:
+   PF_LOCK();
if (!pf_status.running)
error = ENOENT;
else {
@@ -1031,6 +1033,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
pf_remove_queues();
DPFPRINTF(LOG_NOTICE, "pf: stopped");
}
+   PF_UNLOCK();
break;
 
case DIOCGETQUEUES: {
@@ -1038,6 +1041,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
struct pf_queuespec *qs;
u_int32_tnr = 0;
 
+   PF_LOCK();
pq->ticket = pf_main_ruleset.rules.active.ticket;
 
/* save state to not run over them all each time? */
@@ -1047,6 +1051,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
nr++;
}
pq->nr = nr;
+   PF_UNLOCK();
break;
}
 
@@ -1055,8 +1060,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
struct pf_queuespec *qs;
u_int32_tnr = 0;
 
+   PF_LOCK();
if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
error = EBUSY;
+   PF_UNLOCK();
break;
}
 
@@ -1066,9 +1073,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
qs = TAILQ_NEXT(qs, entries);
if (qs == NULL) {
error = EBUSY;
+   PF_UNLOCK();
break;
}
bcopy(qs, >queue, sizeof(pq->queue));
+   PF_UNLOCK();
break;
}
 
@@ -1078,8 +1087,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
u_int32_tnr;
int  nbytes;
 
+   PF_LOCK();
if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
error = EBUSY;
+   PF_UNLOCK();
break;
}
nbytes = pq->nbytes;
@@ -1091,6 +1102,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
qs = TAILQ_NEXT(qs, entries);
if (qs == NULL) {
error = EBUSY;
+   PF_UNLOCK();
break;
}
bcopy(qs, >queue, sizeof(pq->queue));
@@ -1104,6 +1116,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
);
if (error == 0)
pq->nbytes = nbytes;
+   PF_UNLOCK();
break;
}
 
@@ -,13 +1124,16 @@ pfioctl(dev_t dev, u_long

[PATCH] Fix out-of-bounds read in libc/regex

2017-10-27 Thread Vlad Tsyrklevich 
This is a patch for a bug found by oss-fuzz [1] through the copy of
libc/regex in LLVM and fixed downstream [2]. The bug is an
out-of-bounds read detected with address sanitizer that happens when
'sp' in p_b_coll_elems() includes NUL byte[s], e.g. if it's equal to
"GS\x00". In that case len will be equal to 4, and the
strncmp(cp->name, sp, len) call will succeed when cp->name is "GS" but
the cp->name[len] == '\0' comparison will cause the read to go
out-of-bounds. Checking the length using strlen() instead eliminates
the issue.

--- a/lib/libc/regex/regcomp.c
+++ b/lib/libc/regex/regcomp.c
@@ -823,7 +823,7 @@ p_b_coll_elem(struct parse *p,
 {
char *sp = p->next;
struct cname *cp;
-   int len;
+   size_t len;

while (MORE() && !SEETWO(endc, ']'))
NEXT();
@@ -833,7 +833,7 @@ p_b_coll_elem(struct parse *p,
}
len = p->next - sp;
for (cp = cnames; cp->name != NULL; cp++)
-   if (strncmp(cp->name, sp, len) == 0 && cp->name[len] == '\0')
+   if (strncmp(cp->name, sp, len) == 0 && strlen(cp->name) == len)
return(cp->code);   /* known name */
if (len == 1)
return(*sp);/* single character */


[1] https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3727
[2] https://reviews.llvm.org/D39380

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Mark Kettenis 
> Date: Fri, 27 Oct 2017 20:08:43 +0200
> From: Stefan Sperling 
> 
> On Fri, Oct 27, 2017 at 04:47:20PM +0200, Mark Kettenis wrote:
> > I don't know, but with the ioctl removed, there is nothing that sets
> > the IEEE80211_F_QOS flag anymore.  That seems weird.
> 
> This flag is just unused for now.
> It is a left-over from unfinished 11n support code by damien.
> But I expect that it will be useful.
> 
> A-MPDU frames (sent with Tx aggregation) are QoS frames.
> So we're not sending QoS frames yet, we are just receiving them.
> The receiving code path can detect QoS by checking for QoS headers
> in received frames, which works without a flag.
> 
> But the flag is already wired up in some of the Tx logic, and I
> expect the F_QOS flag will eventually be used as part of a Tx agg
> implementation. It could be set by drivers in order to enable QoS
> and thus advertise A-MPDU Tx support to net80211 (there is no good
> reason for using QoS without 11n). The stack then knows that it
> needs to supply QoS frame headers, and can provide a Tx subframe
> re-ordering buffer for drivers for cases where firmware doesn't
> provide one.

Thanks for the explanation!

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Stefan Sperling 
On Fri, Oct 27, 2017 at 04:47:20PM +0200, Mark Kettenis wrote:
> I don't know, but with the ioctl removed, there is nothing that sets
> the IEEE80211_F_QOS flag anymore.  That seems weird.

This flag is just unused for now.
It is a left-over from unfinished 11n support code by damien.
But I expect that it will be useful.

A-MPDU frames (sent with Tx aggregation) are QoS frames.
So we're not sending QoS frames yet, we are just receiving them.
The receiving code path can detect QoS by checking for QoS headers
in received frames, which works without a flag.

But the flag is already wired up in some of the Tx logic, and I
expect the F_QOS flag will eventually be used as part of a Tx agg
implementation. It could be set by drivers in order to enable QoS
and thus advertise A-MPDU Tx support to net80211 (there is no good
reason for using QoS without 11n). The stack then knows that it
needs to supply QoS frame headers, and can provide a Tx subframe
re-ordering buffer for drivers for cases where firmware doesn't
provide one.

Re: [PATCH 1/2] VMD: Require interface to be defined in switches

2017-10-27 Thread Mike Larkin 
On Fri, Oct 27, 2017 at 09:58:09AM +0200, Martin Pieuchot wrote:
> On 26/10/17(Thu) 23:47, Mike Larkin wrote:
> > On Fri, Oct 27, 2017 at 08:23:05AM +0200, Martin Pieuchot wrote:
> > > On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> > > > * Require interface name to be defined for switches in vm.conf
> > > > ** Requires user to create bridge(4) beforehand
> > > > * Remove code to create bridges on the fly
> > > > * Add code to ensure bridge really exists
> > > > * Update manpage switch and example sections
> > > 
> > > What's the motivation to ask people to create a bridge(4) beforehand?
> > > 
> > 
> > This discussion came out of a previous effort to fix a bug where vmd(8) 
> > doesn't
> > deconfigure bridges (and/or undo changes to bridges) it made during
> > initialization. After discussing this a bit with claudio, we decided that 
> > rather than having vmd try to do magic stuff with creating bridges and
> > tearing them down behind the scenes (including corner cases where vmd is
> > killed or crashes/etc), it made sense instead to say "you create the bridge
> > and tell me which one you want to use". This can easily be accomplished
> > with a 1 (or 2) line /etc/hostname.bridgeX file.
> > 
> > We may decide later to have vmd try to create bridges again at some point
> > but for now, I think this is a way to make some forward progress in this
> > area.
> 
> I don't want to stop progress but I don't understand how creating a
> bridge(4) beforehand help to "deconfigure" it.  Hence my question.

We want vmd(8) to be out of the bridge manipulation business, except for adding
and removing tapX interfaces for the guest VMs it manages.

Right now, vmd(8) is in the bridge creation and deletion business, and it is
that second part that is giving us headache. Thus, we decided to remove both
parts since we don't have a good solution as a whole.

-ml

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Mark Kettenis 
> Date: Fri, 27 Oct 2017 23:57:35 +1100
> From: Jonathan Gray 
> 
> On Fri, Oct 27, 2017 at 02:34:32PM +0200, Mark Kettenis wrote:
> > > Date: Fri, 27 Oct 2017 12:33:15 +0200
> > > From: Stefan Sperling 
> > > 
> > > On Fri, Oct 27, 2017 at 09:08:07PM +1100, Jonathan Gray wrote:
> > > > use in ifconfig was removed in 2009
> > > 
> > > Yes, please remove this. Thanks.
> > 
> > How would one go about setting QoS mode without this ioctl?
> 
> Is it needed outside of when it is implied as described in the
> ifconfig commit?

I don't know, but with the ioctl removed, there is nothing that sets
the IEEE80211_F_QOS flag anymore.  That seems weird.

> 
> revision 1.213
> date: 2009/02/15 08:34:36;  author: damien;  state: Exp;  lines: +56 -35;
> make "ifconfig if0 chan" list the channels supported by the device.
> add "ifconfig if0 scan" to scan for access points or to list known
> stations in Host AP mode.
> remove the [-]wmm command while i'm here.  QoS is mandatory with
> 802.11n so there's not much point into making it an option.
> fix parsing of the "powersave" command too.
> 
> discussed with deraadt@
> man page hints from jmc@
> display hints from sobrado@
> "i like it" cnst@, grange@
> 
>

Re: rm exits 0

2017-10-27 Thread Ingo Schwarze 
Hi,

Jason McIntyre wrote on Fri, Oct 27, 2017 at 11:51:25AM +0100:
> On Fri, Oct 27, 2017 at 11:42:30AM +0200, Jan Stary wrote:

>> "rm exits 0" does not seem right (though I'm not a native speaker).

> well it is clear, so i don;t see a problem. virtually every man page
> with an EXIT STATUS section uses this same standard text.

And besides, it has been the default rendering of the .Ex macro
for more than 15 years (Werner Lemberg, 2001):

http://git.savannah.gnu.org/cgit/groff.git/commit/tmac/doc.tmac?id=e345bdfac5bf9ca180445e93afe0a473c668ed8f

schwarze@isnote $ echo .Ex -std | mandoc -mdoc -Tascii | grep exits
The utility exits 0 on success, and >0 if an error occurs.
schwarze@isnote $ echo .Ex -std | groff -mdoc -Tascii 2>/dev/null | grep exits 
The utility exits 0 on success, and >0 if an error occurs.
schwarze@isnote $ export PATH=/usr/local/heirloom-doctools/bin/:$PATH
schwarze@isnote $ echo .Ex -std | nroff -mdoc | grep exits 
The  utility exits 0 on success, and >0 if an error occurs.

The wording was originally intoduced by Cynthia Livingston
on July 24, 1990 for 4.3BSD-Reno, when she set out to rewrite the
whole corpus of BSD manuals in the version 2 predecessor of her
new mdoc(7) language to replace the AT Copyright-encumbered man(7)
versions:

schwarze@isnote $ cd /co/4.3BSD-3Reno
schwarze@isnote $ grep -F .Dd $(grep -RFl 'exits 0' *)  
bin/cat/cat.1:.Dd July 24, 1990
bin/chmod/chmod.1:.Dd July 24, 1990
bin/cp/cp.1:.Dd July 24, 1990
bin/dd/dd.1:.Dd July 24, 1990
bin/echo/echo.1:.Dd July 24, 1990
bin/mkdir/mkdir.1:.Dd July 24, 1990
bin/mv/mv.1:.Dd July 24, 1990
bin/pwd/pwd.1:.Dd July 24, 1990
bin/sh/cd.1:.Dd July 24, 1990
usr.bin/column/column.1:.Dd July 24, 1990
usr.bin/comm/comm.1:.Dd July 24, 1990
usr.bin/cut/cut.1:.Dd July 24, 1990
usr.bin/hexdump/hexdump.1:.Dd July 24, 1990
usr.bin/join/join.1:.Dd July 24, 1990
usr.bin/logger/logger.1:.Dd July 24, 1990
usr.bin/mkfifo/mkfifo.1:.Dd July 24, 1990
usr.bin/paste/paste.1:.Dd July 24, 1990
usr.bin/pr/pr.1:.Dd July 24, 1990
usr.bin/printf/printf.1:.Dd July 24, 1990
usr.sbin/chown/chgrp.1:.Dd July 24, 1990

Yours,
  Ingo

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Jonathan Gray 
On Fri, Oct 27, 2017 at 02:34:32PM +0200, Mark Kettenis wrote:
> > Date: Fri, 27 Oct 2017 12:33:15 +0200
> > From: Stefan Sperling 
> > 
> > On Fri, Oct 27, 2017 at 09:08:07PM +1100, Jonathan Gray wrote:
> > > use in ifconfig was removed in 2009
> > 
> > Yes, please remove this. Thanks.
> 
> How would one go about setting QoS mode without this ioctl?

Is it needed outside of when it is implied as described in the
ifconfig commit?


revision 1.213
date: 2009/02/15 08:34:36;  author: damien;  state: Exp;  lines: +56 -35;
make "ifconfig if0 chan" list the channels supported by the device.
add "ifconfig if0 scan" to scan for access points or to list known
stations in Host AP mode.
remove the [-]wmm command while i'm here.  QoS is mandatory with
802.11n so there's not much point into making it an option.
fix parsing of the "powersave" command too.

discussed with deraadt@
man page hints from jmc@
display hints from sobrado@
"i like it" cnst@, grange@

Re: tftpd(8): diff for ip path rewrite

2017-10-27 Thread Jan Klemkow 
On Wed, Oct 25, 2017 at 04:54:01PM +, Jeremie Courreges-Anglas wrote:
> On Tue, Oct 24 2017, Jeremie Courreges-Anglas  wrote:
> > On Mon, Oct 23 2017, Jan Klemkow  wrote:
> >> On Sun, Oct 22, 2017 at 09:32:54PM +, Jeremie Courreges-Anglas wrote:
> >>> On Sat, Oct 21 2017, Jan Klemkow  wrote:
> >>> > On Fri, Oct 20, 2017 at 12:04:41PM +, Jeremie Courreges-Anglas 
> >>> > wrote:
> >>> >> On Fri, Oct 20 2017, Sebastien Marie  wrote:
> >>> >> > On Thu, Oct 19, 2017 at 08:58:12PM +0200, Jan Klemkow wrote:
> >>> >> >> +   char nfilename[PATH_MAX];
> >>> >> >> +
> >>> >> >> +   snprintf(nfilename, sizeof nfilename, "%s/%s",
> >>> >> >> +   getip(>ss), filename);
> >>> >> >
> >>> >> > - filename has PATH_MAX length
> >>> >> > - getip(>ss) could have NI_MAXHOST length
> >>> >> 
> >>> >> INET6_ADDRSTRLEN since getip() calls getnameinfo(NI_NUMERICHOST), but
> >>> >> your point stands.
> >>> >> 
> >>> >> > so nfilename could be larger than PATH_MAX (sizeof nfilename).
> >>> >> >
> >>> >> > I assume the return of snprintf() need to be checked. if truncation
> >>> >> > occured, a warning should be issued and nfilename discarded (just
> >>> >> > calling tftp_open(client, filename)) ?
> >>> >> 
> >>> >> I think we should refuse the request if truncated.
> >>> >
> >>> > done
> >>> >  
> >>> >> >> +   if (access(nfilename, R_OK) == 0)
> >>> >> >> +   tftp_open(client, nfilename);
> >>> >> >> +   else
> >>> >> >> +   tftp_open(client, filename);
> >>> >> >> +   }
> >>> >> 
> >>> >> Here we look up the same file in both the client-specific subdirectory
> >>> >> and the default directory.  Should we instead look only in the
> >>> >> client-specific directory if the latter exist?
> >>> >
> >>> > Common files should be found in the default directory.  But, host
> >>> > specific files could be overwritten if they exist in the subdirectory.
> >>> 
> >>> I think it would be better to perform those access tests in
> >>> validate_access(); logic in a single place, and a less quirky handling
> >>> of SEEDPATH.  Also the test done should probably depend on the type
> >>> (read, write) of the request.  Retrying with the default directory may
> >>> make sense in read mode, but maybe not in write (and -c, create) mode?
> >>> 
> >>> The updated diff below implements such semantics, but in
> >>> validate_access().  While here,
> >>> - improve diagnostic if both -i and -r are given; usage() doesn't show
> >>>   the conflict
> >>> - also test snprintf return value against -1, as spotted by semarie@
> >>> 
> >>> Maybe we should add a mention in the manpage that the client can
> >>> "escape" its client-specific directory?  (ie /../192.0.2.1/file)
> >>> 
> >>> The logic is more complicated but I hope it's for good.
> >>
> >> I successfully testes jca's diff in my setup and add two lines about
> >> directory escaping to the manpage.
> >
> > I don't think there is a need to expand on security and machines
> > changing their IP address, especially when you're using TFTP, an
> > insecure protocol.  I just wanted to stress that no enforcement was
> > done.
> >
> > Here's an alternate take at documenting -i, addressing a few issues. It
> > moves the "no path enforcement" sentence to CAVEATS.  I hope you agree
> > with this move.
> 
> At least jmc@ thinks that the -i flag description is a better place.
> 
> > While here:
> > - kill .Tn
> > - the content of the previous BUGS section doesn't look like a TFTP bug,
> >   so CAVEATS looks more appropriate to me
> 
> I've kept those changes (to be committed seperately).
> 
> > Feedback & oks welcome.
> 
> New diff after feedback from jmc@

I tested this diff.  It looks fine for me. 

Thanks,
Jan
 
> Index: tftpd.8
> ===
> RCS file: /d/cvs/src/usr.sbin/tftpd/tftpd.8,v
> retrieving revision 1.5
> diff -u -p -r1.5 tftpd.8
> --- tftpd.8   18 Jul 2015 05:32:56 -  1.5
> +++ tftpd.8   25 Oct 2017 16:48:32 -
> @@ -37,16 +37,14 @@
>  .Nd DARPA Trivial File Transfer Protocol daemon
>  .Sh SYNOPSIS
>  .Nm tftpd
> -.Op Fl 46cdv
> +.Op Fl 46cdiv
>  .Op Fl l Ar address
>  .Op Fl p Ar port
>  .Op Fl r Ar socket
>  .Ar directory
>  .Sh DESCRIPTION
>  .Nm
> -is a server which implements the
> -.Tn DARPA
> -Trivial File Transfer Protocol.
> +is a server which implements the DARPA Trivial File Transfer Protocol.
>  .Pp
>  The use of
>  .Xr tftp 1
> @@ -100,6 +98,15 @@ If this option is specified,
>  .Nm
>  will run in the foreground and log
>  the client IP, type of request, and filename to stderr.
> +.It Fl i
> +Look up the requested path in the subdirectory named after the
> +client's IP address.
> +For read requests, if the file is not found,
> +.Nm
> +falls back on the requested path.
> +Note that no attempt is made to limit the client to its subdirectory.
> +This option cannot be

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Mark Kettenis 
> Date: Fri, 27 Oct 2017 12:33:15 +0200
> From: Stefan Sperling 
> 
> On Fri, Oct 27, 2017 at 09:08:07PM +1100, Jonathan Gray wrote:
> > use in ifconfig was removed in 2009
> 
> Yes, please remove this. Thanks.

How would one go about setting QoS mode without this ioctl?

> > Index: ieee80211_ioctl.c
> > ===
> > RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> > retrieving revision 1.54
> > diff -u -p -r1.54 ieee80211_ioctl.c
> > --- ieee80211_ioctl.c   26 Oct 2017 15:00:28 -  1.54
> > +++ ieee80211_ioctl.c   27 Oct 2017 10:03:44 -
> > @@ -407,7 +407,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
> > int i, error = 0;
> > struct ieee80211_nwid nwid;
> > struct ieee80211_wpapsk *psk;
> > -   struct ieee80211_wmmparams *wmm;
> > struct ieee80211_keyavail *ka;
> > struct ieee80211_keyrun *kr;
> > struct ieee80211_power *power;
> > @@ -464,24 +463,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
> > break;
> > case SIOCG80211NWKEY:
> > error = ieee80211_ioctl_getnwkeys(ic, (void *)data);
> > -   break;
> > -   case SIOCS80211WMMPARMS:
> > -   if ((error = suser(curproc, 0)) != 0)
> > -   break;
> > -   if (!(ic->ic_flags & IEEE80211_C_QOS)) {
> > -   error = ENODEV;
> > -   break;
> > -   }
> > -   wmm = (struct ieee80211_wmmparams *)data;
> > -   if (wmm->i_enabled)
> > -   ic->ic_flags |= IEEE80211_F_QOS;
> > -   else
> > -   ic->ic_flags &= ~IEEE80211_F_QOS;
> > -   error = ENETRESET;
> > -   break;
> > -   case SIOCG80211WMMPARMS:
> > -   wmm = (struct ieee80211_wmmparams *)data;
> > -   wmm->i_enabled = (ic->ic_flags & IEEE80211_F_QOS) ? 1 : 0;
> > break;
> > case SIOCS80211WPAPARMS:
> > if ((error = suser(curproc, 0)) != 0)
> > Index: ieee80211_ioctl.h
> > ===
> > RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.h,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 ieee80211_ioctl.h
> > --- ieee80211_ioctl.h   24 Oct 2017 09:36:13 -  1.30
> > +++ ieee80211_ioctl.h   27 Oct 2017 10:03:44 -
> > @@ -255,15 +255,6 @@ struct ieee80211_wpaparams {
> >  #define SIOCS80211WPAPARMS  _IOW('i', 247, struct ieee80211_wpaparams)
> >  #define SIOCG80211WPAPARMS _IOWR('i', 248, struct ieee80211_wpaparams)
> >  
> > -struct ieee80211_wmmparams {
> > -   chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
> > -   int i_enabled;
> > -   /* XXX more */
> > -};
> > -
> > -#define SIOCS80211WMMPARMS  _IOW('i', 249, struct ieee80211_wmmparams)
> > -#define SIOCG80211WMMPARMS _IOWR('i', 250, struct ieee80211_wmmparams)
> > -
> >  struct ieee80211_keyavail {
> > chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
> > u_int8_ti_macaddr[IEEE80211_ADDR_LEN];
> > 
> 
>

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Kevin Lo 
On Fri, Oct 27, 2017 at 09:08:07PM +1100, Jonathan Gray wrote:
> 
> use in ifconfig was removed in 2009

sure, ok kevlo@

> Index: ieee80211_ioctl.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 ieee80211_ioctl.c
> --- ieee80211_ioctl.c 26 Oct 2017 15:00:28 -  1.54
> +++ ieee80211_ioctl.c 27 Oct 2017 10:03:44 -
> @@ -407,7 +407,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   int i, error = 0;
>   struct ieee80211_nwid nwid;
>   struct ieee80211_wpapsk *psk;
> - struct ieee80211_wmmparams *wmm;
>   struct ieee80211_keyavail *ka;
>   struct ieee80211_keyrun *kr;
>   struct ieee80211_power *power;
> @@ -464,24 +463,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   break;
>   case SIOCG80211NWKEY:
>   error = ieee80211_ioctl_getnwkeys(ic, (void *)data);
> - break;
> - case SIOCS80211WMMPARMS:
> - if ((error = suser(curproc, 0)) != 0)
> - break;
> - if (!(ic->ic_flags & IEEE80211_C_QOS)) {
> - error = ENODEV;
> - break;
> - }
> - wmm = (struct ieee80211_wmmparams *)data;
> - if (wmm->i_enabled)
> - ic->ic_flags |= IEEE80211_F_QOS;
> - else
> - ic->ic_flags &= ~IEEE80211_F_QOS;
> - error = ENETRESET;
> - break;
> - case SIOCG80211WMMPARMS:
> - wmm = (struct ieee80211_wmmparams *)data;
> - wmm->i_enabled = (ic->ic_flags & IEEE80211_F_QOS) ? 1 : 0;
>   break;
>   case SIOCS80211WPAPARMS:
>   if ((error = suser(curproc, 0)) != 0)
> Index: ieee80211_ioctl.h
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 ieee80211_ioctl.h
> --- ieee80211_ioctl.h 24 Oct 2017 09:36:13 -  1.30
> +++ ieee80211_ioctl.h 27 Oct 2017 10:03:44 -
> @@ -255,15 +255,6 @@ struct ieee80211_wpaparams {
>  #define SIOCS80211WPAPARMS_IOW('i', 247, struct ieee80211_wpaparams)
>  #define SIOCG80211WPAPARMS   _IOWR('i', 248, struct ieee80211_wpaparams)
>  
> -struct ieee80211_wmmparams {
> - chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
> - int i_enabled;
> - /* XXX more */
> -};
> -
> -#define SIOCS80211WMMPARMS_IOW('i', 249, struct ieee80211_wmmparams)
> -#define SIOCG80211WMMPARMS   _IOWR('i', 250, struct ieee80211_wmmparams)
> -
>  struct ieee80211_keyavail {
>   chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
>   u_int8_ti_macaddr[IEEE80211_ADDR_LEN];
> 
>

Re: urndis printf cleanup

2017-10-27 Thread Jeremie Courreges-Anglas 
On Fri, Oct 27 2017, Mikhail  wrote:
> On Fri, Oct 27, 2017 at 1:19 PM, Jeremie Courreges-Anglas
>  wrote:
>> On Fri, Oct 27 2017, Mikhail  wrote:
>>> add missed DEVNAME's and \n's
>>
>> They are not missing.  The point is to have _attach() print a single
>> line in dmesg.
>>
>> PS: your diff would not apply (mangled whitespace)
>>
>
> it doesn't print like a single line anyway:
>
> urndis0 at uhub3 port 2 configuration 2 interface 0 "Yota Devices LTD
> Modem YOTA 4G LTE" rev 2.00/3.34 addr 5
> urndis0: using RNDISurndis0: IOERROR
> urndis0: query failed
> : unable to get hardware address
> urndis0 detached

I see,

> patch is attached

but this may not be the best way to fix it.

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

Re: rm exits 0

2017-10-27 Thread Jason McIntyre 
On Fri, Oct 27, 2017 at 11:42:30AM +0200, Jan Stary wrote:
> "rm exits 0" does not seem right (though I'm not a native speaker).
> 
>   Jan
> 
> Index: rm.1
> ===
> RCS file: /cvs/src/bin/rm/rm.1,v
> retrieving revision 1.42
> diff -u -p -r1.42 rm.1
> --- rm.1  28 Jun 2017 06:24:39 -  1.42
> +++ rm.1  27 Oct 2017 09:41:37 -
> @@ -115,7 +115,8 @@ doing something like
>  .Sh EXIT STATUS
>  The
>  .Nm
> -utility exits 0 if all of the named files or file hierarchies were removed,
> +utility exits with 0
> +if all of the named files or file hierarchies were removed,
>  or if the
>  .Fl f
>  option was specified and all of the existing files or file hierarchies were
> 

well it is clear, so i don;t see a problem. virtually every man page
with an EXIT STATUS section uses this same standard text.

jmc

Re: elf_abi.h -> elf.h

2017-10-27 Thread Jeremie Courreges-Anglas 
On Fri, Oct 27 2017, Martin Pieuchot  wrote:
> I'd like to get rid of  which is not portable at all.
>
>  is at least present on Solaris, FreeBSD and OSX.
>
> ok?

ok

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

Re: urndis printf cleanup

2017-10-27 Thread Mikhail 
On Fri, Oct 27, 2017 at 1:19 PM, Jeremie Courreges-Anglas
 wrote:
> On Fri, Oct 27 2017, Mikhail  wrote:
>> add missed DEVNAME's and \n's
>
> They are not missing.  The point is to have _attach() print a single
> line in dmesg.
>
> PS: your diff would not apply (mangled whitespace)
>

it doesn't print like a single line anyway:

urndis0 at uhub3 port 2 configuration 2 interface 0 "Yota Devices LTD
Modem YOTA 4G LTE" rev 2.00/3.34 addr 5
urndis0: using RNDISurndis0: IOERROR
urndis0: query failed
: unable to get hardware address
urndis0 detached

patch is attached


rndis_printf.patch
Description: Binary data

Re: remove 80211WMMPARMS ioctls

2017-10-27 Thread Stefan Sperling 
On Fri, Oct 27, 2017 at 09:08:07PM +1100, Jonathan Gray wrote:
> use in ifconfig was removed in 2009

Yes, please remove this. Thanks.

> Index: ieee80211_ioctl.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 ieee80211_ioctl.c
> --- ieee80211_ioctl.c 26 Oct 2017 15:00:28 -  1.54
> +++ ieee80211_ioctl.c 27 Oct 2017 10:03:44 -
> @@ -407,7 +407,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   int i, error = 0;
>   struct ieee80211_nwid nwid;
>   struct ieee80211_wpapsk *psk;
> - struct ieee80211_wmmparams *wmm;
>   struct ieee80211_keyavail *ka;
>   struct ieee80211_keyrun *kr;
>   struct ieee80211_power *power;
> @@ -464,24 +463,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   break;
>   case SIOCG80211NWKEY:
>   error = ieee80211_ioctl_getnwkeys(ic, (void *)data);
> - break;
> - case SIOCS80211WMMPARMS:
> - if ((error = suser(curproc, 0)) != 0)
> - break;
> - if (!(ic->ic_flags & IEEE80211_C_QOS)) {
> - error = ENODEV;
> - break;
> - }
> - wmm = (struct ieee80211_wmmparams *)data;
> - if (wmm->i_enabled)
> - ic->ic_flags |= IEEE80211_F_QOS;
> - else
> - ic->ic_flags &= ~IEEE80211_F_QOS;
> - error = ENETRESET;
> - break;
> - case SIOCG80211WMMPARMS:
> - wmm = (struct ieee80211_wmmparams *)data;
> - wmm->i_enabled = (ic->ic_flags & IEEE80211_F_QOS) ? 1 : 0;
>   break;
>   case SIOCS80211WPAPARMS:
>   if ((error = suser(curproc, 0)) != 0)
> Index: ieee80211_ioctl.h
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 ieee80211_ioctl.h
> --- ieee80211_ioctl.h 24 Oct 2017 09:36:13 -  1.30
> +++ ieee80211_ioctl.h 27 Oct 2017 10:03:44 -
> @@ -255,15 +255,6 @@ struct ieee80211_wpaparams {
>  #define SIOCS80211WPAPARMS_IOW('i', 247, struct ieee80211_wpaparams)
>  #define SIOCG80211WPAPARMS   _IOWR('i', 248, struct ieee80211_wpaparams)
>  
> -struct ieee80211_wmmparams {
> - chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
> - int i_enabled;
> - /* XXX more */
> -};
> -
> -#define SIOCS80211WMMPARMS_IOW('i', 249, struct ieee80211_wmmparams)
> -#define SIOCG80211WMMPARMS   _IOWR('i', 250, struct ieee80211_wmmparams)
> -
>  struct ieee80211_keyavail {
>   chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
>   u_int8_ti_macaddr[IEEE80211_ADDR_LEN];
>

Re: urndis printf cleanup

2017-10-27 Thread Jeremie Courreges-Anglas 
On Fri, Oct 27 2017, Mikhail  wrote:
> add missed DEVNAME's and \n's

They are not missing.  The point is to have _attach() print a single
line in dmesg.

PS: your diff would not apply (mangled whitespace)

> Index: dev/usb/if_urndis.c
> ===
> RCS file: /home/misha/work/cvs/src/sys/dev/usb/if_urndis.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 if_urndis.c
> --- dev/usb/if_urndis.c 19 Jul 2017 16:31:56 - 1.67
> +++ dev/usb/if_urndis.c 27 Oct 2017 12:10:28 -
> @@ -1396,7 +1396,7 @@ urndis_attach(struct device *parent, str
>   }
>
>   uc = urndis_lookup(id);
> - printf("%s: using %s", DEVNAME(sc), uc->typestr);
> + printf("%s: using %s\n", DEVNAME(sc), uc->typestr);
>
>   id = usbd_get_interface_descriptor(sc->sc_iface_data);
>   cd = usbd_get_config_descriptor(sc->sc_udev);
> @@ -1404,7 +1404,8 @@ urndis_attach(struct device *parent, str
>
>   for (j = 0; j < altcnt; j++) {
>   if (usbd_set_interface(sc->sc_iface_data, j)) {
> - printf(": interface alternate setting %u failed\n", j);
> + printf("%s: interface alternate setting %u failed\n",
> + DEVNAME(sc), j);
>   return;
>   }
>   /* Find endpoints. */
> @@ -1414,8 +1415,8 @@ urndis_attach(struct device *parent, str
>   ed = usbd_interface2endpoint_descriptor(
>   sc->sc_iface_data, i);
>   if (!ed) {
> - printf(": no descriptor for bulk endpoint "
> - "%u\n", i);
> + printf("%s: no descriptor for bulk endpoint "
> + "%u\n", DEVNAME(sc), i);
>   return;
>   }
>   if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN &&
> @@ -1439,9 +1440,9 @@ urndis_attach(struct device *parent, str
>   }
>
>   if (sc->sc_bulkin_no == -1)
> - printf(": could not find data bulk in\n");
> + printf("%s: could not find data bulk in\n", DEVNAME(sc));
>   if (sc->sc_bulkout_no == -1 )
> - printf(": could not find data bulk out\n");
> + printf("%s: could not find data bulk out\n", DEVNAME(sc));
>   return;
>
>   found:
> @@ -1461,7 +1462,7 @@ urndis_attach(struct device *parent, str
>
>   if (urndis_ctrl_query(sc, OID_802_3_PERMANENT_ADDRESS, NULL, 0,
>   , ) != RNDIS_STATUS_SUCCESS) {
> - printf(": unable to get hardware address\n");
> + printf("%s: unable to get hardware address\n", DEVNAME(sc));
>   splx(s);
>   return;
>   }
>

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

remove 80211WMMPARMS ioctls

2017-10-27 Thread Jonathan Gray 
use in ifconfig was removed in 2009

Index: ieee80211_ioctl.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
retrieving revision 1.54
diff -u -p -r1.54 ieee80211_ioctl.c
--- ieee80211_ioctl.c   26 Oct 2017 15:00:28 -  1.54
+++ ieee80211_ioctl.c   27 Oct 2017 10:03:44 -
@@ -407,7 +407,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
int i, error = 0;
struct ieee80211_nwid nwid;
struct ieee80211_wpapsk *psk;
-   struct ieee80211_wmmparams *wmm;
struct ieee80211_keyavail *ka;
struct ieee80211_keyrun *kr;
struct ieee80211_power *power;
@@ -464,24 +463,6 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
break;
case SIOCG80211NWKEY:
error = ieee80211_ioctl_getnwkeys(ic, (void *)data);
-   break;
-   case SIOCS80211WMMPARMS:
-   if ((error = suser(curproc, 0)) != 0)
-   break;
-   if (!(ic->ic_flags & IEEE80211_C_QOS)) {
-   error = ENODEV;
-   break;
-   }
-   wmm = (struct ieee80211_wmmparams *)data;
-   if (wmm->i_enabled)
-   ic->ic_flags |= IEEE80211_F_QOS;
-   else
-   ic->ic_flags &= ~IEEE80211_F_QOS;
-   error = ENETRESET;
-   break;
-   case SIOCG80211WMMPARMS:
-   wmm = (struct ieee80211_wmmparams *)data;
-   wmm->i_enabled = (ic->ic_flags & IEEE80211_F_QOS) ? 1 : 0;
break;
case SIOCS80211WPAPARMS:
if ((error = suser(curproc, 0)) != 0)
Index: ieee80211_ioctl.h
===
RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.h,v
retrieving revision 1.30
diff -u -p -r1.30 ieee80211_ioctl.h
--- ieee80211_ioctl.h   24 Oct 2017 09:36:13 -  1.30
+++ ieee80211_ioctl.h   27 Oct 2017 10:03:44 -
@@ -255,15 +255,6 @@ struct ieee80211_wpaparams {
 #define SIOCS80211WPAPARMS  _IOW('i', 247, struct ieee80211_wpaparams)
 #define SIOCG80211WPAPARMS _IOWR('i', 248, struct ieee80211_wpaparams)
 
-struct ieee80211_wmmparams {
-   chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
-   int i_enabled;
-   /* XXX more */
-};
-
-#define SIOCS80211WMMPARMS  _IOW('i', 249, struct ieee80211_wmmparams)
-#define SIOCG80211WMMPARMS _IOWR('i', 250, struct ieee80211_wmmparams)
-
 struct ieee80211_keyavail {
chari_name[IFNAMSIZ];   /* if_name, e.g. "wi0" */
u_int8_ti_macaddr[IEEE80211_ADDR_LEN];

rm exits 0

2017-10-27 Thread Jan Stary 
"rm exits 0" does not seem right (though I'm not a native speaker).

Jan

Index: rm.1
===
RCS file: /cvs/src/bin/rm/rm.1,v
retrieving revision 1.42
diff -u -p -r1.42 rm.1
--- rm.128 Jun 2017 06:24:39 -  1.42
+++ rm.127 Oct 2017 09:41:37 -
@@ -115,7 +115,8 @@ doing something like
 .Sh EXIT STATUS
 The
 .Nm
-utility exits 0 if all of the named files or file hierarchies were removed,
+utility exits with 0
+if all of the named files or file hierarchies were removed,
 or if the
 .Fl f
 option was specified and all of the existing files or file hierarchies were

urndis printf cleanup

2017-10-27 Thread Mikhail 
add missed DEVNAME's and \n's

Index: dev/usb/if_urndis.c
===
RCS file: /home/misha/work/cvs/src/sys/dev/usb/if_urndis.c,v
retrieving revision 1.67
diff -u -p -r1.67 if_urndis.c
--- dev/usb/if_urndis.c 19 Jul 2017 16:31:56 - 1.67
+++ dev/usb/if_urndis.c 27 Oct 2017 12:10:28 -
@@ -1396,7 +1396,7 @@ urndis_attach(struct device *parent, str
  }

  uc = urndis_lookup(id);
- printf("%s: using %s", DEVNAME(sc), uc->typestr);
+ printf("%s: using %s\n", DEVNAME(sc), uc->typestr);

  id = usbd_get_interface_descriptor(sc->sc_iface_data);
  cd = usbd_get_config_descriptor(sc->sc_udev);
@@ -1404,7 +1404,8 @@ urndis_attach(struct device *parent, str

  for (j = 0; j < altcnt; j++) {
  if (usbd_set_interface(sc->sc_iface_data, j)) {
- printf(": interface alternate setting %u failed\n", j);
+ printf("%s: interface alternate setting %u failed\n",
+ DEVNAME(sc), j);
  return;
  }
  /* Find endpoints. */
@@ -1414,8 +1415,8 @@ urndis_attach(struct device *parent, str
  ed = usbd_interface2endpoint_descriptor(
  sc->sc_iface_data, i);
  if (!ed) {
- printf(": no descriptor for bulk endpoint "
- "%u\n", i);
+ printf("%s: no descriptor for bulk endpoint "
+ "%u\n", DEVNAME(sc), i);
  return;
  }
  if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN &&
@@ -1439,9 +1440,9 @@ urndis_attach(struct device *parent, str
  }

  if (sc->sc_bulkin_no == -1)
- printf(": could not find data bulk in\n");
+ printf("%s: could not find data bulk in\n", DEVNAME(sc));
  if (sc->sc_bulkout_no == -1 )
- printf(": could not find data bulk out\n");
+ printf("%s: could not find data bulk out\n", DEVNAME(sc));
  return;

  found:
@@ -1461,7 +1462,7 @@ urndis_attach(struct device *parent, str

  if (urndis_ctrl_query(sc, OID_802_3_PERMANENT_ADDRESS, NULL, 0,
  , ) != RNDIS_STATUS_SUCCESS) {
- printf(": unable to get hardware address\n");
+ printf("%s: unable to get hardware address\n", DEVNAME(sc));
  splx(s);
  return;
  }

elf_abi.h -> elf.h

2017-10-27 Thread Martin Pieuchot 
I'd like to get rid of  which is not portable at all.

 is at least present on Solaris, FreeBSD and OSX.

ok?

Index: games/hangman/ksyms.c
===
RCS file: /cvs/src/games/hangman/ksyms.c,v
retrieving revision 1.10
diff -u -p -r1.10 ksyms.c
--- games/hangman/ksyms.c   8 Jan 2016 13:40:05 -   1.10
+++ games/hangman/ksyms.c   27 Oct 2017 08:49:41 -
@@ -18,7 +18,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
Index: lib/libc/gen/nlist.c
===
RCS file: /cvs/src/lib/libc/gen/nlist.c,v
retrieving revision 1.68
diff -u -p -r1.68 nlist.c
--- lib/libc/gen/nlist.c25 Dec 2016 20:30:41 -  1.68
+++ lib/libc/gen/nlist.c27 Oct 2017 08:49:41 -
@@ -40,7 +40,7 @@
 #include 
 #include 
 #include  /* pulls in nlist.h */
-#include 
+#include 
 
 #define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
 
Index: libexec/ld.so/alpha/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/alpha/archdep.h,v
retrieving revision 1.20
diff -u -p -r1.20 archdep.h
--- libexec/ld.so/alpha/archdep.h   24 Jan 2017 07:48:37 -  1.20
+++ libexec/ld.so/alpha/archdep.h   27 Oct 2017 08:49:41 -
@@ -33,7 +33,7 @@
 
 #defineMACHID  EM_ALPHA_EXP/* ELF e_machine ID value checked */
 
-#include 
+#include 
 #include 
 #include "syscall.h"
 #include "util.h"
Index: libexec/ld.so/amd64/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/amd64/archdep.h,v
retrieving revision 1.11
diff -u -p -r1.11 archdep.h
--- libexec/ld.so/amd64/archdep.h   21 Jan 2017 01:15:00 -  1.11
+++ libexec/ld.so/amd64/archdep.h   27 Oct 2017 08:49:41 -
@@ -33,7 +33,7 @@
 
 #defineMACHID  EM_AMD64/* ELF e_machine ID value checked */
 
-#include 
+#include 
 #include 
 #include "syscall.h"
 #include "util.h"
Index: libexec/ld.so/arm/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/arm/archdep.h,v
retrieving revision 1.12
diff -u -p -r1.12 archdep.h
--- libexec/ld.so/arm/archdep.h 24 Jan 2017 07:48:37 -  1.12
+++ libexec/ld.so/arm/archdep.h 27 Oct 2017 08:49:41 -
@@ -33,7 +33,7 @@
 
 #defineMACHID  EM_ARM  /* ELF e_machine ID value checked */
 
-#include 
+#include 
 #include 
 #include "syscall.h"
 #include "util.h"
Index: libexec/ld.so/hppa/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/hppa/archdep.h,v
retrieving revision 1.15
diff -u -p -r1.15 archdep.h
--- libexec/ld.so/hppa/archdep.h24 Jan 2017 07:48:37 -  1.15
+++ libexec/ld.so/hppa/archdep.h27 Oct 2017 08:49:41 -
@@ -35,7 +35,7 @@
 
 #defineMACHID  EM_PARISC   /* ELF e_machine ID value 
checked */
 
-#include 
+#include 
 #include 
 #include "syscall.h"
 #include "util.h"
Index: libexec/ld.so/i386/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/i386/archdep.h,v
retrieving revision 1.19
diff -u -p -r1.19 archdep.h
--- libexec/ld.so/i386/archdep.h24 Jan 2017 07:48:37 -  1.19
+++ libexec/ld.so/i386/archdep.h27 Oct 2017 08:49:41 -
@@ -33,7 +33,7 @@
 
 #defineMACHID  EM_386  /* ELF e_machine ID value checked */
 
-#include 
+#include 
 #include 
 #include "syscall.h"
 #include "util.h"
Index: libexec/ld.so/ldd/ldd.c
===
RCS file: /cvs/src/libexec/ld.so/ldd/ldd.c,v
retrieving revision 1.21
diff -u -p -r1.21 ldd.c
--- libexec/ld.so/ldd/ldd.c 2 Jul 2017 19:06:12 -   1.21
+++ libexec/ld.so/ldd/ldd.c 27 Oct 2017 08:49:41 -
@@ -26,7 +26,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
Index: libexec/ld.so/m88k/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/m88k/archdep.h,v
retrieving revision 1.5
diff -u -p -r1.5 archdep.h
--- libexec/ld.so/m88k/archdep.h24 Jan 2017 07:48:37 -  1.5
+++ libexec/ld.so/m88k/archdep.h27 Oct 2017 08:49:41 -
@@ -33,7 +33,7 @@
 
 #defineMACHID  EM_88K  /* ELF e_machine ID value 
checked */
 
-#include 
+#include 
 #include 
 #include "syscall.h"
 #include "util.h"
Index: libexec/ld.so/mips64/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/mips64/archdep.h,v
retrieving revision 1.13
diff -u -p -r1.13 archdep.h
--- libexec/ld.so/mips64/archdep.h  13 Aug 2017 14:57:19 -  1.13
+++ libexec/ld.so/mips64/archdep.h  27 Oct 2017 08:49:42 -
@@ -29,7 +29,7 @@
 #ifndef _MIPS_ARCHDEP_H_
 #define _MIPS_ARCHDEP_H_
 
-#include

Re: [patch] make ifconfig report 'SIOCSIFMEDIA' when ioctl fails

2017-10-27 Thread Martin Pieuchot 
On 26/10/17(Thu) 23:41, Jesper Wallin wrote:
> Hi all,
> 
> First off, as always, I apologize if I'm wasting anyone's time because
> I'm missing something obvious here.
> 
> So, I accidentally ran "ifconfig iwm0 mode 11g" as a regular user and
> noticed it didn't throw an error.  A quick look at the code and it seems
> like the error was left out intentionally.  I consulted stsp@ on IRC
> about it and he explained that the 'media' subcommand will fail when
> running as a regular user if we error out here.  However, after
> re-adding the err(3) and doing some quick testing and trying to
> understand the code some more, things do seem to work?

Running "$ ifconfig em0 media" under ktrace(1) reveals that the only
media ioctl(2) issued is SIOCGIFMEDIA.  So there's no problem there.

> The err(3) was removed in rev 1.203 by deraadt@, about 9 years ago,
> probably because of the reasons stsp@ explained?  Could it be that the
> 'media' part has been rewritten to work even if process_media_commands()
> errors out?
> 
> Anyway, I just thought it's a bit confusing when ifconfig exits
> normally without any notices or errors, even if the command fails.

Diff is correct, ok mpi@

> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.348
> diff -u -p -r1.348 ifconfig.c
> --- ifconfig.c29 Aug 2017 21:10:20 -  1.348
> +++ ifconfig.c26 Oct 2017 19:49:44 -
> @@ -2449,7 +2449,7 @@ process_media_commands(void)
>   ifr.ifr_media = media_current;
>  
>   if (ioctl(s, SIOCSIFMEDIA, (caddr_t)) < 0)
> - ;
> + err(1, "SIOCSIFMEDIA");
>  }
>  
>  /* ARGSUSED */
>

Re: [PATCH 1/2] VMD: Require interface to be defined in switches

2017-10-27 Thread Martin Pieuchot 
On 26/10/17(Thu) 23:47, Mike Larkin wrote:
> On Fri, Oct 27, 2017 at 08:23:05AM +0200, Martin Pieuchot wrote:
> > On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> > > * Require interface name to be defined for switches in vm.conf
> > > ** Requires user to create bridge(4) beforehand
> > > * Remove code to create bridges on the fly
> > > * Add code to ensure bridge really exists
> > > * Update manpage switch and example sections
> > 
> > What's the motivation to ask people to create a bridge(4) beforehand?
> > 
> 
> This discussion came out of a previous effort to fix a bug where vmd(8) 
> doesn't
> deconfigure bridges (and/or undo changes to bridges) it made during
> initialization. After discussing this a bit with claudio, we decided that 
> rather than having vmd try to do magic stuff with creating bridges and
> tearing them down behind the scenes (including corner cases where vmd is
> killed or crashes/etc), it made sense instead to say "you create the bridge
> and tell me which one you want to use". This can easily be accomplished
> with a 1 (or 2) line /etc/hostname.bridgeX file.
> 
> We may decide later to have vmd try to create bridges again at some point
> but for now, I think this is a way to make some forward progress in this
> area.

I don't want to stop progress but I don't understand how creating a
bridge(4) beforehand help to "deconfigure" it.  Hence my question.

Re: [PATCH 1/2] VMD: Require interface to be defined in switches

2017-10-27 Thread Mike Larkin 
On Fri, Oct 27, 2017 at 08:23:05AM +0200, Martin Pieuchot wrote:
> On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> > * Require interface name to be defined for switches in vm.conf
> > ** Requires user to create bridge(4) beforehand
> > * Remove code to create bridges on the fly
> > * Add code to ensure bridge really exists
> > * Update manpage switch and example sections
> 
> What's the motivation to ask people to create a bridge(4) beforehand?
> 

This discussion came out of a previous effort to fix a bug where vmd(8) doesn't
deconfigure bridges (and/or undo changes to bridges) it made during
initialization. After discussing this a bit with claudio, we decided that 
rather than having vmd try to do magic stuff with creating bridges and
tearing them down behind the scenes (including corner cases where vmd is
killed or crashes/etc), it made sense instead to say "you create the bridge
and tell me which one you want to use". This can easily be accomplished
with a 1 (or 2) line /etc/hostname.bridgeX file.

We may decide later to have vmd try to create bridges again at some point
but for now, I think this is a way to make some forward progress in this
area.

-ml

> > diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
> > index 55a9b0c7acc..a0b21fa8ef1 100644
> > --- usr.sbin/vmd/parse.y
> > +++ usr.sbin/vmd/parse.y
> > @@ -90,7 +90,6 @@ static struct vm_create_params*vcp;
> >  static struct vmd_switch   *vsw;
> >  static struct vmd_if   *vif;
> >  static struct vmd_vm   *vm;
> > -static unsigned int vsw_unit;
> >  static char vsw_type[IF_NAMESIZE];
> >  static int  vcp_disable;
> >  static size_t   vcp_nnics;
> > @@ -194,12 +193,17 @@ switch: SWITCH string 
> > {
> > vsw->sw_id = env->vmd_nswitches + 1;
> > vsw->sw_name = $2;
> > vsw->sw_flags = VMIFF_UP;
> > -   snprintf(vsw->sw_ifname, sizeof(vsw->sw_ifname),
> > -   "%s%u", vsw_type, vsw_unit++);
> > TAILQ_INIT(>sw_ifs);
> >  
> > vcp_disable = 0;
> > } '{' optnl switch_opts_l '}'   {
> > +   if (strnlen(vsw->sw_ifname,
> > +   sizeof(vsw->sw_ifname)) == 0) {
> > +   yyerror("switch \"%s\" is missing interface 
> > name",
> > +   vsw->sw_name);
> > +   YYERROR;
> > +   }
> > +
> > if (vcp_disable) {
> > log_debug("%s:%d: switch \"%s\""
> > " skipped (disabled)",
> > @@ -244,13 +248,12 @@ switch_opts   : disable   {
> > vsw->sw_group = $2;
> > }
> > | INTERFACE string  {
> > -   if (priv_getiftype($2, vsw_type, _unit) == -1 ||
> > +   if (priv_getiftype($2, vsw_type, NULL) == -1 ||
> > priv_findname(vsw_type, vmd_descsw) == -1) {
> > yyerror("invalid switch interface: %s", $2);
> > free($2);
> > YYERROR;
> > }
> > -   vsw_unit++;
> >  
> > if (strlcpy(vsw->sw_ifname, $2,
> > sizeof(vsw->sw_ifname)) >= sizeof(vsw->sw_ifname)) {
> > diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
> > index ef42549d105..d66bdcc9b4f 100644
> > --- usr.sbin/vmd/priv.c
> > +++ usr.sbin/vmd/priv.c
> > @@ -87,8 +87,8 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, 
> > struct imsg *imsg)
> >  
> > switch (imsg->hdr.type) {
> > case IMSG_VMDOP_PRIV_IFDESCR:
> > -   case IMSG_VMDOP_PRIV_IFCREATE:
> > case IMSG_VMDOP_PRIV_IFRDOMAIN:
> > +   case IMSG_VMDOP_PRIV_IFEXISTS:
> > case IMSG_VMDOP_PRIV_IFADD:
> > case IMSG_VMDOP_PRIV_IFUP:
> > case IMSG_VMDOP_PRIV_IFDOWN:
> > @@ -118,13 +118,6 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, 
> > struct imsg *imsg)
> > if (ioctl(env->vmd_fd, SIOCSIFDESCR, ) < 0)
> > log_warn("SIOCSIFDESCR");
> > break;
> > -   case IMSG_VMDOP_PRIV_IFCREATE:
> > -   /* Create the bridge if it doesn't exist */
> > -   strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> > -   if (ioctl(env->vmd_fd, SIOCIFCREATE, ) < 0 &&
> > -   errno != EEXIST)
> > -   log_warn("SIOCIFCREATE");
> > -   break;
> > case IMSG_VMDOP_PRIV_IFRDOMAIN:
> > strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> > ifr.ifr_rdomainid = vfr.vfr_id;
> > @@ -145,6 +138,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, 
> > struct imsg *imsg)
> > errno != EEXIST)
> >

Re: [PATCH 1/2] VMD: Require interface to be defined in switches

2017-10-27 Thread Martin Pieuchot 
On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> * Require interface name to be defined for switches in vm.conf
> ** Requires user to create bridge(4) beforehand
> * Remove code to create bridges on the fly
> * Add code to ensure bridge really exists
> * Update manpage switch and example sections

What's the motivation to ask people to create a bridge(4) beforehand?

> diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
> index 55a9b0c7acc..a0b21fa8ef1 100644
> --- usr.sbin/vmd/parse.y
> +++ usr.sbin/vmd/parse.y
> @@ -90,7 +90,6 @@ static struct vm_create_params  *vcp;
>  static struct vmd_switch *vsw;
>  static struct vmd_if *vif;
>  static struct vmd_vm *vm;
> -static unsigned int   vsw_unit;
>  static char   vsw_type[IF_NAMESIZE];
>  static intvcp_disable;
>  static size_t vcp_nnics;
> @@ -194,12 +193,17 @@ switch  : SWITCH string {
>   vsw->sw_id = env->vmd_nswitches + 1;
>   vsw->sw_name = $2;
>   vsw->sw_flags = VMIFF_UP;
> - snprintf(vsw->sw_ifname, sizeof(vsw->sw_ifname),
> - "%s%u", vsw_type, vsw_unit++);
>   TAILQ_INIT(>sw_ifs);
>  
>   vcp_disable = 0;
>   } '{' optnl switch_opts_l '}'   {
> + if (strnlen(vsw->sw_ifname,
> + sizeof(vsw->sw_ifname)) == 0) {
> + yyerror("switch \"%s\" is missing interface 
> name",
> + vsw->sw_name);
> + YYERROR;
> + }
> +
>   if (vcp_disable) {
>   log_debug("%s:%d: switch \"%s\""
>   " skipped (disabled)",
> @@ -244,13 +248,12 @@ switch_opts : disable   {
>   vsw->sw_group = $2;
>   }
>   | INTERFACE string  {
> - if (priv_getiftype($2, vsw_type, _unit) == -1 ||
> + if (priv_getiftype($2, vsw_type, NULL) == -1 ||
>   priv_findname(vsw_type, vmd_descsw) == -1) {
>   yyerror("invalid switch interface: %s", $2);
>   free($2);
>   YYERROR;
>   }
> - vsw_unit++;
>  
>   if (strlcpy(vsw->sw_ifname, $2,
>   sizeof(vsw->sw_ifname)) >= sizeof(vsw->sw_ifname)) {
> diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
> index ef42549d105..d66bdcc9b4f 100644
> --- usr.sbin/vmd/priv.c
> +++ usr.sbin/vmd/priv.c
> @@ -87,8 +87,8 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct 
> imsg *imsg)
>  
>   switch (imsg->hdr.type) {
>   case IMSG_VMDOP_PRIV_IFDESCR:
> - case IMSG_VMDOP_PRIV_IFCREATE:
>   case IMSG_VMDOP_PRIV_IFRDOMAIN:
> + case IMSG_VMDOP_PRIV_IFEXISTS:
>   case IMSG_VMDOP_PRIV_IFADD:
>   case IMSG_VMDOP_PRIV_IFUP:
>   case IMSG_VMDOP_PRIV_IFDOWN:
> @@ -118,13 +118,6 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, 
> struct imsg *imsg)
>   if (ioctl(env->vmd_fd, SIOCSIFDESCR, ) < 0)
>   log_warn("SIOCSIFDESCR");
>   break;
> - case IMSG_VMDOP_PRIV_IFCREATE:
> - /* Create the bridge if it doesn't exist */
> - strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> - if (ioctl(env->vmd_fd, SIOCIFCREATE, ) < 0 &&
> - errno != EEXIST)
> - log_warn("SIOCIFCREATE");
> - break;
>   case IMSG_VMDOP_PRIV_IFRDOMAIN:
>   strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
>   ifr.ifr_rdomainid = vfr.vfr_id;
> @@ -145,6 +138,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, 
> struct imsg *imsg)
>   errno != EEXIST)
>   log_warn("SIOCBRDGADD");
>   break;
> + case IMSG_VMDOP_PRIV_IFEXISTS:
> + /* Determine if bridge/switch exists */
> + strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> + if (ioctl(env->vmd_fd, SIOCGIFFLAGS, ) < 0)
> + fatalx("%s: bridge \"%s\" does not exist",
> + __func__, vfr.vfr_name);
> + break;
>   case IMSG_VMDOP_PRIV_IFUP:
>   case IMSG_VMDOP_PRIV_IFDOWN:
>   /* Set the interface status */
> @@ -319,10 +319,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
>   log_debug("%s: interface %s add %s", __func__,
>   vfbr.vfr_name, vfbr.vfr_value);
>  
> - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFCREATE,
> - , sizeof(vfbr));
> -