Re: IPv6 Support for umb(4)

2020-02-17 Thread Claudio Jeker
On Tue, Feb 18, 2020 at 08:25:48AM +0100, Gerhard Roth wrote:
> Hi Claudio,
> 
> thanks for looking at it.
> 
> For your questions find my replies below.
> 

Thanks. Bummer about not knowing what the cause of no IPv6 config is.
I guess it is time to get this in an have people play with it.

> 
> On Mon, 17 Feb 2020 17:30:03 +0100 Claudio Jeker  
> wrote:
> > On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:
> > > Hi Alex,
> > > 
> > > thanks for looking into it.
> > > 
> > > 
> > > On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm 
> > >  wrote:  
> > > > On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:  
> > > > > this patch adds IPv6 support to umb(4).
> > > > 
> > > > It breaks my IPv4 setup.
> > > > 
> > > > umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 
> > > > 2.00/0.00 addr 2
> > > > provider Vodafone.de
> > > > firmware CXP 901 8700/1 - R3C18
> > > > 
> > > > When I apply the diff, my umb device does not get an IPv4 address.
> > > > 
> > > > umb0: state going up from 'open' to 'radio on'
> > > > umb0: none state unlocked (-1 attempts left)
> > > > umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> > > > umb0: packet service changed from detached to attaching, class none, 
> > > > speed: 0 up / 0 down
> > > > umb0: SIM initialized
> > > > umb0: state going up from 'radio on' to 'SIM is ready'
> > > > umb0: packet service changed from attaching to attached, class HSPA, 
> > > > speed: 576 up / 1440 down
> > > > umb0: state going up from 'SIM is ready' to 'attached'
> > > > umb0: connecting ...
> > > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT  
> > > 
> > > That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
> > > Well at least it gives a decent error code.
> > > 
> > >   
> > > > umb0: state change timeout
> > > > umb0: connecting ...
> > > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > > umb0: state change timeout
> > > > umb0: connecting ...
> > > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > > umb0: state change timeout
> > > > ...
> > > > 
> > > > A few comments inline.
> > > >   
> > > > > +#ifdef INET6
> > > > > +int   umb_add_inet6_config(struct umb_softc *, struct 
> > > > > in6_addr *,
> > > > > + u_int, struct in6_addr *);
> > > > > +#endif
> > > > 
> > > > Usually I avoid #ifdef for prototypes.  It does not matter whether
> > > > the compiler reads them and without #ifdef the code is nicer.  
> > > 
> > > Removed it.
> > > 
> > >   
> > > >   
> > > > > +tryv6:;
> > > > 
> > > > The ; is wrong.  
> > > 
> > > Not really, because the label 'tryv6' is immediately followed by
> > > an "#ifdef INET6". So looking just at this label I cannot directly tell
> > > whether there is code that follows or not. And a label with no code
> > > following is a syntax error in C.
> > > 
> > > I just followed a old "C Style and Coding Standards for SunOS" paper
> > > by Bill Shannon from 1993 that says:
> > > 
> > >   If a label is not followed by a program statement (e.g.
> > >   if the next token is a closing brace( } )) a NULL
> > >   statement ( ; ) must follow the label.
> > > 
> > >   
> > > >   
> > > > > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > > > > + goto done;
> > > > > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > > > + log(LOG_INFO, "%s: more than one IPv6 addr: 
> > > > > %d\n",
> > > > > + DEVNAM(ifp->if_softc), n);
> > > > > +
> > > > > + /* Only pick the first one */
> > > > > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > > > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > > > +
> > > > > + off = letoh32(ic->ipv6_gwoffs);
> > > > > + memcpy(&gw6, data + off, sizeof (gw6));
> > > > 
> > > > I think we should check the data length like above.
> > > > 
> > > > if (off + sizeof (gw6) > len)
> > > > goto done;
> > > > 
> > > > And IPv4 should get the same check.  
> > > 
> > > Thanks for spotting that. Added check to both cases.
> > > 
> > >   
> > > >   
> > > > > @@ -380,6 +381,6 @@ struct umb_softc {
> > > > >
> > > > >  #define sc_state sc_info.state
> > > > >  #define sc_roaming   sc_info.enable_roaming
> > > > > - struct umb_info sc_info;
> > > > > + struct umb_info  sc_info;
> > > > >  };
> > > > >  #endif /* _KERNEL */
> > > > 
> > > > This whitespace chunk is wrong.  
> > > 
> > > I think it was wrong before. It's common idiom to add an extra space
> > > to non-pointer members to keep the member names aligned, e.g.
> > > 
> > >   struct foo {
> > >   int *abc;
> > >   int  def;
> > >   int *bar;
> > >   };
> > > 
> > > Please correct me if I'm wrong.
> > >   
> > > > 
> > > > bluhm  
> > > 
> > > 
> > > The upda

Re: IPv6 Support for umb(4)

2020-02-17 Thread Gerhard Roth
Hi Claudio,

thanks for looking at it.

For your questions find my replies below.


On Mon, 17 Feb 2020 17:30:03 +0100 Claudio Jeker  
wrote:
> On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:
> > Hi Alex,
> > 
> > thanks for looking into it.
> > 
> > 
> > On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm  
> > wrote:  
> > > On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:  
> > > > this patch adds IPv6 support to umb(4).
> > > 
> > > It breaks my IPv4 setup.
> > > 
> > > umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 
> > > 2.00/0.00 addr 2
> > > provider Vodafone.de
> > > firmware CXP 901 8700/1 - R3C18
> > > 
> > > When I apply the diff, my umb device does not get an IPv4 address.
> > > 
> > > umb0: state going up from 'open' to 'radio on'
> > > umb0: none state unlocked (-1 attempts left)
> > > umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> > > umb0: packet service changed from detached to attaching, class none, 
> > > speed: 0 up / 0 down
> > > umb0: SIM initialized
> > > umb0: state going up from 'radio on' to 'SIM is ready'
> > > umb0: packet service changed from attaching to attached, class HSPA, 
> > > speed: 576 up / 1440 down
> > > umb0: state going up from 'SIM is ready' to 'attached'
> > > umb0: connecting ...
> > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT  
> > 
> > That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
> > Well at least it gives a decent error code.
> > 
> >   
> > > umb0: state change timeout
> > > umb0: connecting ...
> > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > umb0: state change timeout
> > > umb0: connecting ...
> > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > umb0: state change timeout
> > > ...
> > > 
> > > A few comments inline.
> > >   
> > > > +#ifdef INET6
> > > > +int umb_add_inet6_config(struct umb_softc *, struct 
> > > > in6_addr *,
> > > > +   u_int, struct in6_addr *);
> > > > +#endif
> > > 
> > > Usually I avoid #ifdef for prototypes.  It does not matter whether
> > > the compiler reads them and without #ifdef the code is nicer.  
> > 
> > Removed it.
> > 
> >   
> > >   
> > > > +tryv6:;
> > > 
> > > The ; is wrong.  
> > 
> > Not really, because the label 'tryv6' is immediately followed by
> > an "#ifdef INET6". So looking just at this label I cannot directly tell
> > whether there is code that follows or not. And a label with no code
> > following is a syntax error in C.
> > 
> > I just followed a old "C Style and Coding Standards for SunOS" paper
> > by Bill Shannon from 1993 that says:
> > 
> > If a label is not followed by a program statement (e.g.
> > if the next token is a closing brace( } )) a NULL
> > statement ( ; ) must follow the label.
> > 
> >   
> > >   
> > > > +   if (n == 0 || off + sizeof (ipv6elem) > len)
> > > > +   goto done;
> > > > +   if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > > +   log(LOG_INFO, "%s: more than one IPv6 addr: 
> > > > %d\n",
> > > > +   DEVNAM(ifp->if_softc), n);
> > > > +
> > > > +   /* Only pick the first one */
> > > > +   memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > > +   memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > > +
> > > > +   off = letoh32(ic->ipv6_gwoffs);
> > > > +   memcpy(&gw6, data + off, sizeof (gw6));
> > > 
> > > I think we should check the data length like above.
> > > 
> > >   if (off + sizeof (gw6) > len)
> > >   goto done;
> > > 
> > > And IPv4 should get the same check.  
> > 
> > Thanks for spotting that. Added check to both cases.
> > 
> >   
> > >   
> > > > @@ -380,6 +381,6 @@ struct umb_softc {
> > > >
> > > >  #define sc_state   sc_info.state
> > > >  #define sc_roaming sc_info.enable_roaming
> > > > -   struct umb_info sc_info;
> > > > +   struct umb_info  sc_info;
> > > >  };
> > > >  #endif /* _KERNEL */
> > > 
> > > This whitespace chunk is wrong.  
> > 
> > I think it was wrong before. It's common idiom to add an extra space
> > to non-pointer members to keep the member names aligned, e.g.
> > 
> > struct foo {
> > int *abc;
> > int  def;
> > int *bar;
> > };
> > 
> > Please correct me if I'm wrong.
> >   
> > > 
> > > bluhm  
> > 
> > 
> > The updated patch below introduces a UMBFLG_NO_INET6 which is set on
> > receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
> > MBIM_CID_CONNECT. The code will then retry the connect operation in
> > IPv4-only mode.
> > 
> > That won't give you any IPv6 support, but at least it won't break
> > your setup.
> >   
> 
> A few whitespace fixes and some comments inline but apart from that
> OK claudio@
> 
> > Index: sbin/ifconfig/ifconfig.c

Re: cwm(1): minor edit to keybinding

2020-02-17 Thread Ryan Lennox
Good point! I retract the diff, please ignore.

‐‐‐ Original Message ‐‐‐
On Tuesday, February 18, 2020 12:36 AM, Martijn van Duren 
 wrote:

> No, "?" is not always under "/", this is just for the US layout.
> One example that comes to mind is the Belgian AZERTY keyboard
> layout, where "?" on the same key as "," and there are probably more
> examples out there.
>
> martijn@
>
> On 2/18/20 1:56 AM, Ryan Lennox wrote:
>
> > Hi,
> > As a new cwm user, I spent several minutes trying to figure out why the
> > M-question prompt wasn't launching any programs.
> > I smacked my forehead when I realized I was just pressing M-slash, which
> > happens to display a very similar prompt.
> > Normally I wouldn't bother posting about something this simple, but I
> > later came across a thread on the OpenBSD subreddit where another user
> > was equally confused over the same issue. [1]
> > I think the confusion stems from this keybinding being the only one in
> > the manual with an implicit modifier.
> > [1] 
> > https://www.reddit.com/r/openbsd/comments/d3elv6/opening_programs_in_cwm_am_i_missing_something/
> >
> > Index: app/cwm/cwm.1
> >
> > =
> >
> > RCS file: /cvs/xenocara/app/cwm/cwm.1,v
> > retrieving revision 1.65
> > diff -u -p -r1.65 cwm.1
> > --- app/cwm/cwm.1 9 Jul 2019 21:38:44 - 1.65
> > +++ app/cwm/cwm.1 17 Feb 2020 22:29:08 -
> > @@ -136,7 +136,7 @@ Resize window by a small amount.
> > .It Ic CMS-[hjkl]
> > Resize window by a large amount; see
> > .Xr cwmrc 5 .
> > -.It Ic M-question
> > +.It Ic MS-slash
> > Spawn
> > .Dq exec program
> > dialog.




Re: pppx(4): rwsleep(9) -> rwsleep_nsec(9)

2020-02-17 Thread Claudio Jeker
On Mon, Feb 17, 2020 at 06:35:38PM -0600, Scott Cheloha wrote:
> Infinite sleep, trivial.
> 
> ok?

OK claudio@
 
> Index: net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 if_pppx.c
> --- net/if_pppx.c 28 Jan 2020 16:26:09 -  1.74
> +++ net/if_pppx.c 18 Feb 2020 00:35:08 -
> @@ -295,8 +295,8 @@ pppxread(dev_t dev, struct uio *uio, int
>  
>   NET_LOCK();
>   pxd->pxd_waiting = 1;
> - error = rwsleep(pxd, &netlock,
> - (PZERO + 1)|PCATCH, "pppxread", 0);
> + error = rwsleep_nsec(pxd, &netlock,
> + (PZERO + 1)|PCATCH, "pppxread", INFSLP);
>   NET_UNLOCK();
>   if (error != 0) {
>   return (error);
> 

-- 
:wq Claudio



Re: cwm(1): minor edit to keybinding

2020-02-17 Thread Martijn van Duren
No, "?" is not always under "/", this is just for the US layout.
One example that comes to mind is the Belgian AZERTY keyboard
layout, where "?" on the same key as "," and there are probably more
examples out there.

martijn@

On 2/18/20 1:56 AM, Ryan Lennox wrote:
> Hi,
> 
> As a new cwm user, I spent several minutes trying to figure out why the
> M-question prompt wasn't launching any programs.
> 
> I smacked my forehead when I realized I was just pressing M-slash, which
> happens to display a very similar prompt.
> 
> Normally I wouldn't bother posting about something this simple, but I
> later came across a thread on the OpenBSD subreddit where another user
> was equally confused over the same issue. [1]
> 
> I think the confusion stems from this keybinding being the only one in
> the manual with an implicit modifier.
> 
> [1] 
> https://www.reddit.com/r/openbsd/comments/d3elv6/opening_programs_in_cwm_am_i_missing_something/
> 
> 
> Index: app/cwm/cwm.1
> ===
> RCS file: /cvs/xenocara/app/cwm/cwm.1,v
> retrieving revision 1.65
> diff -u -p -r1.65 cwm.1
> --- app/cwm/cwm.1 9 Jul 2019 21:38:44 -   1.65
> +++ app/cwm/cwm.1 17 Feb 2020 22:29:08 -
> @@ -136,7 +136,7 @@ Resize window by a small amount.
>  .It Ic CMS-[hjkl]
>  Resize window by a large amount; see
>  .Xr cwmrc 5 .
> -.It Ic M-question
> +.It Ic MS-slash
>  Spawn
>  .Dq exec program
>  dialog.
> 



vmm(4): handle invalid writes to cr0 - patch

2020-02-17 Thread Adam Steen
Hi

Please see the patch below to handle invalid writes to cr0 as per the
Intel SDM Volume 3.

The 3 cases i am handling with this change are
1. CR0.PG: Setting the PG flag when the PE flag is clear causes a
   general-protection exception (#GP). (Intel SDM Volume 3abcd page 76, Section
   2.5 Control Registers)

2. CR0.CD and CR0.NW, CD: 0 and NW: 1 Invalid setting. Generates a
   general-protection exception (#GP) with an error code of 0.
   (Intel SDM Volume 3abcd page 438, Table 11-5. Cache Operating Modes,
   via Intel SDM Volume 3abcd page 76, See also comment from CR0.CD

3. Bits 63:32 of CR0 and CR4 are reserved and must be written with zeros.
   Writing a nonzero value to any of the upper 32 bits results in a
   general-protection exception, #GP(0). (Intel SDM Volume 3abcd page
   75, 2.5 Control Registers, Paragraph 2, bullet point 2.

Cheers
Adam

? div
Index: sys/arch/amd64/amd64/vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.262
diff -u -p -u -p -r1.262 vmm.c
--- sys/arch/amd64/amd64/vmm.c  17 Feb 2020 18:16:10 -  1.262
+++ sys/arch/amd64/amd64/vmm.c  18 Feb 2020 02:59:53 -
@@ -5685,6 +5685,33 @@ vmx_handle_cr0_write(struct vcpu *vcpu, 
return (EINVAL);
}
 
+   /* 2.5 CONTROL REGISTERS CR0.PG */
+   if ((r & CR0_PG) && (r & CR0_PE) == 0) {
+   DPRINTF("%s: PG flag set when the PE flag is clear,"
+   " inject #GP, cr0=0x%llx\n", __func__, r);
+   vmm_inject_gp(vcpu);
+   return (0);
+   }
+
+   /* 
+* 11.5.1 Cache Control Registers and Bits
+* Table 11-5. Cache Operating Modes
+*/
+   if ((r & CR0_NW) && (r & CR0_CD) == 0) {
+   DPRINTF("%s: NW flag set when the CD flag is clear,"
+   " inject #GP, cr0=0x%llx\n", __func__, r);
+   vmm_inject_gp(vcpu);
+   return (0);
+   }
+
+   /* 2.5 CONTROL REGISTERS */
+   if (r & 0xULL) {
+   DPRINTF("%s: setting bits 63:32 of %%cr0 is invalid,"
+   " inject #GP, cr0=0x%llx\n", __func__, r);
+   vmm_inject_gp(vcpu);
+   return (0);
+   }
+
/* CR0 must always have NE set */
r |= CR0_NE;
 



Re: apmd: fix autoaction timeout

2020-02-17 Thread Jeremie Courreges-Anglas
On Sat, Feb 15 2020, Jeremie Courreges-Anglas  wrote:
> On Fri, Feb 14 2020, Scott Cheloha  wrote:
>> On Thu, Feb 13, 2020 at 02:08:32PM +0100, Jeremie Courreges-Anglas wrote:
>>> On Wed, Feb 12 2020, Scott Cheloha  wrote:
>>> > On Wed, Feb 12, 2020 at 01:35:22PM +0100, Jeremie Courreges-Anglas wrote:
>>> >> On Wed, Feb 12 2020, Jeremie Courreges-Anglas  wrote:

[...]

>>> >> On top of the previous diff, here's a diff to block autoaction for 60
>>> >> seconds after:
>>> >> - autoaction triggers; this prevents apmd from sending multiple suspend
>>> >> requests before the system goes to sleep
>>> >> - a resume happens; this gives you 60 seconds to fetch and plug your AC
>>> >> cable if you notice you're low on power
>>> >> 
>>> >> A side effect is that apmd now ignores stale acpiac(4) data at resume
>>> >> time, so it apmd doesn't suspend the system again when you resume with
>>> >> a low battery and AC plugged.
>>> >> 
>>> >> cc'ing Scott since he has a thing for everything time-related. :)
>>> >
>>> > For the first case, is there any way you can detect that a suspend is
>>> > in-progress but not done yet?
>>> 
>>> Well, apmd could record that it asked the kernel for a suspend/hibernate
>>> and skip autoaction as long as it doesn't get a resume event.
>>
>> Hmmm.  So what happens if the suspend/hibernate fails?  Could apmd(8)
>> get stuck waiting for a resume that will never happen?
>
> Well, if suspend fails maybe there's no point in having apmd retry
> a suspend. Also what would get stuck is only the autoaction behavior,
> the rest of apmd would keep on working as usual.
>
> acpi_sleep_state seems to properly send a RESUME event even if
> suspend/hibernate fails, except in one error case.
>
> But depending on a resume event is not portable, the APM code in i386
> and loongson doesn't notify userland about resumes. Something that ought
> to be fixed.

Looks like i386 apm(4) actually sends resume events, and I teached
loongson to send an APM_NORMAL_RESUME event too.  So unthrottling
autoaction using resume events has a chance to work on all relevant
platforms.

If autoaction asks for a suspend and the suspend fails and the kernel
fails to send a resume event, autoaction will stay disabled in apmd(8).
I think that's reasonable, after all, why would a second suspend request
succeed?

The diff below (on top of -current):
- blocks autoaction after it kicks in, until a resume event is received.
  This prevents autoaction from sending multiple suspend requests before
  suspend happens, and avoids spurious suspend/resume cycles.
- blocks autoaction for 60 seconds after a resume event, so that the
  user has time to control the system / disable apmd(8) if needed, etc

Thanks for the feedback so far.
Comments, tests and oks welcome.


Index: apmd.c
===
--- apmd.c.orig
+++ apmd.c
@@ -368,18 +368,20 @@ resumed(int ctl_fd)
 }
 
 #define TIMO (10*60)   /* 10 minutes */
+#define AUTOACTION_GRACE_PERIOD (60)   /* 1mn after resume */
 
 int
 main(int argc, char *argv[])
 {
const char *fname = _PATH_APM_CTLDEV;
int ctl_fd, sock_fd, ch, suspends, standbys, hibernates, resumes;
-   int autoaction = 0;
+   int autoaction = 0, autoaction_inflight = 0;
int autolimit = 0;
int statonly = 0;
int powerstatus = 0, powerbak = 0, powerchange = 0;
int noacsleep = 0;
struct timespec ts = {TIMO, 0}, sts = {0, 0};
+   struct timespec last_resume = { 0, 0 };
struct apm_power_info pinfo;
const char *sockname = _PATH_APM_SOCKET;
const char *errstr;
@@ -566,6 +568,8 @@ main(int argc, char *argv[])
powerstatus = powerbak;
powerchange = 1;
}
+   clock_gettime(CLOCK_MONOTONIC, &last_resume);
+   autoaction_inflight = 0;
resumes++;
break;
case APM_POWER_CHANGE:
@@ -577,17 +581,30 @@ main(int argc, char *argv[])
 
if (!powerstatus && autoaction &&
autolimit > (int)pinfo.battery_life) {
+   struct timespec graceperiod, now;
+
+   graceperiod = last_resume;
+   graceperiod.tv_sec += AUTOACTION_GRACE_PERIOD;
+   clock_gettime(CLOCK_MONOTONIC, &now);
+
logmsg(LOG_NOTICE,
"estimated battery life %d%%"
-   " below configured limit %d%%",
-   pinfo.battery_life,
-   autolimit
+   " below configured limit %d%%%s%s",
+   pinfo.battery_life, autolimit,
+   !autoaction_inflight ? "" : "

cwm(1): minor edit to keybinding

2020-02-17 Thread Ryan Lennox
Hi,

As a new cwm user, I spent several minutes trying to figure out why the
M-question prompt wasn't launching any programs.

I smacked my forehead when I realized I was just pressing M-slash, which
happens to display a very similar prompt.

Normally I wouldn't bother posting about something this simple, but I
later came across a thread on the OpenBSD subreddit where another user
was equally confused over the same issue. [1]

I think the confusion stems from this keybinding being the only one in
the manual with an implicit modifier.

[1] 
https://www.reddit.com/r/openbsd/comments/d3elv6/opening_programs_in_cwm_am_i_missing_something/


Index: app/cwm/cwm.1
===
RCS file: /cvs/xenocara/app/cwm/cwm.1,v
retrieving revision 1.65
diff -u -p -r1.65 cwm.1
--- app/cwm/cwm.1   9 Jul 2019 21:38:44 -   1.65
+++ app/cwm/cwm.1   17 Feb 2020 22:29:08 -
@@ -136,7 +136,7 @@ Resize window by a small amount.
 .It Ic CMS-[hjkl]
 Resize window by a large amount; see
 .Xr cwmrc 5 .
-.It Ic M-question
+.It Ic MS-slash
 Spawn
 .Dq exec program
 dialog.



wi(4): tsleep(9) -> tsleep_nsec(9), timeout_add(9) -> timeout_add_msec(9)

2020-02-17 Thread Scott Cheloha
Convert WI_WAVELAN_RES_TIMEOUT from ticks to milliseconds and use
timeout_add_msec(9) directly.

We can also easily use tsleep_nsec(9) here instead of tsleep(9).

ok?

Index: if_wi.c
===
RCS file: /cvs/src/sys/dev/ic/if_wi.c,v
retrieving revision 1.171
diff -u -p -r1.171 if_wi.c
--- if_wi.c 31 Dec 2019 10:05:32 -  1.171
+++ if_wi.c 18 Feb 2020 00:39:16 -
@@ -1519,7 +1519,7 @@ wi_setdef(struct wi_softc *sc, struct wi
 STATIC int
 wi_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
 {
-   int s, error = 0, i, j, len;
+   int s, error = 0, i, j, len, msecs;
struct wi_softc *sc = ifp->if_softc;
struct ifreq*ifr = (struct ifreq *)data;
struct proc *p = curproc;
@@ -1857,16 +1857,16 @@ wi_ioctl(struct ifnet *ifp, u_long comma
}
sc->wi_scan_lock = 0;
timeout_set(&sc->wi_scan_timeout, wi_scan_timeout, sc);
-   len = WI_WAVELAN_RES_TIMEOUT;
+   msecs = WI_WAVELAN_RES_TIMEOUT;
if (sc->wi_flags & WI_FLAGS_BUS_USB) {
/* Use a longer timeout for wi@usb */
-   len = WI_WAVELAN_RES_TIMEOUT * 4;
+   msecs = WI_WAVELAN_RES_TIMEOUT * 4;
}
-   timeout_add(&sc->wi_scan_timeout, len);
+   timeout_add_msec(&sc->wi_scan_timeout, msecs);
 
/* Let the userspace process wait for completion */
-   error = tsleep(&sc->wi_scan_lock, PCATCH, "wiscan",
-   hz * IEEE80211_SCAN_TIMEOUT);
+   error = tsleep_nsec(&sc->wi_scan_lock, PCATCH, "wiscan",
+   SEC_TO_NSEC(IEEE80211_SCAN_TIMEOUT));
break;
case SIOCG80211ALLNODES:
{
@@ -2036,7 +2036,8 @@ wi_scan_timeout(void *arg)
if (wi_read_record(sc, (struct wi_ltv_gen *)&wreq) == 0 &&
((struct wi_scan_p2_hdr *)wreq.wi_val)->wi_reason == 0) {
/* Wait some more time for scan completion */
-   timeout_add(&sc->wi_scan_timeout, 
WI_WAVELAN_RES_TIMEOUT);
+   timeout_add_msec(&sc->wi_scan_timeout,
+   WI_WAVELAN_RES_TIMEOUT);
return;
}
}
Index: if_wi_hostap.c
===
RCS file: /cvs/src/sys/dev/ic/if_wi_hostap.c,v
retrieving revision 1.52
diff -u -p -r1.52 if_wi_hostap.c
--- if_wi_hostap.c  19 Feb 2018 08:59:52 -  1.52
+++ if_wi_hostap.c  18 Feb 2020 00:39:16 -
@@ -410,7 +410,7 @@ wihap_sta_timeout(void *v)
 
/* Add wihap timeout if we have not already done so. */
if (!timeout_pending(&whi->tmo))
-   timeout_add(&whi->tmo, hz / 10);
+   timeout_add_msec(&whi->tmo, 100);
 
splx(s);
 }
Index: if_wi_ieee.h
===
RCS file: /cvs/src/sys/dev/ic/if_wi_ieee.h,v
retrieving revision 1.30
diff -u -p -r1.30 if_wi_ieee.h
--- if_wi_ieee.h24 Aug 2014 18:01:27 -  1.30
+++ if_wi_ieee.h18 Feb 2020 00:39:16 -
@@ -674,7 +674,7 @@ struct wi_scan_res {
 #define WI_WAVELAN_RES_11M 0x6e
 
 #define WI_WAVELAN_RES_SIZE50
-#define WI_WAVELAN_RES_TIMEOUT ((hz / 10) * 2) /* 200ms */
+#define WI_WAVELAN_RES_TIMEOUT 200 /* ms */
 #define WI_WAVELAN_RES_TRIES   100
 
 struct wi_scan_p2_hdr {



pppx(4): rwsleep(9) -> rwsleep_nsec(9)

2020-02-17 Thread Scott Cheloha
Infinite sleep, trivial.

ok?

Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.74
diff -u -p -r1.74 if_pppx.c
--- net/if_pppx.c   28 Jan 2020 16:26:09 -  1.74
+++ net/if_pppx.c   18 Feb 2020 00:35:08 -
@@ -295,8 +295,8 @@ pppxread(dev_t dev, struct uio *uio, int
 
NET_LOCK();
pxd->pxd_waiting = 1;
-   error = rwsleep(pxd, &netlock,
-   (PZERO + 1)|PCATCH, "pppxread", 0);
+   error = rwsleep_nsec(pxd, &netlock,
+   (PZERO + 1)|PCATCH, "pppxread", INFSLP);
NET_UNLOCK();
if (error != 0) {
return (error);



Re: ips(4): tsleep(9) -> tsleep_nsec(9)

2020-02-17 Thread Scott Cheloha
On Sat, Jan 11, 2020 at 02:57:14AM -0600, Scott Cheloha wrote:
> The sleep duration is in milliseconds.  We can rip out the timeval and
> tvtohz(9) and use MSEC_TO_NSEC() directly instead.  I've added a local
> "msecs" variable because "timo" (a) feels a bit ambiguous and (b) is
> used with different units earlier in the same function.
> 
> [...]

Bump and rebase.

This is a straightforward ticks-to-milliseconds change.  We also kill
off one of the remaining tvtohz(9) calls.

ok?

Index: ips.c
===
RCS file: /cvs/src/sys/dev/pci/ips.c,v
retrieving revision 1.117
diff -u -p -r1.117 ips.c
--- ips.c   14 Feb 2020 18:37:03 -  1.117
+++ ips.c   18 Feb 2020 00:21:48 -
@@ -1409,8 +1409,7 @@ ips_cmd(struct ips_softc *sc, struct ips
 int
 ips_poll(struct ips_softc *sc, struct ips_ccb *ccb)
 {
-   struct timeval tv;
-   int error, timo;
+   int error, msecs, timo;
 
splassert(IPL_BIO);
 
@@ -1427,14 +1426,11 @@ ips_poll(struct ips_softc *sc, struct ip
}
} else {
/* sleep */
-   timo = ccb->c_xfer ? ccb->c_xfer->timeout : IPS_TIMEOUT;
-   tv.tv_sec = timo / 1000;
-   tv.tv_usec = (timo % 1000) * 1000;
-   timo = tvtohz(&tv);
+   msecs = ccb->c_xfer ? ccb->c_xfer->timeout : IPS_TIMEOUT;
 
-   DPRINTF(IPS_D_XFER, ("%s: ips_poll: sleep %d hz\n",
-   sc->sc_dev.dv_xname, timo));
-   tsleep(ccb, PRIBIO + 1, "ipscmd", timo);
+   DPRINTF(IPS_D_XFER, ("%s: ips_poll: sleep %d ms\n",
+   sc->sc_dev.dv_xname, msecs));
+   tsleep_nsec(ccb, PRIBIO + 1, "ipscmd", MSEC_TO_NSEC(msecs));
}
DPRINTF(IPS_D_XFER, ("%s: ips_poll: state %d\n", sc->sc_dev.dv_xname,
ccb->c_state));



Re: openssl.1: Tag command names

2020-02-17 Thread Theo Buehler
On Mon, Feb 17, 2020 at 05:19:27PM +0100, Klemens Nanni wrote:
> 
> I'd like to commit this soon, it allows me to jump to the command I'm
> looking for, e.g. ":tx509" shows me the synopsis right away.

Without tags I currently achieve pretty much the same by doing "/^X509"
or "/^S_CLIENT" etc. However, having to know the special trick for each
page (if there is one) is annoying, so I think this is an improvement.

ok



Re: ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set

2020-02-17 Thread Remi Pommarel
On Tue, Feb 18, 2020 at 09:00:13AM +1100, Damien Miller wrote:
> On Mon, 17 Feb 2020, Remi Pommarel wrote:
> 
> > When remote side fails to create tun (e.g. tun device is already opened)
> > it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
> > channel is marked dead on client side. But because tun forward channel
> > is not an interactive channel it has no cleanup callback and is kept in
> > a Zombie state until ssh is manually terminated.
> > 
> > This makes "ssh -Nw" to not fail and exit in such situation even if
> > ExitOnForwardFailure is set.
> > 
> > This patch registers a cleanup callback to tun forward channel if
> > ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
> > remote fails to established the tunnel correctly on its side.
> 
> Please try this patch. It handles tunnel forwarding failures via
> the same logic as remote forwards.

Tried it and seems to work.
Thanks.

> 
> diff --git a/clientloop.c b/clientloop.c
> index f7ce3a2..f35ccfb 100644
> --- a/clientloop.c
> +++ b/clientloop.c
> @@ -1641,7 +1641,7 @@ client_request_agent(struct ssh *ssh, const char 
> *request_type, int rchan)
>  
>  char *
>  client_request_tun_fwd(struct ssh *ssh, int tun_mode,
> -int local_tun, int remote_tun)
> +int local_tun, int remote_tun, channel_open_fn *cb, void *cbctx)
>  {
>   Channel *c;
>   int r, fd;
> @@ -1663,6 +1663,9 @@ client_request_tun_fwd(struct ssh *ssh, int tun_mode,
>   CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, "tun", 1);
>   c->datagram = 1;
>  
> + if (cb != NULL)
> + channel_register_open_confirm(ssh, c->self, cb, cbctx);
> +
>   if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_OPEN)) != 0 ||
>   (r = sshpkt_put_cstring(ssh, "t...@openssh.com")) != 0 ||
>   (r = sshpkt_put_u32(ssh, c->self)) != 0 ||
> diff --git a/clientloop.h b/clientloop.h
> index bf79c87..4604e7c 100644
> --- a/clientloop.h
> +++ b/clientloop.h
> @@ -46,7 +46,8 @@ int  client_x11_get_proto(struct ssh *, const char *, const 
> char *,
>  void  client_global_request_reply_fwd(int, u_int32_t, void *);
>  void  client_session2_setup(struct ssh *, int, int, int,
>   const char *, struct termios *, int, struct sshbuf *, char **);
> -char  *client_request_tun_fwd(struct ssh *, int, int, int);
> +char  *client_request_tun_fwd(struct ssh *, int, int, int,
> +channel_open_fn *, void *);
>  void  client_stop_mux(void);
>  
>  /* Escape filter for protocol 2 sessions */
> diff --git a/ssh.c b/ssh.c
> index 314b3c5..37a693c 100644
> --- a/ssh.c
> +++ b/ssh.c
> @@ -175,7 +175,7 @@ struct sshbuf *command;
>  int subsystem_flag = 0;
>  
>  /* # of replies received for global requests */
> -static int remote_forward_confirms_received = 0;
> +static int forward_confirms_pending = -1;
>  
>  /* mux.c */
>  extern int muxserver_sock;
> @@ -1640,6 +1640,16 @@ fork_postauth(void)
>   fatal("daemon() failed: %.200s", strerror(errno));
>  }
>  
> +static void
> +forwarding_success(void)
> +{
> + if (forward_confirms_pending > 0 && --forward_confirms_pending == 0) {
> + debug("All forwarding requests processed");
> + if (fork_after_authentication_flag)
> + fork_postauth();
> + }
> +}
> +
>  /* Callback for remote forward global requests */
>  static void
>  ssh_confirm_remote_forward(struct ssh *ssh, int type, u_int32_t seq, void 
> *ctxt)
> @@ -1699,11 +1709,7 @@ ssh_confirm_remote_forward(struct ssh *ssh, int type, 
> u_int32_t seq, void *ctxt)
>   "for listen port %d", rfwd->listen_port);
>   }
>   }
> - if (++remote_forward_confirms_received == options.num_remote_forwards) {
> - debug("All remote forwarding requests processed");
> - if (fork_after_authentication_flag)
> - fork_postauth();
> - }
> + forwarding_success();
>  }
>  
>  static void
> @@ -1720,6 +1726,19 @@ ssh_stdio_confirm(struct ssh *ssh, int id, int 
> success, void *arg)
>   fatal("stdio forwarding failed");
>  }
>  
> +static void
> +ssh_tun_confirm(struct ssh *ssh, int id, int success, void *arg)
> +{
> + if (!success) {
> + error("Tunnel forwarding failed");
> + if (options.exit_on_forward_failure)
> + cleanup_exit(255);
> + }
> +
> + debug("%s: tunnel forward established, id=%d", __func__, id);
> + forwarding_success();
> +}
> +
>  static void
>  ssh_init_stdio_forwarding(struct ssh *ssh)
>  {
> @@ -1783,32 +1802,29 @@ ssh_init_forwarding(struct ssh *ssh, char **ifname)
>   options.remote_forwards[i].connect_path :
>   options.remote_forwards[i].connect_host,
>   options.remote_forwards[i].connect_port);
> - options.remote_forwards[i].handle =
> + if ((options.remote_forwards[i].handle =
>   channel_request_remote_forwarding(ssh,

Re: openssl.1: Tag command names

2020-02-17 Thread Remi Locherer
On Mon, Feb 17, 2020 at 05:19:27PM +0100, Klemens Nanni wrote:
> 
> I'd like to commit this soon, it allows me to jump to the command I'm
> looking for, e.g. ":tx509" shows me the synopsis right away.
> 
> FWIW, some Linux distributions ship with separate manuals, e.g. x509(1SSL).
> 
> Patch was done with a VIM macro by adding a new line after each `.Sh'
> line with the respective name but lowercased, so no typos in the added
> strings.
> 
> Specifying it is required since the markup following the `.Tg' markup
> always starts with "openssl";  the tag must not include it (`.Tg'
> accepts at most one word anyway).
> 

I like the idea!

To me it would be more logical to put .Tg above .Sh, but that is a minor
thing.

> 
> Index: openssl.1
> ===
> RCS file: /cvs/src/usr.bin/openssl/openssl.1,v
> retrieving revision 1.119
> diff -u -p -U1 -r1.119 openssl.1
> --- openssl.1 16 Feb 2020 16:39:01 -  1.119
> +++ openssl.1 17 Feb 2020 16:11:22 -
> @@ -203,2 +203,3 @@ itself.
>  .Sh ASN1PARSE
> +.Tg asn1parse
>  .Bl -hang -width "openssl asn1parse"
> @@ -299,2 +300,3 @@ into a nested structure.
>  .Sh CA
> +.Tg ca
>  .Bl -hang -width "openssl ca"
> @@ -848,2 +850,3 @@ The same as
>  .Sh CIPHERS
> +.Tg ciphers
>  .Nm openssl ciphers
> @@ -880,2 +883,3 @@ but without cipher suite codes.
>  .Sh CMS
> +.Tg cms
>  .Bl -hang -width "openssl cms"
> @@ -1396,2 +1400,3 @@ is specified.
>  .Sh CRL
> +.Tg crl
>  .Bl -hang -width "openssl crl"
> @@ -1472,2 +1477,3 @@ Verify the signature on the CRL.
>  .Sh CRL2PKCS7
> +.Tg crl2pkcs7
>  .Bl -hang -width "openssl crl2pkcs7"
> @@ -1517,2 +1523,3 @@ The output format.
>  .Sh DGST
> +.Tg dgst
>  .Bl -hang -width "openssl dgst"
> @@ -1631,2 +1638,3 @@ If no files are specified then standard 
>  .Sh DHPARAM
> +.Tg dhparam
>  .Bl -hang -width "openssl dhparam"
> @@ -1707,2 +1715,3 @@ parameters are generated instead.
>  .Sh DSA
> +.Tg dsa
>  .Bl -hang -width "openssl dsa"
> @@ -1795,2 +1804,3 @@ Print the public/private key in plain te
>  .Sh DSAPARAM
> +.Tg dsaparam
>  .Bl -hang -width "openssl dsaparam"
> @@ -1847,2 +1857,3 @@ If this option is included, the input fi
>  .Sh EC
> +.Tg ec
>  .Bl -hang -width "openssl ec"
> @@ -1959,2 +1970,3 @@ Print the public/private key in plain te
>  .Sh ECPARAM
> +.Tg ecparam
>  .Bl -hang -width "openssl ecparam"
> @@ -2054,2 +2066,3 @@ Print the EC parameters in plain text.
>  .Sh ENC
> +.Tg enc
>  .Bl -hang -width "openssl enc"
> @@ -2217,2 +2230,3 @@ Print extra details about the processing
>  .Sh ERRSTR
> +.Tg errstr
>  .Nm openssl errstr
> @@ -2247,2 +2261,3 @@ Print debugging statistics about various
>  .Sh GENDSA
> +.Tg gendsa
>  .Bl -hang -width "openssl gendsa"
> @@ -2293,2 +2308,3 @@ The parameters in this file determine th
>  .Sh GENPKEY
> +.Tg genpkey
>  .Bl -hang -width "openssl genpkey"
> @@ -2397,2 +2413,3 @@ Print the private/public key in plain te
>  .Sh GENRSA
> +.Tg genrsa
>  .Bl -hang -width "openssl genrsa"
> @@ -2454,2 +2471,3 @@ The default is 2048.
>  .Sh NSEQ
> +.Tg nseq
>  .Nm openssl nseq
> @@ -2484,2 +2502,3 @@ a Netscape certificate sequence is creat
>  .Sh OCSP
> +.Tg ocsp
>  .Bl -hang -width "openssl ocsp"
> @@ -2836,2 +2855,3 @@ option.
>  .Sh PASSWD
> +.Tg passwd
>  .Bl -hang -width "openssl passwd"
> @@ -2899,2 +2919,3 @@ to each password hash.
>  .Sh PKCS7
> +.Tg pkcs7
>  .Bl -hang -width "openssl pkcs7"
> @@ -2944,2 +2965,3 @@ Print certificate details in full rather
>  .Sh PKCS8
> +.Tg pkcs8
>  .Bl -hang -width "openssl pkcs8"
> @@ -3027,2 +3049,3 @@ It is recommended that des3 is used.
>  .Sh PKCS12
> +.Tg pkcs12
>  .Bl -hang -width "openssl pkcs12"
> @@ -3244,2 +3267,3 @@ is equivalent to
>  .Sh PKEY
> +.Tg pkey
>  .Bl -hang -width "openssl pkey"
> @@ -3307,2 +3331,3 @@ even if a private key is being processed
>  .Sh PKEYPARAM
> +.Tg pkeyparam
>  .Cm openssl pkeyparam
> @@ -3332,2 +3357,3 @@ Print the parameters in plain text.
>  .Sh PKEYUTL
> +.Tg pkeyutl
>  .Bl -hang -width "openssl pkeyutl"
> @@ -3484,2 +3510,3 @@ Verify the input data and output the rec
>  .Sh PRIME
> +.Tg prime
>  .Cm openssl prime
> @@ -3528,2 +3555,3 @@ is prime.
>  .Sh RAND
> +.Tg rand
>  .Bl -hang -width "openssl rand"
> @@ -3555,2 +3583,3 @@ or standard output if not specified.
>  .Sh REQ
> +.Tg req
>  .Bl -hang -width "openssl req"
> @@ -4004,2 +4033,3 @@ Any additional fields will be treated as
>  .Sh RSA
> +.Tg rsa
>  .Bl -hang -width "openssl rsa"
> @@ -4101,2 +4131,3 @@ Print the public/private key components 
>  .Sh RSAUTL
> +.Tg rsautl
>  .Bl -hang -width "openssl rsautl"
> @@ -4175,2 +4206,3 @@ Verify the input data and output the rec
>  .Sh S_CLIENT
> +.Tg s_client
>  .Bl -hang -width "openssl s_client"
> @@ -4473,2 +4505,3 @@ will be used.
>  .Sh S_SERVER
> +.Tg s_server
>  .Bl -hang -width "openssl s_server"
> @@ -4778,2 +4811,3 @@ a certificate is requested but the clien
>  .Sh S_TIME
> +.Tg s_time
>  .Bl -hang -width "openssl s_time"
> @@ -4888,2 +

Re: ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set

2020-02-17 Thread Damien Miller
On Mon, 17 Feb 2020, Remi Pommarel wrote:

> When remote side fails to create tun (e.g. tun device is already opened)
> it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
> channel is marked dead on client side. But because tun forward channel
> is not an interactive channel it has no cleanup callback and is kept in
> a Zombie state until ssh is manually terminated.
> 
> This makes "ssh -Nw" to not fail and exit in such situation even if
> ExitOnForwardFailure is set.
> 
> This patch registers a cleanup callback to tun forward channel if
> ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
> remote fails to established the tunnel correctly on its side.

Please try this patch. It handles tunnel forwarding failures via
the same logic as remote forwards.

diff --git a/clientloop.c b/clientloop.c
index f7ce3a2..f35ccfb 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1641,7 +1641,7 @@ client_request_agent(struct ssh *ssh, const char 
*request_type, int rchan)
 
 char *
 client_request_tun_fwd(struct ssh *ssh, int tun_mode,
-int local_tun, int remote_tun)
+int local_tun, int remote_tun, channel_open_fn *cb, void *cbctx)
 {
Channel *c;
int r, fd;
@@ -1663,6 +1663,9 @@ client_request_tun_fwd(struct ssh *ssh, int tun_mode,
CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, "tun", 1);
c->datagram = 1;
 
+   if (cb != NULL)
+   channel_register_open_confirm(ssh, c->self, cb, cbctx);
+
if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_OPEN)) != 0 ||
(r = sshpkt_put_cstring(ssh, "t...@openssh.com")) != 0 ||
(r = sshpkt_put_u32(ssh, c->self)) != 0 ||
diff --git a/clientloop.h b/clientloop.h
index bf79c87..4604e7c 100644
--- a/clientloop.h
+++ b/clientloop.h
@@ -46,7 +46,8 @@ intclient_x11_get_proto(struct ssh *, const char *, const 
char *,
 voidclient_global_request_reply_fwd(int, u_int32_t, void *);
 voidclient_session2_setup(struct ssh *, int, int, int,
const char *, struct termios *, int, struct sshbuf *, char **);
-char*client_request_tun_fwd(struct ssh *, int, int, int);
+char*client_request_tun_fwd(struct ssh *, int, int, int,
+channel_open_fn *, void *);
 voidclient_stop_mux(void);
 
 /* Escape filter for protocol 2 sessions */
diff --git a/ssh.c b/ssh.c
index 314b3c5..37a693c 100644
--- a/ssh.c
+++ b/ssh.c
@@ -175,7 +175,7 @@ struct sshbuf *command;
 int subsystem_flag = 0;
 
 /* # of replies received for global requests */
-static int remote_forward_confirms_received = 0;
+static int forward_confirms_pending = -1;
 
 /* mux.c */
 extern int muxserver_sock;
@@ -1640,6 +1640,16 @@ fork_postauth(void)
fatal("daemon() failed: %.200s", strerror(errno));
 }
 
+static void
+forwarding_success(void)
+{
+   if (forward_confirms_pending > 0 && --forward_confirms_pending == 0) {
+   debug("All forwarding requests processed");
+   if (fork_after_authentication_flag)
+   fork_postauth();
+   }
+}
+
 /* Callback for remote forward global requests */
 static void
 ssh_confirm_remote_forward(struct ssh *ssh, int type, u_int32_t seq, void 
*ctxt)
@@ -1699,11 +1709,7 @@ ssh_confirm_remote_forward(struct ssh *ssh, int type, 
u_int32_t seq, void *ctxt)
"for listen port %d", rfwd->listen_port);
}
}
-   if (++remote_forward_confirms_received == options.num_remote_forwards) {
-   debug("All remote forwarding requests processed");
-   if (fork_after_authentication_flag)
-   fork_postauth();
-   }
+   forwarding_success();
 }
 
 static void
@@ -1720,6 +1726,19 @@ ssh_stdio_confirm(struct ssh *ssh, int id, int success, 
void *arg)
fatal("stdio forwarding failed");
 }
 
+static void
+ssh_tun_confirm(struct ssh *ssh, int id, int success, void *arg)
+{
+   if (!success) {
+   error("Tunnel forwarding failed");
+   if (options.exit_on_forward_failure)
+   cleanup_exit(255);
+   }
+
+   debug("%s: tunnel forward established, id=%d", __func__, id);
+   forwarding_success();
+}
+
 static void
 ssh_init_stdio_forwarding(struct ssh *ssh)
 {
@@ -1783,32 +1802,29 @@ ssh_init_forwarding(struct ssh *ssh, char **ifname)
options.remote_forwards[i].connect_path :
options.remote_forwards[i].connect_host,
options.remote_forwards[i].connect_port);
-   options.remote_forwards[i].handle =
+   if ((options.remote_forwards[i].handle =
channel_request_remote_forwarding(ssh,
-   &options.remote_forwards[i]);
-   if (options.remote_forwards[i].handle < 0) {
-   if (options.exit_on_forward_failure)
-   fatal("Could not request remote forwarding.");
-   

Part 2: reduce SCHED_LOCKing while a proc is running

2020-02-17 Thread Amit Kulkarni
Hi,

This is part 2 of previous diff sent earlier and includes it also: "sync 
quantum for a proc with its per-CPU tracking".

While a proc is running, try not to disturb its running. Only change proc 
priority just before adding to runqueue. This reduces SCHED_LOCK occurences in 
kernel by a tiny bit: between 1.5 - 2% in speedup in testing on a kernel bsd.mp 
build. If a proc runs for less than 3 ticks, then its priority is left 
undisturbed. Otherwise it is adjusted just like in schedclock() in the 
equivalent function decay_prio_ifrun().

In this diff, I made changes to non-AMD64 arch without having any access to it, 
I believe it should compile and run.

Thanks

Index: arch/alpha/alpha/interrupt.c
===
RCS file: /cvs/src/sys/arch/alpha/alpha/interrupt.c,v
retrieving revision 1.40
diff -u -p -u -p -r1.40 interrupt.c
--- arch/alpha/alpha/interrupt.c21 Jan 2017 05:42:03 -  1.40
+++ arch/alpha/alpha/interrupt.c13 Feb 2020 01:10:03 -
@@ -234,14 +234,6 @@ interrupt(unsigned long a0, unsigned lon
 * will also deal with time-of-day stuff.
 */
(*platform.clockintr)((struct clockframe *)framep);
-
-   /*
-* If it's time to call the scheduler clock,
-* do so.
-*/
-   if ((++ci->ci_schedstate.spc_schedticks & 0x3f) == 0 &&
-   schedhz != 0)
-   schedclock(ci->ci_curproc);
}
break;
 
Index: arch/sparc64/sparc64/clock.c
===
RCS file: /cvs/src/sys/arch/sparc64/sparc64/clock.c,v
retrieving revision 1.59
diff -u -p -u -p -r1.59 clock.c
--- arch/sparc64/sparc64/clock.c30 Apr 2017 16:45:45 -  1.59
+++ arch/sparc64/sparc64/clock.c13 Feb 2020 01:10:04 -
@@ -879,8 +879,6 @@ int
 schedintr(arg)
void *arg;
 {
-   if (curproc)
-   schedclock(curproc);
return (1);
 }
 
Index: kern/kern_clock.c
===
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.101
diff -u -p -u -p -r1.101 kern_clock.c
--- kern/kern_clock.c   21 Jan 2020 16:16:23 -  1.101
+++ kern/kern_clock.c   13 Feb 2020 01:10:04 -
@@ -397,14 +397,6 @@ statclock(struct clockframe *frame)
 
if (p != NULL) {
p->p_cpticks++;
-   /*
-* If no schedclock is provided, call it here at ~~12-25 Hz;
-* ~~16 Hz is best
-*/
-   if (schedhz == 0) {
-   if ((++spc->spc_schedticks & 3) == 0)
-   schedclock(p);
-   }
}
 }
 
Index: kern/sched_bsd.c
===
RCS file: /cvs/src/sys/kern/sched_bsd.c,v
retrieving revision 1.62
diff -u -p -u -p -r1.62 sched_bsd.c
--- kern/sched_bsd.c30 Jan 2020 08:51:27 -  1.62
+++ kern/sched_bsd.c13 Feb 2020 01:10:04 -
@@ -62,6 +62,7 @@ int   rrticks_init;   /* # of hardclock tic
 struct __mp_lock sched_lock;
 #endif
 
+void   decay_prio_ifrun(struct proc*);
 void   schedcpu(void *);
 uint32_t   decay_aftersleep(uint32_t, uint32_t);
 
@@ -90,21 +91,13 @@ roundrobin(struct cpu_info *ci)
 {
struct schedstate_percpu *spc = &ci->ci_schedstate;
 
-   spc->spc_rrticks = rrticks_init;
-
if (ci->ci_curproc != NULL) {
-   if (spc->spc_schedflags & SPCF_SEENRR) {
-   /*
-* The process has already been through a roundrobin
-* without switching and may be hogging the CPU.
-* Indicate that the process should yield.
-*/
-   atomic_setbits_int(&spc->spc_schedflags,
-   SPCF_SHOULDYIELD);
-   } else {
-   atomic_setbits_int(&spc->spc_schedflags,
-   SPCF_SEENRR);
-   }
+   /*
+* The thread has now completed its full time quantum
+* without being moved off the CPU and may be hogging the CPU.
+* Indicate that the process should yield.
+*/
+   atomic_setbits_int(&spc->spc_schedflags, SPCF_SHOULDYIELD);
}
 
if (spc->spc_nrun)
@@ -291,6 +284,27 @@ decay_aftersleep(uint32_t estcpu, uint32
return (newcpu);
 }
 
+/* Adjust priority depending on how much curproc actually ran */
+void
+decay_prio_ifrun(struct proc *p)
+{
+   struct schedstate_percpu *spc = &p->p_cpu->ci_schedstate;
+   if (p != spc->spc_idleproc) {
+   int j = (rrticks_init - spc->sp

ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set

2020-02-17 Thread Remi Pommarel
When remote side fails to create tun (e.g. tun device is already opened)
it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
channel is marked dead on client side. But because tun forward channel
is not an interactive channel it has no cleanup callback and is kept in
a Zombie state until ssh is manually terminated.

This makes "ssh -Nw" to not fail and exit in such situation even if
ExitOnForwardFailure is set.

This patch registers a cleanup callback to tun forward channel if
ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
remote fails to established the tunnel correctly on its side.

Index: usr.bin/ssh/clientloop.c
===
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.340
diff -u -p -r1.340 clientloop.c
--- usr.bin/ssh/clientloop.c2 Feb 2020 09:45:34 -   1.340
+++ usr.bin/ssh/clientloop.c17 Feb 2020 15:42:59 -
@@ -1673,6 +1673,11 @@ client_request_tun_fwd(struct ssh *ssh, 
(r = sshpkt_send(ssh)) != 0)
sshpkt_fatal(ssh, r, "%s: send reply", __func__);
 
+   if (options.exit_on_forward_failure) {
+   channel_register_cleanup(ssh, c->self,
+   client_channel_closed, 0);
+   }
+
return ifname;
 }
 



Re: Adjust some DLT_IEEE802_11_RADIO bpf taps

2020-02-17 Thread Stefan Sperling
On Mon, Feb 17, 2020 at 05:07:42PM +0100, Claudio Jeker wrote:
> As already done on iwm(4) and one of the athn(4), there is no need to pass
> the radio tap structure to bpf_mtap by faking up an mbuf. The code can
> just use bpf_mtap_hdr() (which does a similar dance but with far less
> memory on the stack).
> 
> There are many other wifi driver that do the same thing so I would be
> happy if somebody else would apply the same logic there too :)
> 
> I could only compile test this diff.

OK for the ar9003.c part right away. There is no way to otherwise test
it cause this code does not work with real hardware yet.

I'll check if I can easily test the others.



sync quantum for a proc with its per-CPU tracking

2020-02-17 Thread Amit Kulkarni
Hi,

This diff makes sure that a proc and its per-CPU structure which tracks when to 
schedule another proc in round-robin are in sync. No observable change in time 
spent compiling the bsd.mp kernel.

This diff in addition with another which replaces schedclock() with equivalent 
code, shaves approx 5 secs in real time from a bsd.mp kernel build.

Thanks

Index: kern/sched_bsd.c
===
RCS file: /cvs/src/sys/kern/sched_bsd.c,v
retrieving revision 1.62
diff -u -p -u -p -r1.62 sched_bsd.c
--- kern/sched_bsd.c30 Jan 2020 08:51:27 -  1.62
+++ kern/sched_bsd.c11 Feb 2020 14:00:17 -
@@ -90,21 +90,13 @@ roundrobin(struct cpu_info *ci)
 {
struct schedstate_percpu *spc = &ci->ci_schedstate;
 
-   spc->spc_rrticks = rrticks_init;
-
if (ci->ci_curproc != NULL) {
-   if (spc->spc_schedflags & SPCF_SEENRR) {
-   /*
-* The process has already been through a roundrobin
-* without switching and may be hogging the CPU.
-* Indicate that the process should yield.
-*/
-   atomic_setbits_int(&spc->spc_schedflags,
-   SPCF_SHOULDYIELD);
-   } else {
-   atomic_setbits_int(&spc->spc_schedflags,
-   SPCF_SEENRR);
-   }
+   /*
+* The process is now completing a roundrobin
+* without switching off the CPU and may be hogging the CPU.
+* Indicate that the process should yield.
+*/
+   atomic_setbits_int(&spc->spc_schedflags, SPCF_SHOULDYIELD);
}
 
if (spc->spc_nrun)
@@ -384,6 +376,16 @@ mi_switch(void)
 * scheduling flags.
 */
atomic_clearbits_int(&spc->spc_schedflags, SPCF_SWITCHCLEAR);
+
+   /* 
+* We start afresh here, sync the proc and the per-cpu state
+* to match exactly on how much time to allow the proc to run.
+* This gives a chance to a proc to get its full quantum, and
+* not worry if there is a chance to have it taken off the CPU
+* at way less than its alloted quantum or have another proc
+* take way more than its alloted quantum.
+*/
+   spc->spc_rrticks = rrticks_init;
 
nextproc = sched_chooseproc();
 
Index: sys/sched.h
===
RCS file: /cvs/src/sys/sys/sched.h,v
retrieving revision 1.56
diff -u -p -u -p -r1.56 sched.h
--- sys/sched.h 21 Oct 2019 10:24:01 -  1.56
+++ sys/sched.h 11 Feb 2020 14:00:17 -
@@ -131,9 +131,8 @@ struct cpustats {
 #ifdef _KERNEL
 
 /* spc_flags */
-#define SPCF_SEENRR 0x0001  /* process has seen roundrobin() */
 #define SPCF_SHOULDYIELD0x0002  /* process should yield the CPU */
-#define SPCF_SWITCHCLEAR(SPCF_SEENRR|SPCF_SHOULDYIELD)
+#define SPCF_SWITCHCLEAR(SPCF_SHOULDYIELD)
 #define SPCF_SHOULDHALT0x0004  /* CPU should be vacated */
 #define SPCF_HALTED0x0008  /* CPU has been halted */
 



Re: IPv6 Support for umb(4)

2020-02-17 Thread Claudio Jeker
On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:
> Hi Alex,
> 
> thanks for looking into it.
> 
> 
> On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm  
> wrote:
> > On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:
> > > this patch adds IPv6 support to umb(4).  
> > 
> > It breaks my IPv4 setup.
> > 
> > umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 
> > 2.00/0.00 addr 2
> > provider Vodafone.de
> > firmware CXP 901 8700/1 - R3C18
> > 
> > When I apply the diff, my umb device does not get an IPv4 address.
> > 
> > umb0: state going up from 'open' to 'radio on'
> > umb0: none state unlocked (-1 attempts left)
> > umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> > umb0: packet service changed from detached to attaching, class none, speed: 
> > 0 up / 0 down
> > umb0: SIM initialized
> > umb0: state going up from 'radio on' to 'SIM is ready'
> > umb0: packet service changed from attaching to attached, class HSPA, speed: 
> > 576 up / 1440 down
> > umb0: state going up from 'SIM is ready' to 'attached'
> > umb0: connecting ...
> > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> 
> That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
> Well at least it gives a decent error code.
> 
> 
> > umb0: state change timeout
> > umb0: connecting ...
> > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > umb0: state change timeout
> > umb0: connecting ...
> > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > umb0: state change timeout
> > ...
> > 
> > A few comments inline.
> > 
> > > +#ifdef INET6
> > > +int   umb_add_inet6_config(struct umb_softc *, struct 
> > > in6_addr *,
> > > + u_int, struct in6_addr *);
> > > +#endif  
> > 
> > Usually I avoid #ifdef for prototypes.  It does not matter whether
> > the compiler reads them and without #ifdef the code is nicer.
> 
> Removed it.
> 
> 
> > 
> > > +tryv6:;  
> > 
> > The ; is wrong.
> 
> Not really, because the label 'tryv6' is immediately followed by
> an "#ifdef INET6". So looking just at this label I cannot directly tell
> whether there is code that follows or not. And a label with no code
> following is a syntax error in C.
> 
> I just followed a old "C Style and Coding Standards for SunOS" paper
> by Bill Shannon from 1993 that says:
> 
>   If a label is not followed by a program statement (e.g.
>   if the next token is a closing brace( } )) a NULL
>   statement ( ; ) must follow the label.
> 
> 
> > 
> > > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > > + goto done;
> > > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > > + DEVNAM(ifp->if_softc), n);
> > > +
> > > + /* Only pick the first one */
> > > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > +
> > > + off = letoh32(ic->ipv6_gwoffs);
> > > + memcpy(&gw6, data + off, sizeof (gw6));  
> > 
> > I think we should check the data length like above.
> > 
> > if (off + sizeof (gw6) > len)
> > goto done;
> > 
> > And IPv4 should get the same check.
> 
> Thanks for spotting that. Added check to both cases.
> 
> 
> > 
> > > @@ -380,6 +381,6 @@ struct umb_softc {
> > >
> > >  #define sc_state sc_info.state
> > >  #define sc_roaming   sc_info.enable_roaming
> > > - struct umb_info sc_info;
> > > + struct umb_info  sc_info;
> > >  };
> > >  #endif /* _KERNEL */  
> > 
> > This whitespace chunk is wrong.
> 
> I think it was wrong before. It's common idiom to add an extra space
> to non-pointer members to keep the member names aligned, e.g.
> 
>   struct foo {
>   int *abc;
>   int  def;
>   int *bar;
>   };
> 
> Please correct me if I'm wrong.
> 
> > 
> > bluhm
> 
> 
> The updated patch below introduces a UMBFLG_NO_INET6 which is set on
> receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
> MBIM_CID_CONNECT. The code will then retry the connect operation in
> IPv4-only mode.
> 
> That won't give you any IPv6 support, but at least it won't break
> your setup.
> 

A few whitespace fixes and some comments inline but apart from that
OK claudio@

> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.417
> diff -u -p -u -p -r1.417 ifconfig.c
> --- sbin/ifconfig/ifconfig.c  27 Dec 2019 14:34:46 -  1.417
> +++ sbin/ifconfig/ifconfig.c  28 Jan 2020 12:16:23 -
> @@ -5666,6 +5666,7 @@ umb_status(void)
>   char apn[UMB_APN_MAXLEN+1];
>   char pn[UMB_PHONENR_MAXLEN+1];
>   int  i, n;
> + char astr[INET6_ADDRSTRLEN];
>  
>   memset((char *)&mi, 0, sizeo

openssl.1: Tag command names

2020-02-17 Thread Klemens Nanni


I'd like to commit this soon, it allows me to jump to the command I'm
looking for, e.g. ":tx509" shows me the synopsis right away.

FWIW, some Linux distributions ship with separate manuals, e.g. x509(1SSL).

Patch was done with a VIM macro by adding a new line after each `.Sh'
line with the respective name but lowercased, so no typos in the added
strings.

Specifying it is required since the markup following the `.Tg' markup
always starts with "openssl";  the tag must not include it (`.Tg'
accepts at most one word anyway).


Index: openssl.1
===
RCS file: /cvs/src/usr.bin/openssl/openssl.1,v
retrieving revision 1.119
diff -u -p -U1 -r1.119 openssl.1
--- openssl.1   16 Feb 2020 16:39:01 -  1.119
+++ openssl.1   17 Feb 2020 16:11:22 -
@@ -203,2 +203,3 @@ itself.
 .Sh ASN1PARSE
+.Tg asn1parse
 .Bl -hang -width "openssl asn1parse"
@@ -299,2 +300,3 @@ into a nested structure.
 .Sh CA
+.Tg ca
 .Bl -hang -width "openssl ca"
@@ -848,2 +850,3 @@ The same as
 .Sh CIPHERS
+.Tg ciphers
 .Nm openssl ciphers
@@ -880,2 +883,3 @@ but without cipher suite codes.
 .Sh CMS
+.Tg cms
 .Bl -hang -width "openssl cms"
@@ -1396,2 +1400,3 @@ is specified.
 .Sh CRL
+.Tg crl
 .Bl -hang -width "openssl crl"
@@ -1472,2 +1477,3 @@ Verify the signature on the CRL.
 .Sh CRL2PKCS7
+.Tg crl2pkcs7
 .Bl -hang -width "openssl crl2pkcs7"
@@ -1517,2 +1523,3 @@ The output format.
 .Sh DGST
+.Tg dgst
 .Bl -hang -width "openssl dgst"
@@ -1631,2 +1638,3 @@ If no files are specified then standard 
 .Sh DHPARAM
+.Tg dhparam
 .Bl -hang -width "openssl dhparam"
@@ -1707,2 +1715,3 @@ parameters are generated instead.
 .Sh DSA
+.Tg dsa
 .Bl -hang -width "openssl dsa"
@@ -1795,2 +1804,3 @@ Print the public/private key in plain te
 .Sh DSAPARAM
+.Tg dsaparam
 .Bl -hang -width "openssl dsaparam"
@@ -1847,2 +1857,3 @@ If this option is included, the input fi
 .Sh EC
+.Tg ec
 .Bl -hang -width "openssl ec"
@@ -1959,2 +1970,3 @@ Print the public/private key in plain te
 .Sh ECPARAM
+.Tg ecparam
 .Bl -hang -width "openssl ecparam"
@@ -2054,2 +2066,3 @@ Print the EC parameters in plain text.
 .Sh ENC
+.Tg enc
 .Bl -hang -width "openssl enc"
@@ -2217,2 +2230,3 @@ Print extra details about the processing
 .Sh ERRSTR
+.Tg errstr
 .Nm openssl errstr
@@ -2247,2 +2261,3 @@ Print debugging statistics about various
 .Sh GENDSA
+.Tg gendsa
 .Bl -hang -width "openssl gendsa"
@@ -2293,2 +2308,3 @@ The parameters in this file determine th
 .Sh GENPKEY
+.Tg genpkey
 .Bl -hang -width "openssl genpkey"
@@ -2397,2 +2413,3 @@ Print the private/public key in plain te
 .Sh GENRSA
+.Tg genrsa
 .Bl -hang -width "openssl genrsa"
@@ -2454,2 +2471,3 @@ The default is 2048.
 .Sh NSEQ
+.Tg nseq
 .Nm openssl nseq
@@ -2484,2 +2502,3 @@ a Netscape certificate sequence is creat
 .Sh OCSP
+.Tg ocsp
 .Bl -hang -width "openssl ocsp"
@@ -2836,2 +2855,3 @@ option.
 .Sh PASSWD
+.Tg passwd
 .Bl -hang -width "openssl passwd"
@@ -2899,2 +2919,3 @@ to each password hash.
 .Sh PKCS7
+.Tg pkcs7
 .Bl -hang -width "openssl pkcs7"
@@ -2944,2 +2965,3 @@ Print certificate details in full rather
 .Sh PKCS8
+.Tg pkcs8
 .Bl -hang -width "openssl pkcs8"
@@ -3027,2 +3049,3 @@ It is recommended that des3 is used.
 .Sh PKCS12
+.Tg pkcs12
 .Bl -hang -width "openssl pkcs12"
@@ -3244,2 +3267,3 @@ is equivalent to
 .Sh PKEY
+.Tg pkey
 .Bl -hang -width "openssl pkey"
@@ -3307,2 +3331,3 @@ even if a private key is being processed
 .Sh PKEYPARAM
+.Tg pkeyparam
 .Cm openssl pkeyparam
@@ -3332,2 +3357,3 @@ Print the parameters in plain text.
 .Sh PKEYUTL
+.Tg pkeyutl
 .Bl -hang -width "openssl pkeyutl"
@@ -3484,2 +3510,3 @@ Verify the input data and output the rec
 .Sh PRIME
+.Tg prime
 .Cm openssl prime
@@ -3528,2 +3555,3 @@ is prime.
 .Sh RAND
+.Tg rand
 .Bl -hang -width "openssl rand"
@@ -3555,2 +3583,3 @@ or standard output if not specified.
 .Sh REQ
+.Tg req
 .Bl -hang -width "openssl req"
@@ -4004,2 +4033,3 @@ Any additional fields will be treated as
 .Sh RSA
+.Tg rsa
 .Bl -hang -width "openssl rsa"
@@ -4101,2 +4131,3 @@ Print the public/private key components 
 .Sh RSAUTL
+.Tg rsautl
 .Bl -hang -width "openssl rsautl"
@@ -4175,2 +4206,3 @@ Verify the input data and output the rec
 .Sh S_CLIENT
+.Tg s_client
 .Bl -hang -width "openssl s_client"
@@ -4473,2 +4505,3 @@ will be used.
 .Sh S_SERVER
+.Tg s_server
 .Bl -hang -width "openssl s_server"
@@ -4778,2 +4811,3 @@ a certificate is requested but the clien
 .Sh S_TIME
+.Tg s_time
 .Bl -hang -width "openssl s_time"
@@ -4888,2 +4922,3 @@ but not transfer any payload data.
 .Sh SESS_ID
+.Tg sess_id
 .Bl -hang -width "openssl sess_id"
@@ -4980,2 +5015,3 @@ debugging purposes.
 .Sh SMIME
+.Tg smime
 .Bl -hang -width "openssl smime"
@@ -5276,2 +5312,3 @@ An error occurred writing certificates.
 .Sh SPEED
+.Tg speed
 .Bl -hang -width "openssl speed"
@@ -5313,2 +5350,3 @@ benchmarks in parallel.
 .Sh SPKAC
+.Tg spkac
 .Bl -hang -width "openssl spkac"
@@ -5374,2 +5412,3 @@ Verify the digital signature 

Adjust some DLT_IEEE802_11_RADIO bpf taps

2020-02-17 Thread Claudio Jeker
As already done on iwm(4) and one of the athn(4), there is no need to pass
the radio tap structure to bpf_mtap by faking up an mbuf. The code can
just use bpf_mtap_hdr() (which does a similar dance but with far less
memory on the stack).

There are many other wifi driver that do the same thing so I would be
happy if somebody else would apply the same logic there too :)

I could only compile test this diff.
-- 
:wq Claudio

Index: ar9003.c
===
RCS file: /cvs/src/sys/dev/ic/ar9003.c,v
retrieving revision 1.49
diff -u -p -r1.49 ar9003.c
--- ar9003.c12 Sep 2019 12:55:06 -  1.49
+++ ar9003.c17 Feb 2020 15:51:19 -
@@ -856,7 +856,6 @@ ar9003_rx_radiotap(struct athn_softc *sc
 
struct athn_rx_radiotap_header *tap = &sc->sc_rxtap;
struct ieee80211com *ic = &sc->sc_ic;
-   struct mbuf mb;
uint64_t tsf;
uint32_t tstamp;
uint8_t rate;
@@ -905,13 +904,7 @@ ar9003_rx_radiotap(struct athn_softc *sc
case 0xc: tap->wr_rate = 108; break;
}
}
-   mb.m_data = (caddr_t)tap;
-   mb.m_len = sc->sc_rxtap_len;
-   mb.m_next = m;
-   mb.m_nextpkt = NULL;
-   mb.m_type = 0;
-   mb.m_flags = 0;
-   bpf_mtap(sc->sc_drvbpf, &mb, BPF_DIRECTION_IN);
+   bpf_mtap_hdr(sc->sc_drvbpf, tap, sc->sc_rxtap_len, m, BPF_DIRECTION_IN);
 }
 #endif
 
@@ -1484,7 +1477,6 @@ ar9003_tx(struct athn_softc *sc, struct 
 #if NBPFILTER > 0
if (__predict_false(sc->sc_drvbpf != NULL)) {
struct athn_tx_radiotap_header *tap = &sc->sc_txtap;
-   struct mbuf mb;
 
tap->wt_flags = 0;
/* Use initial transmit rate. */
@@ -1496,13 +1488,8 @@ ar9003_tx(struct athn_softc *sc, struct 
ridx[0] != ATHN_RIDX_CCK1 &&
(ic->ic_flags & IEEE80211_F_SHPREAMBLE))
tap->wt_flags |= IEEE80211_RADIOTAP_F_SHORTPRE;
-   mb.m_data = (caddr_t)tap;
-   mb.m_len = sc->sc_txtap_len;
-   mb.m_next = m;
-   mb.m_nextpkt = NULL;
-   mb.m_type = 0;
-   mb.m_flags = 0;
-   bpf_mtap(sc->sc_drvbpf, &mb, BPF_DIRECTION_OUT);
+   bpf_mtap_hdr(sc->sc_drvbpf, tap, sc->sc_txtap_len, m,
+   BPF_DIRECTION_OUT);
}
 #endif
 
Index: ath.c
===
RCS file: /cvs/src/sys/dev/ic/ath.c,v
retrieving revision 1.117
diff -u -p -r1.117 ath.c
--- ath.c   12 Sep 2019 12:55:06 -  1.117
+++ ath.c   17 Feb 2020 15:50:05 -
@@ -1922,8 +1922,6 @@ ath_rx_proc(void *arg, int npending)
 
 #if NBPFILTER > 0
if (sc->sc_drvbpf) {
-   struct mbuf mb;
-
sc->sc_rxtap.wr_flags = IEEE80211_RADIOTAP_F_FCS;
sc->sc_rxtap.wr_rate =
sc->sc_hwmap[ds->ds_rxstat.rs_rate] &
@@ -1932,13 +1930,8 @@ ath_rx_proc(void *arg, int npending)
sc->sc_rxtap.wr_rssi = ds->ds_rxstat.rs_rssi;
sc->sc_rxtap.wr_max_rssi = ic->ic_max_rssi;
 
-   mb.m_data = (caddr_t)&sc->sc_rxtap;
-   mb.m_len = sc->sc_rxtap_len;
-   mb.m_next = m;
-   mb.m_nextpkt = NULL;
-   mb.m_type = 0;
-   mb.m_flags = 0;
-   bpf_mtap(sc->sc_drvbpf, &mb, BPF_DIRECTION_IN);
+   bpf_mtap_hdr(sc->sc_drvbpf, &sc->sc_rxtap,
+   sc->sc_rxtap_len, m, BPF_DIRECTION_IN);
}
 #endif
m_adj(m, -IEEE80211_CRC_LEN);
@@ -2307,8 +2300,6 @@ ath_tx_start(struct ath_softc *sc, struc
bpf_mtap(ic->ic_rawbpf, m0, BPF_DIRECTION_OUT);
 
if (sc->sc_drvbpf) {
-   struct mbuf mb;
-
sc->sc_txtap.wt_flags = 0;
if (shortPreamble)
sc->sc_txtap.wt_flags |= IEEE80211_RADIOTAP_F_SHORTPRE;
@@ -2320,13 +2311,8 @@ ath_tx_start(struct ath_softc *sc, struc
sc->sc_txtap.wt_antenna = antenna;
sc->sc_txtap.wt_hwqueue = hwqueue;
 
-   mb.m_data = (caddr_t)&sc->sc_txtap;
-   mb.m_len = sc->sc_txtap_len;
-   mb.m_next = m0;
-   mb.m_nextpkt = NULL;
-   mb.m_type = 0;
-   mb.m_flags = 0;
-   bpf_mtap(sc->sc_drvbpf, &mb, BPF_DIRECTION_OUT);
+   bpf_mtap_hdr(sc->sc_drvbpf, &sc->sc_txtap, sc->sc_txtap_len,
+   m0, BPF_DIRECTION_OUT);
}
 #endif
 
Index: rt2560.c
===
RCS file: /cvs/src/sys/dev/ic/rt2560.c,v
retrieving revision 1.85
diff -u -p -r1.85 rt2560.c
--- rt2560.c12 Sep 2019 12:55:07 -  1.85
+++ rt2560.c17 Feb 2020 16:00:23 -00

net80211: fix mbuf corruption in AP mode

2020-02-17 Thread Stefan Sperling
I have debugged an mbuf corruption issue which can occur in net80211
hostap mode, with help from claudio, kettenis, and dlg.

The gist of the fix is this:

m = ieee80211_getmgmt(M_DONTWAIT, MT_DATA,
8 + 2 + 2 +
-   2 + ni->ni_esslen +
+   2 + ic->ic_bss->ni_esslen +

This code allocates a frame for a probe response the AP will send to an
interested client. When sizing this allocation the old code used the SSID
length stored in the node structure which represents the client. Later on
ic_bss->ni_esslen (in the AP's own node structure) is used when copying
the AP's SSID into the probe response frame.
If this length is longer than ni->ni_esslen we end up writing past the
memory buffer, which can result in corruption of an adjacent mbuf on the
free list. Then bad things happen later on when the adjacent mbuf is used.

To prevent such a mistake from occuring again I am removing 'ni' from
this function altogether. All information in the probe response should
be based on local parameters of the AP (stored in ic_bss).

Ok?

This bug is likely the reason for several past crash reports, including
https://marc.info/?l=openbsd-bugs&m=154729090512697&w=2
and https://marc.info/?l=openbsd-bugs&m=155916096205470&w=2


Index: ieee80211_output.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v
retrieving revision 1.126
diff -u -p -r1.126 ieee80211_output.c
--- ieee80211_output.c  29 Jul 2019 10:50:09 -  1.126
+++ ieee80211_output.c  17 Feb 2020 15:27:27 -
@@ -73,8 +73,7 @@ structmbuf *ieee80211_getmgmt(int, int,
 struct mbuf *ieee80211_get_probe_req(struct ieee80211com *,
struct ieee80211_node *);
 #ifndef IEEE80211_STA_ONLY
-struct mbuf *ieee80211_get_probe_resp(struct ieee80211com *,
-   struct ieee80211_node *);
+struct mbuf *ieee80211_get_probe_resp(struct ieee80211com *);
 #endif
 struct mbuf *ieee80211_get_auth(struct ieee80211com *,
struct ieee80211_node *, u_int16_t, u_int16_t);
@@ -1239,7 +1238,7 @@ ieee80211_get_probe_req(struct ieee80211
  * [tlv] HT Operation (802.11n)
  */
 struct mbuf *
-ieee80211_get_probe_resp(struct ieee80211com *ic, struct ieee80211_node *ni)
+ieee80211_get_probe_resp(struct ieee80211com *ic)
 {
const struct ieee80211_rateset *rs = &ic->ic_bss->ni_rates;
struct mbuf *m;
@@ -1247,7 +1246,7 @@ ieee80211_get_probe_resp(struct ieee8021
 
m = ieee80211_getmgmt(M_DONTWAIT, MT_DATA,
8 + 2 + 2 +
-   2 + ni->ni_esslen +
+   2 + ic->ic_bss->ni_esslen +
2 + min(rs->rs_nrates, IEEE80211_RATE_SIZE) +
2 + 1 +
((ic->ic_opmode == IEEE80211_M_IBSS) ? 2 + 2 : 0) +
@@ -1268,13 +1267,13 @@ ieee80211_get_probe_resp(struct ieee8021
frm = mtod(m, u_int8_t *);
memset(frm, 0, 8); frm += 8;/* timestamp is set by hardware */
LE_WRITE_2(frm, ic->ic_bss->ni_intval); frm += 2;
-   frm = ieee80211_add_capinfo(frm, ic, ni);
+   frm = ieee80211_add_capinfo(frm, ic, ic->ic_bss);
frm = ieee80211_add_ssid(frm, ic->ic_bss->ni_essid,
ic->ic_bss->ni_esslen);
frm = ieee80211_add_rates(frm, rs);
-   frm = ieee80211_add_ds_params(frm, ic, ni);
+   frm = ieee80211_add_ds_params(frm, ic, ic->ic_bss);
if (ic->ic_opmode == IEEE80211_M_IBSS)
-   frm = ieee80211_add_ibss_params(frm, ni);
+   frm = ieee80211_add_ibss_params(frm, ic->ic_bss);
if (ic->ic_curmode == IEEE80211_MODE_11G)
frm = ieee80211_add_erp(frm, ic);
if (rs->rs_nrates > IEEE80211_RATE_SIZE)
@@ -1777,7 +1776,7 @@ ieee80211_send_mgmt(struct ieee80211com 
break;
 #ifndef IEEE80211_STA_ONLY
case IEEE80211_FC0_SUBTYPE_PROBE_RESP:
-   if ((m = ieee80211_get_probe_resp(ic, ni)) == NULL)
+   if ((m = ieee80211_get_probe_resp(ic)) == NULL)
senderr(ENOMEM, is_tx_nombuf);
break;
 #endif