Re: libkvm requires kvm_getargv before kvm_getenv when both needed

[Theo de Raadt]

ktrace makes the problem more clear:

 25908 ps   CALL  
sysctl(1.55.75675.1,0xed0cc78,0x7f7cd3d8,0,0)
 25908 ps   RET   sysctl -1 errno 14 Bad address

Re: Push the netlock down in in_control()

[Martin Pieuchot]

On 01/05/18(Tue) 19:08, Theo Buehler wrote:
> On Mon, Apr 30, 2018 at 02:55:21PM +0200, Martin Pieuchot wrote:
> > On 30/04/18(Mon) 12:00, Theo Buehler wrote:
> > > With mpi's encouragement and guidance, here's a diff that reduces the
> > > scope of the NET_LOCK() a bit.
> > > 
> > > in_control() is the only caller of mrt_ioctl() and the latter is a
> > > simple function only requiring a read lock.
> > > 
> > > There are only a handful callers of in_ioctl(). The two switches create
> > > relatively tangled codepaths, so this will need some refactoring before
> > > we can push the lock further down and split the read and write cases.
> > > For now, establish a single exit point, grab the netlock at the
> > > beginning and release it at the end.
> > 
> > That makes sense, the same could be done for in6_ioctl().
> 
> Here's the corresponding diff for in6_control().
> 
> Push the diff down into mrt6_ioctl() and in6_ioctl(). Much like the IPv4
> version, mrt6_ioctl() only needs a read lock.
> 
> in6_ioctl() is a bit messier. It starts off with a call to nd6_ioctl(),
> which only needs a read lock. In the diff, I pushed the NET_RLOCK() down
> into nd6_ioctl(), but perhaps it would be better to keep the locking
> inside in6_ioctl()

No!  It's much better to push it down.  We must push it down everywhere
until it dies. 

Diff is ok mpi@

libkvm requires kvm_getargv before kvm_getenv when both needed

[Vadim Zhukov]

Hi all.

So I finally got bored of ps not displaying command args when "-e" is
present. Yes, ps(1) is broken: compare end of lines in output of "ps
-ww" and "ps -eww". And IIRC it behaves this way long enough, but I
always thought that it's me not missing something in ps(1) manual. Bad
zhuk@.

This is not a ps(1) bug, though: the simple diff below "fixes" it.
Yep, calling kvm_getargv(3) before kvm_getenv(3) makes everyone happy
again.

I've tried to dive into libkvm but went out of oxygen. The only
problem I found was misuse of reallocarray(), to be addressed in
another letter.

So, does any libkvm hacker have any clues where to look for this
argv-envp bug? I'm not sure that I'll find the root issue myself fast
enough (in less than half a year).

--
  WBR,
  Vadim Zhukov


Index: print.c
===
RCS file: /cvs/src/bin/ps/print.c,v
retrieving revision 1.69
diff -u -p -r1.69 print.c
--- print.c 8 Sep 2016 15:11:29 -   1.69
+++ print.c 1 May 2018 18:29:52 -
@@ -118,6 +118,7 @@ command(const struct kinfo_proc *kp, VAR
left = INT_MAX;

if (needenv && kd != NULL) {
+   argv = kvm_getargv(kd, kp, termwidth);
argv = kvm_getenvv(kd, kp, termwidth);
if ((p = argv) != NULL) {
while (*p) {

wrong realloc in libkvm

[Vadim Zhukov]

Hi all.

The "p = realloc(p, size)" idiom is obviously wrong. Since both
places to be fixed were similar, I went further and unified the code
as well.

Note that "argv" is later initialized from kd->argv, so there is
no problem in reusing it here.

The amd64 is happy. Okay?

--
WBR,
  Vadim Zhukov


Index: kvm_proc.c
===
RCS file: /cvs/src/lib/libkvm/kvm_proc.c,v
retrieving revision 1.58
diff -u -p -r1.58 kvm_proc.c
--- kvm_proc.c  7 Nov 2016 00:26:33 -   1.58
+++ kvm_proc.c  1 May 2018 18:41:59 -
@@ -262,6 +262,7 @@ kvm_argv(kvm_t *kd, const struct kinfo_p
char *np, *cp, *ep, *ap, **argv;
u_long oaddr = -1;
int len, cc;
+   size_t argc;
 
/*
 * Check that there aren't an unreasonable number of arguments,
@@ -270,22 +271,19 @@ kvm_argv(kvm_t *kd, const struct kinfo_p
if (narg > ARG_MAX || addr < VM_MIN_ADDRESS || addr >= 
VM_MAXUSER_ADDRESS)
return (0);
 
-   if (kd->argv == 0) {
-   /*
-* Try to avoid reallocs.
-*/
-   kd->argc = MAX(narg + 1, 32);
-   kd->argv = _kvm_reallocarray(kd, NULL, kd->argc,
-   sizeof(*kd->argv));
-   if (kd->argv == 0)
-   return (0);
-   } else if (narg + 1 > kd->argc) {
-   kd->argc = MAX(2 * kd->argc, narg + 1);
-   kd->argv = (char **)_kvm_reallocarray(kd, kd->argv, kd->argc,
-   sizeof(*kd->argv));
-   if (kd->argv == 0)
-   return (0);
-   }
+   if (kd->argv == 0)
+   argc = MAX(narg + 1, 32);
+   else if (narg + 1 > kd->argc)
+   argc = MAX(2 * kd->argc, narg + 1);
+   else
+   goto argv_allocated;
+   argv = _kvm_reallocarray(kd, kd->argv, argc, sizeof(*kd->argv));
+   if (argv == 0)
+   return (0);
+   kd->argv = argv;
+   kd->argc = argc;
+
+argv_allocated:
if (kd->argspc == 0) {
kd->argspc = _kvm_malloc(kd, kd->nbpg);
if (kd->argspc == 0)

Add 1500000 baud entry to gettytab(5)

[Mark Kettenis]

The various Rockchip SoCs seem to favour 150 for the serial
console speed.  And since for some of those we still have to rely on
closed source firmware components, I think I'll have to bite the
bullit and properly support this.  Here is a first diff that adds an
appropriate entry to gettytab(5).  Not entirely sure whether the
150-baud alias is necessary.  But I don't think it'll hurt us.

ok?


Index: etc/gettytab
===
RCS file: /cvs/src/etc/gettytab,v
retrieving revision 1.5
diff -u -p -r1.5 gettytab
--- etc/gettytab10 Dec 2013 20:56:59 -  1.5
+++ etc/gettytab1 May 2018 18:51:45 -
@@ -47,6 +47,8 @@ std.57600|57600-baud:\
:sp#57600:
 std.115200|115200-baud:\
:sp#115200:
+std.150|150-baud:\
+   :sp#150:
 
 #
 # Dial in rotary tables, speed selection via 'break'

Re: Push the netlock down in in_control()

[Florian Obser]

looks reasonable to me.
OK

On Tue, May 01, 2018 at 07:08:59PM +0200, Theo Buehler wrote:
> On Mon, Apr 30, 2018 at 02:55:21PM +0200, Martin Pieuchot wrote:
> > On 30/04/18(Mon) 12:00, Theo Buehler wrote:
> > > With mpi's encouragement and guidance, here's a diff that reduces the
> > > scope of the NET_LOCK() a bit.
> > > 
> > > in_control() is the only caller of mrt_ioctl() and the latter is a
> > > simple function only requiring a read lock.
> > > 
> > > There are only a handful callers of in_ioctl(). The two switches create
> > > relatively tangled codepaths, so this will need some refactoring before
> > > we can push the lock further down and split the read and write cases.
> > > For now, establish a single exit point, grab the netlock at the
> > > beginning and release it at the end.
> > 
> > That makes sense, the same could be done for in6_ioctl().
> 
> Here's the corresponding diff for in6_control().
> 
> Push the diff down into mrt6_ioctl() and in6_ioctl(). Much like the IPv4
> version, mrt6_ioctl() only needs a read lock.
> 
> in6_ioctl() is a bit messier. It starts off with a call to nd6_ioctl(),
> which only needs a read lock. In the diff, I pushed the NET_RLOCK() down
> into nd6_ioctl(), but perhaps it would be better to keep the locking
> inside in6_ioctl(), like this:
> 
>   switch (cmd) {
>   case SIOCGIFINFO_IN6:
>   case SIOCGNBRINFO_IN6:
>   NET_RLOCK();
>   error = nd6_ioctl(cmd, data, ifp);
>   NET_RUNLOCK();
>   return error;
>   }
> 
> I tested this as well as I could, but I don't usually use IPv6, so I
> dabbled a bit with it on my home network and I tried to use the netinet6
> regress tests, only with moderate success.
> 
> A test from people who actually use IPv6 would be welcome.
> 
> Index: sys/netinet6/in6.c
> ===
> RCS file: /var/cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.222
> diff -u -p -r1.222 in6.c
> --- sys/netinet6/in6.c24 Apr 2018 19:53:38 -  1.222
> +++ sys/netinet6/in6.c1 May 2018 09:14:07 -
> @@ -183,7 +183,6 @@ in6_control(struct socket *so, u_long cm
>   int privileged;
>   int error;
>  
> - NET_LOCK();
>   privileged = 0;
>   if ((so->so_state & SS_PRIV) != 0)
>   privileged++;
> @@ -200,7 +199,6 @@ in6_control(struct socket *so, u_long cm
>   break;
>   }
>  
> - NET_UNLOCK();
>   return error;
>  }
>  
> @@ -211,12 +209,11 @@ in6_ioctl(u_long cmd, caddr_t data, stru
>   struct  in6_ifaddr *ia6 = NULL;
>   struct  in6_aliasreq *ifra = (struct in6_aliasreq *)data;
>   struct sockaddr_in6 *sa6;
> + int error = 0;
>  
>   if (ifp == NULL)
>   return (ENXIO);
>  
> - NET_ASSERT_LOCKED();
> -
>   switch (cmd) {
>   case SIOCGIFINFO_IN6:
>   case SIOCGNBRINFO_IN6:
> @@ -252,13 +249,16 @@ in6_ioctl(u_long cmd, caddr_t data, stru
>   case SIOCSIFNETMASK:
>   /*
>* Do not pass those ioctl to driver handler since they are not
> -  * properly setup. Instead just error out.
> +  * properly set up. Instead just error out.
>*/
>   return (EINVAL);
>   default:
>   sa6 = NULL;
>   break;
>   }
> +
> + NET_LOCK();
> +
>   if (sa6 && sa6->sin6_family == AF_INET6) {
>   if (IN6_IS_ADDR_LINKLOCAL(>sin6_addr)) {
>   if (sa6->sin6_addr.s6_addr16[1] == 0) {
> @@ -267,12 +267,15 @@ in6_ioctl(u_long cmd, caddr_t data, stru
>   htons(ifp->if_index);
>   } else if (sa6->sin6_addr.s6_addr16[1] !=
>   htons(ifp->if_index)) {
> - return (EINVAL);/* link ID contradicts 
> */
> + error = EINVAL; /* link ID contradicts */
> + goto err;
>   }
>   if (sa6->sin6_scope_id) {
>   if (sa6->sin6_scope_id !=
> - (u_int32_t)ifp->if_index)
> - return (EINVAL);
> + (u_int32_t)ifp->if_index) {
> + error = EINVAL;
> + goto err;
> + }
>   sa6->sin6_scope_id = 0; /* XXX: good way? */
>   }
>   }
> @@ -289,8 +292,10 @@ in6_ioctl(u_long cmd, caddr_t data, stru
>* interface address from the day one, we consider "remove the
>* first one" semantics to be not preferable.
>*/
> - if (ia6 == NULL)
> - return (EADDRNOTAVAIL);
> + if (ia6 == NULL) {
> + error = EADDRNOTAVAIL;
> + 

Re: libkvm requires kvm_getargv before kvm_getenv when both needed

[Theo de Raadt]

Vadim Zhukov  wrote:

> 2018-05-01 21:53 GMT+03:00 Theo de Raadt :
> > ktrace makes the problem more clear:
> > 
> >  25908 ps   CALL  
> > sysctl(1.55.75675.1,0xed0cc78,0x7f7cd3d8,0,0)
> >  25908 ps   RET   sysctl -1 errno 14 Bad address
> 
> And that's it, thanks!
> 
> Now little ps(1) is happy. But now there's a question, about
> kvm_getargv() and kvm_getenv(): what behaviour do we want?
> 
>   a) They use same space, overwriting each other results (this is what
>  happens now, and noone complains).
>
>   b) Their working space should be independent of each other. This
>  isn't hard, just splitting kd->argbuf into kd->argbuf and
>  kd->envbuf. Seems a bit saner.
>
> I'm fine with any direction. The patch below implements (a), since
> it's less patching. Is it okay, or should it be (b)?

I think (b) would be the better solution, this seems very fragile.

Todd and Guenther -- what do you think?

Re: libkvm requires kvm_getargv before kvm_getenv when both needed

[Vadim Zhukov]

2018-05-01 21:53 GMT+03:00 Theo de Raadt :
> ktrace makes the problem more clear:
> 
>  25908 ps   CALL  
> sysctl(1.55.75675.1,0xed0cc78,0x7f7cd3d8,0,0)
>  25908 ps   RET   sysctl -1 errno 14 Bad address

And that's it, thanks!

Now little ps(1) is happy. But now there's a question, about
kvm_getargv() and kvm_getenv(): what behaviour do we want?

  a) They use same space, overwriting each other results (this is what
 happens now, and noone complains).

  b) Their working space should be independent of each other. This
 isn't hard, just splitting kd->argbuf into kd->argbuf and
 kd->envbuf. Seems a bit saner.

I'm fine with any direction. The patch below implements (a), since
it's less patching. Is it okay, or should it be (b)?

--
WBR,
  Vadim Zhukov


Index: kvm_proc.c
===
RCS file: /cvs/src/lib/libkvm/kvm_proc.c,v
retrieving revision 1.58
diff -u -p -r1.58 kvm_proc.c
--- kvm_proc.c  7 Nov 2016 00:26:33 -   1.58
+++ kvm_proc.c  1 May 2018 19:23:01 -
@@ -458,12 +458,14 @@ kvm_arg_sysctl(kvm_t *kd, pid_t pid, int
 {
size_t len, orglen;
int mib[4], ret;
-   char *buf;
+   void *buf;
 
orglen = env ? kd->nbpg : 8 * kd->nbpg; /* XXX - should be ARG_MAX */
-   if (kd->argbuf == NULL &&
-   (kd->argbuf = _kvm_malloc(kd, orglen)) == NULL)
-   return (NULL);
+
+   buf = _kvm_realloc(kd, kd->argbuf, orglen);
+   if (buf == NULL)
+   return NULL;
+   kd->argbuf = buf;
 
 again:
mib[0] = CTL_KERN;

Re: Push the netlock down in in_control()

[Klemens Nanni]

On Tue, May 01, 2018 at 07:08:59PM +0200, Theo Buehler wrote:
> I tested this as well as I could, but I don't usually use IPv6, so I
> dabbled a bit with it on my home network and I tried to use the netinet6
> regress tests, only with moderate success.
> 
> A test from people who actually use IPv6 would be welcome.
Works fine for me so far on a X230 in an IPv6 only network with every
day usage, package upgrades and backups over the wire.

Re: in_ioctl(): use read lock for SIOCGIF*

[Theo Buehler]

On Wed, May 02, 2018 at 01:58:06AM +0200, Theo Buehler wrote:
> Similarly to what we did for ifioctl() a few months back, we can factor
> out the handling of reading ioctls into a new function, in_ioctl_get().
> Treat these cases before grabbing the NET_LOCK() in in_ioctl().
> 
> I only moved and copied a few pieces of code around and did not try to
> make any simplifications to keep the idea of the refactoring clearly
> visible.  This results in a bit of code duplication, but it is not too
> bad.
> 
> We can think about further simplifications and introducing a helper
> function or two in a later step.
> 
> Index: sys/netinet/in.c
> ===
> RCS file: /var/cvs/src/sys/netinet/in.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 in.c
> --- sys/netinet/in.c  30 Apr 2018 19:07:44 -  1.150
> +++ sys/netinet/in.c  1 May 2018 23:02:04 -
> @@ -84,6 +84,7 @@
>  
>  void in_socktrim(struct sockaddr_in *);
>  
> +int in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp);

Bad habit from LibreSSL work... I will remove the variable names in the
prototype before committing.

reduce hppa interrupt guts awfulness

[Miod Vallat]

[this is long, and hppa-specific, feel free to ignore if you don't care
 about OpenBSD/hppa. Noone does anyway nowadays]

At the beginning of the hppa port, no interrupts were shared - the
processor allows for 32 interrupt sources and the existing designs made
sure to allocate distinct interrupts to every device.

Every device, but the PS/2 keyboard and mouse controller. So mickey
added a kluge about 15 years ago for them when I wrote the keyboard
controller driver. [Yes, really 15 years ago now... time flies]

And then the port grew PCI bus support, where interrupts have to be
shareable, by design. So the interrupt code got some form of shared
interrupts, but, well, the interrupt handling code is definitely not for
the faint of heart. Yet it works... to some extent. This mostly involves
magic, and the ability to understand locore0.s.

Because most of the native hppa drivers have been designed with
exclusive interrupts in mind, the hppa kernel configuration has many
hardwired interrupt vector lines (`irq' locators).

This is the reason why you have distinct foo0 and foo1 lines in the
kernel configuration, even for devices which driver do not use
interrupts at all, such as sti(4).

While working on ``card-mode'' dino(4) support [coming soon], I ended up
with having more than one dino(4) attachment to the same parent. To my
surprise, both were using the same interrupt, which - in the current
state of the interrupt handling code, which I won't touch without a
ten-feet pole - meant interrupts from the first device were completely
ignored, because the second one would happily overwrite all the relevant
interrupt structures for that interrupt bit.

This happens, because of this in mbsubmatch() in
sys/arch/hppa/hppa/mainbus.c:

int
mbsubmatch(parent, match, aux)
struct device *parent;
void *match, *aux;
{
[..]
if ((ret = (*cf->cf_attach->ca_match)(parent, match, aux)))
ca->ca_irq = cf->hppacf_irq;

return ret;
}

This means that "everytime a kernel configuration line matches, we pick
its irq value". And while many devices have stricter checks based on
their physical addresses, PCI bridges such as dino(4) don't.

So, with a kernel configuration having
dino0   at phantomas? irq 26# PCI bus bridge on [AB]*
dino1   at phantomas? irq 25
both lines would match, and both dino controllers would end up using -
but not sharing! - irq 25.

This was not noticed because systems with two dino attachments AT THE
SAME PARENT do not exist in their DEFAULT configuration.

However, adding support for cardmode dino changes the rules. And one can
end up with THREE dino controllers although the kernel only allows for
dino0 and dino1.

The only correct way to address this is to relax this fixed irq
assignments and let the kernel pick unused interrupt vectors. In fact,
this logic was already in use on higher-end elroy(4) systems.

The following diff does:
- dynamic allocation of interrupts for dino(4) and gsc(4).
- only mention irq locators for devices which use interrupts.
- for gsc(4), this actually postpones interrupt allocation until gsc(4)
  attaches, so asp(4)/lasi(4)/wax(4) won't route the interrupt yet, but
  the gsc(4) child will take care of this.
- because of hardwired interrupt line assignment on the pre-PCI systems,
  the logic looking for a free interrupt pin looks from 31 to 0, while
  it used to search in the opposite order. This WILL affect all elroy
  systems (C3xxx, J) but shouldn't have any noticeable effect. Even
  though gsckbc will pick interrupt 26, systems with gsckbc will have
  the asp/lasi/wax controller attaching early enough to leave irq 26
  available by the time gsckbc attacheѕ.
- remove the irq locator for the drivers which do not need a fixed
  assignment. siop and moongoose could benefit from this but don't work
  at the moment so this is left as an exercize to future kernel hackers.
  A side effect from this is that we do not need multiple sti(4)
  attachment lines, so PCI sti(4) devices will now attach as sti0 rather
  than sti2.

With this diff, multiple dino(4) devices attached to the same parent
will use different interrupt sources, and more than two dino(4) devices
can attach.

Tested on 715/75 (old-gen using asp), 715/100/XC (new-gen using lasi),
B132L+, B180L, C240, B2000, C3650.

B2000 and C3650 test have been done with sys/dev/ic/com.c downgraded to
1.166 in order to avoid freezes (bug resolution ongoing and not related
to this diff).

715 and B1xx tests have been done with sys/dev/pckbc/pckbd.c downgraded
to 1.43 (bug resolution ongoing and not related to this diff).

$ cat levandes.vari
Index: conf/GENERIC
===
RCS file: /OpenBSD/src/sys/arch/hppa/conf/GENERIC,v
retrieving revision 1.175
diff -u -p -r1.175 GENERIC
--- conf/GENERIC14 Feb 2018 23:51:49 -  

in_ioctl(): use read lock for SIOCGIF*

[Theo Buehler]

Similarly to what we did for ifioctl() a few months back, we can factor
out the handling of reading ioctls into a new function, in_ioctl_get().
Treat these cases before grabbing the NET_LOCK() in in_ioctl().

I only moved and copied a few pieces of code around and did not try to
make any simplifications to keep the idea of the refactoring clearly
visible.  This results in a bit of code duplication, but it is not too
bad.

We can think about further simplifications and introducing a helper
function or two in a later step.

Index: sys/netinet/in.c
===
RCS file: /var/cvs/src/sys/netinet/in.c,v
retrieving revision 1.150
diff -u -p -r1.150 in.c
--- sys/netinet/in.c30 Apr 2018 19:07:44 -  1.150
+++ sys/netinet/in.c1 May 2018 23:02:04 -
@@ -84,6 +84,7 @@
 
 void in_socktrim(struct sockaddr_in *);
 
+int in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp);
 void in_purgeaddr(struct ifaddr *);
 int in_addhost(struct in_ifaddr *, struct sockaddr_in *);
 int in_scrubhost(struct in_ifaddr *, struct sockaddr_in *);
@@ -220,6 +221,14 @@ in_ioctl(u_long cmd, caddr_t data, struc
if (ifp == NULL)
return (ENXIO);
 
+   switch (cmd) {
+   case SIOCGIFADDR:
+   case SIOCGIFNETMASK:
+   case SIOCGIFDSTADDR:
+   case SIOCGIFBRDADDR:
+   return in_ioctl_get(cmd, data, ifp);
+   }
+
NET_LOCK();
 
TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
@@ -230,7 +239,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
}
 
switch (cmd) {
-
case SIOCAIFADDR:
case SIOCDIFADDR:
if (ifra->ifra_addr.sin_family == AF_INET) {
@@ -279,12 +287,7 @@ in_ioctl(u_long cmd, caddr_t data, struc
error = EPERM;
goto err;
}
-   /* FALLTHROUGH */
 
-   case SIOCGIFADDR:
-   case SIOCGIFNETMASK:
-   case SIOCGIFDSTADDR:
-   case SIOCGIFBRDADDR:
if (ia && satosin(>ifr_addr)->sin_addr.s_addr) {
for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
if ((ifa->ifa_addr->sa_family == AF_INET) &&
@@ -302,31 +305,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
break;
}
switch (cmd) {
-
-   case SIOCGIFADDR:
-   *satosin(>ifr_addr) = ia->ia_addr;
-   break;
-
-   case SIOCGIFBRDADDR:
-   if ((ifp->if_flags & IFF_BROADCAST) == 0) {
-   error = EINVAL;
-   break;
-   }
-   *satosin(>ifr_dstaddr) = ia->ia_broadaddr;
-   break;
-
-   case SIOCGIFDSTADDR:
-   if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
-   error = EINVAL;
-   break;
-   }
-   *satosin(>ifr_dstaddr) = ia->ia_dstaddr;
-   break;
-
-   case SIOCGIFNETMASK:
-   *satosin(>ifr_addr) = ia->ia_sockmask;
-   break;
-
case SIOCSIFDSTADDR:
if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
error = EINVAL;
@@ -422,6 +400,73 @@ err:
NET_UNLOCK();
return (error);
 }
+
+int
+in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp)
+{
+   struct ifreq *ifr = (struct ifreq *)data;
+   struct ifaddr *ifa;
+   struct in_ifaddr *ia = NULL;
+   int error = 0;
+
+   NET_RLOCK();
+
+   TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
+   if (ifa->ifa_addr->sa_family == AF_INET) {
+   ia = ifatoia(ifa);
+   break;
+   }
+   }
+
+   if (ia && satosin(>ifr_addr)->sin_addr.s_addr) {
+   for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
+   if ((ifa->ifa_addr->sa_family == AF_INET) &&
+   ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
+   satosin(>ifr_addr)->sin_addr.s_addr) {
+   ia = ifatoia(ifa);
+   break;
+   }
+   }
+   }
+   if (ia == NULL) {
+   error = EADDRNOTAVAIL;
+   goto err;
+   }
+
+   switch(cmd) {
+   case SIOCGIFADDR:
+   *satosin(>ifr_addr) = ia->ia_addr;
+   break;
+
+   case SIOCGIFBRDADDR:
+   if ((ifp->if_flags & IFF_BROADCAST) == 0) {
+   error = EINVAL;
+   break;
+   }
+   *satosin(>ifr_dstaddr) = ia->ia_broadaddr;
+   break;
+
+   case SIOCGIFDSTADDR:
+   if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+   error = EINVAL;
+   break;
+   }
+   *satosin(>ifr_dstaddr) = ia->ia_dstaddr;
+   break;
+
+   case 

Re: 5GHz AP RSSI measurement problem

[Patrick Dohman]

I believe you are correct to correlate RSSI & probe response.
My understanding is that 802.11 beacon is a management frame to synchronize 
networks.
Essentially the beacon interval determines the multiple for the DTIM & may 
include additional capability frames.
The beacon blinks at a configurable interval (100 MS) & establishes a countdown 
for broadcast eligibility.
In some implementations the reception of blinks may indicate congestion or 
interference & perhaps infer signal strength. 
Regards
Patrick

> On Apr 30, 2018, at 3:55 AM, Stefan Sperling  wrote:
> 
> I've ran into what seems to be a fairly modern dual-band AP (issued
> by a French ISP). This AP camps on channel 112. This channel requires
> radar detection which may explain the behaviour described below.
> 
> The AP sends 5GHz beacons with a ridicously low RSSI while no client
> is connected, and OpenBSD prefers the 2GHz band...
> 
>  + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
>  + b8:ee:0e:cb:b3:09  112+6 54M   ess  privacy   rsn  "ESSID"
> 
> ... unless we get lucky and the most recently measured RSSI is obtained
> from a probe response instead of a beacon:
> 
>  + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
>  + b8:ee:0e:cb:b3:09  112   +61 54M   ess  privacy   rsn  "ESSID"
> 
> tcpdump confirms that beacons are received with a low RSSI of -10dBm
> as long as no other client is connected (nevermind that the positive
> dBm values shown here are bogus; that is a separate issue):
> 
> 10:18:59.773885 802.11 flags=0<>: beacon, timestamp 974393344059, interval 
> 100,
> caps=2421, ssid (ESSID), rates 6M* 
> 9M
> 12M* 18M 24M* 36M 48M 54M, ds (chan 112), tim 0x0003, country 'FR ',
> channel 36 limit 23dB, channel 40 limit 23dB, channel 44 limit 23dB, channel 
> 48
> limit 23dB, channel 52 limit 23dB, channel 56 limit 23dB, channel 60 limit
> 23dB, channel 64 limit 23dB, channel 100 limit 23dB, channel 104 limit 23dB,
> channel 108 limit 23dB, channel 112 limit 23dB, channel 116 limit 23dB, 
> channel
> 132 limit 23dB, channel 136 limit 23dB, channel 140 limit 23dB, power
> constraint 0dB,  noise -127dBm>
> 
> Whereas probe responses consistently arrive with much more promising
> RSSI values of about -60dBm:
> 
> 10:18:58.889338 802.11 flags=8: probe response, timestamp 974392459305,
> interval 100, caps=2421, ssid
> (ESSID), rates 6M* 9M 12M* 18M 24M* 36M 48M 54M, ds (chan 112), country 'FR ',
> channel 36 limit 23dB, channel 40 limit 23dB, channel 44 limit 23dB, channel 
> 48
> limit 23dB, channel 52 limit 23dB, channel 56 limit 23dB, channel 60 limit
> 23dB, channel 64 limit 23dB, channel 100 limit 23dB, channel 104 limit 23dB,
> channel 108 limit 23dB, channel 112 limit 23dB, channel 116 limit 23dB, 
> channel
> 132 limit 23dB, channel 136 limit 23dB, channel 140 limit 23dB, power
> constraint 0dB,  noise -127dBm>
> 
> I have no idea if and where the 802.11 standard describes this behaviour.
> Maybe there is a way to tell the real RSSI even from beacon frames?
> Does anyone know more about this?
> 
> Setting aside concerns about my lack of understanding of the underlying
> reason for this behaviour, the hack below is sufficient to make this AP
> show up as a strong contender in the candidate list and be preferred
> over 2GHz as it should be.
> 
> Should I commit this hack? I don't see any downsides.
> 
> Index: ieee80211_input.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
> retrieving revision 1.200
> diff -u -p -r1.200 ieee80211_input.c
> --- ieee80211_input.c 29 Apr 2018 12:11:48 -  1.200
> +++ ieee80211_input.c 30 Apr 2018 08:32:58 -
> @@ -1689,13 +1689,26 @@ ieee80211_recv_probe_resp(struct ieee802
>   memcpy(ni->ni_essid, [2], ssid[1]);
>   }
>   IEEE80211_ADDR_COPY(ni->ni_bssid, wh->i_addr3);
> - ni->ni_rssi = rxi->rxi_rssi;
> + /* XXX validate channel # */
> + ni->ni_chan = >ic_channels[chan];
> + if (ic->ic_state == IEEE80211_S_SCAN &&
> + IEEE80211_IS_CHAN_5GHZ(ni->ni_chan)) {
> + /*
> +  * During a scan on 5Ghz, prefer RSSI measured for probe
> +  * response frames. i.e. don't allow beacons to lower the
> +  * measured RSSI. Some 5GHz APs send beacons with much
> +  * less Tx power than they use for probe responses.
> +  */
> +  if (isprobe)
> + ni->ni_rssi = rxi->rxi_rssi;
> + else if (ni->ni_rssi < rxi->rxi_rssi)
> + ni->ni_rssi = rxi->rxi_rssi;
> + } else
> + ni->ni_rssi = rxi->rxi_rssi;
>   ni->ni_rstamp = rxi->rxi_tstamp;
>   memcpy(ni->ni_tstamp, tstamp, sizeof(ni->ni_tstamp));
>   ni->ni_intval = bintval;
>   ni->ni_capinfo = capinfo;
> - /* XXX validate channel # */
> - 

Re: avoid uninitialised attr in rasops_scrollback()

[Miod Vallat]

> Thanks, looks good/ok.  Shouldn't the eraserows call in
> rasops_doswitch() also use scr->rs_defattr for the attr argument?

Yes, indeed. Good catch!

Re: avoid uninitialised attr in rasops_scrollback()

[Jonathan Gray]

On Tue, May 01, 2018 at 05:03:26PM +, Miod Vallat wrote:
> > scrollback isn't part of wsdisplay_emulops but rather wsdisplay_accessops.
> > It isn't clear how to properly get an attr in wsscrollback()
> > GETCHAR/getchar is part of wsmoused and sc->sc_accessops->getchar
> > may be NULL.
> 
> Ok, ignore that. There is a simpler solution.

Thanks, looks good/ok.  Shouldn't the eraserows call in
rasops_doswitch() also use scr->rs_defattr for the attr argument?

Index: rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.53
diff -u -p -r1.53 rasops.c
--- rasops.c27 Apr 2018 21:36:12 -  1.53
+++ rasops.c2 May 2018 01:53:38 -
@@ -1373,6 +1373,7 @@ struct rasops_screen {
int rs_visible;
int rs_crow;
int rs_ccol;
+   long rs_defattr;
 
int rs_sbscreens;
 #define RS_SCROLLBACK_SCREENS 5
@@ -1412,10 +1413,11 @@ rasops_alloc_screen(void *v, void **cook
scr->rs_visible = (ri->ri_nscreens == 0);
scr->rs_crow = -1;
scr->rs_ccol = -1;
+   scr->rs_defattr = *attrp;
 
for (i = 0; i < scr->rs_dispoffset; i++) {
scr->rs_bs[i].uc = ' ';
-   scr->rs_bs[i].attr = *attrp;
+   scr->rs_bs[i].attr = scr->rs_defattr;
}
 
if (ri->ri_bs && scr->rs_visible) {
@@ -1425,7 +1427,8 @@ rasops_alloc_screen(void *v, void **cook
} else {
for (i = 0; i < ri->ri_rows * ri->ri_cols; i++) {
scr->rs_bs[scr->rs_dispoffset + i].uc = ' ';
-   scr->rs_bs[scr->rs_dispoffset + i].attr = *attrp;
+   scr->rs_bs[scr->rs_dispoffset + i].attr =
+   scr->rs_defattr;
}
}
 
@@ -1474,12 +1477,10 @@ rasops_doswitch(void *v)
struct rasops_info *ri = v;
struct rasops_screen *scr = ri->ri_switchcookie;
int row, col;
-   long attr;
 
rasops_cursor(ri, 0, 0, 0);
ri->ri_active->rs_visible = 0;
-   ri->ri_alloc_attr(ri, 0, 0, 0, );
-   ri->ri_eraserows(ri, 0, ri->ri_rows, attr);
+   ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr);
ri->ri_active = scr;
ri->ri_active->rs_visible = 1;
ri->ri_active->rs_visibleoffset = ri->ri_active->rs_dispoffset;
@@ -1906,7 +1907,6 @@ rasops_scrollback(void *v, void *cookie,
struct rasops_info *ri = v;
struct rasops_screen *scr = cookie;
int row, col, oldvoff;
-   long attr;
 
oldvoff = scr->rs_visibleoffset;
 
@@ -1927,7 +1927,7 @@ rasops_scrollback(void *v, void *cookie,
return;
 
rasops_cursor(ri, 0, 0, 0);
-   ri->ri_eraserows(ri, 0, ri->ri_rows, attr);
+   ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr);
for (row = 0; row < ri->ri_rows; row++) {
for (col = 0; col < ri->ri_cols; col++) {
int off = row * scr->rs_ri->ri_cols + col +

Hyper-V network: let hvn_iff handle promisc mode activation

[Mike Belopuhov]

Hi,

A user has reported an issue with not having a proper promiscuous mode
on reddit a while back but as of now didn't manage to test the diff.

If somebody is running OpenBSD on Hyper-V, please test the diff below
and see if there are any regressions (like increased CPU load) without
running tcpdump and then try running 'tcpdump -nvvi hvn0' and see if
that works: one indicator is the PROMISC flag in the ifconfig output
that should get set when tcpdump is started and cleared once it's
finished.

I'm not certain if Hyper-V will allow you to sniff a whole lot of
traffic on the virtual switch, however, one of the goals here is to
instruct RNDIS to enter "all multicast capture" mode when one or more
multicast addresses are configured.  Therefore one of the goals is to
attempt to support CARP operation on the virtual switch.


commit 6e6b001dae79505e3e0fbca663c31f9eb6da285b
Author: Mike Belopuhov 
Date:   Sun Mar 11 13:38:21 2018 +0100

Add support for promisc mode

diff --git sys/dev/pv/if_hvn.c sys/dev/pv/if_hvn.c
index 3ca35165565..6d5e919b01c 100644
--- sys/dev/pv/if_hvn.c
+++ sys/dev/pv/if_hvn.c
@@ -134,11 +134,10 @@ struct hvn_softc {
bus_dma_tag_tsc_dmat;
 
struct arpcomsc_ac;
struct ifmedia   sc_media;
int  sc_link_state;
-   int  sc_promisc;
 
/* NVS protocol */
int  sc_proto;
uint32_t sc_nvstid;
uint8_t  sc_nvsrsp[HVN_NVS_MSGSIZE];
@@ -208,11 +207,10 @@ void  hvn_rxeof(struct hvn_softc *, caddr_t, 
uint32_t, struct mbuf_list *);
 void   hvn_rndis_complete(struct hvn_softc *, caddr_t, uint32_t);
 inthvn_rndis_output(struct hvn_softc *, struct hvn_tx_desc *);
 void   hvn_rndis_status(struct hvn_softc *, caddr_t, uint32_t);
 inthvn_rndis_query(struct hvn_softc *, uint32_t, void *, size_t *);
 inthvn_rndis_set(struct hvn_softc *, uint32_t, void *, size_t);
-inthvn_rndis_open(struct hvn_softc *);
 inthvn_rndis_close(struct hvn_softc *);
 void   hvn_rndis_detach(struct hvn_softc *);
 
 struct cfdriver hvn_cd = {
NULL, "hvn", DV_IFNET
@@ -401,26 +399,44 @@ hvn_link_status(struct hvn_softc *sc)
 }
 
 int
 hvn_iff(struct hvn_softc *sc)
 {
-   /* XXX */
-   sc->sc_promisc = 0;
+   struct ifnet *ifp = >sc_ac.ac_if;
+   uint32_t filter = 0;
+   int rv;
 
-   return (0);
+   ifp->if_flags &= ~IFF_ALLMULTI;
+
+   if ((ifp->if_flags & IFF_PROMISC) || sc->sc_ac.ac_multirangecnt > 0) {
+   ifp->if_flags |= IFF_ALLMULTI;
+   filter = NDIS_PACKET_TYPE_PROMISCUOUS;
+   } else {
+   filter = NDIS_PACKET_TYPE_BROADCAST |
+   NDIS_PACKET_TYPE_DIRECTED;
+   if (sc->sc_ac.ac_multicnt > 0) {
+   ifp->if_flags |= IFF_ALLMULTI;
+   filter |= NDIS_PACKET_TYPE_ALL_MULTICAST;
+   }
+   }
+
+   rv = hvn_rndis_set(sc, OID_GEN_CURRENT_PACKET_FILTER,
+   , sizeof(filter));
+   if (rv)
+   DPRINTF("%s: failed to set RNDIS filter to %#x\n",
+   sc->sc_dev.dv_xname, filter);
+   return (rv);
 }
 
 void
 hvn_init(struct hvn_softc *sc)
 {
struct ifnet *ifp = >sc_ac.ac_if;
 
hvn_stop(sc);
 
-   hvn_iff(sc);
-
-   if (hvn_rndis_open(sc) == 0) {
+   if (hvn_iff(sc) == 0) {
ifp->if_flags |= IFF_RUNNING;
ifq_clr_oactive(>if_snd);
}
 }
 
@@ -1723,31 +1739,10 @@ hvn_rndis_set(struct hvn_softc *sc, uint32_t oid, void 
*data, size_t length)
hvn_free_cmd(sc, rc);
 
return (rv);
 }
 
-int
-hvn_rndis_open(struct hvn_softc *sc)
-{
-   uint32_t filter;
-   int rv;
-
-   if (sc->sc_promisc)
-   filter = NDIS_PACKET_TYPE_PROMISCUOUS;
-   else
-   filter = NDIS_PACKET_TYPE_BROADCAST |
-   NDIS_PACKET_TYPE_ALL_MULTICAST |
-   NDIS_PACKET_TYPE_DIRECTED;
-
-   rv = hvn_rndis_set(sc, OID_GEN_CURRENT_PACKET_FILTER,
-   , sizeof(filter));
-   if (rv)
-   DPRINTF("%s: failed to set RNDIS filter to %#x\n",
-   sc->sc_dev.dv_xname, filter);
-   return (rv);
-}
-
 int
 hvn_rndis_close(struct hvn_softc *sc)
 {
uint32_t filter = 0;
int rv;

Re: remove 2 obsolete libraries from Xenocara

[Theo Buehler]

On Sat, Apr 28, 2018 at 07:24:28PM +0200, Matthieu Herrb wrote:
> Hi,
> 
> libXfontcache and libXxf86misc are implementing the client part of X
> extensions that have been disabled/removed for a while in the X
> server. So builing them or linking to them is useless.
> 
> The patch below stops building them in xenocara. A few ports where
> using libXxf86misc. Rebuilding them without this library just requires
> an update to WANTLIB (patch below, too).
> 
> To test make sure you build xenocara with an empty /usr/X11R6 or that
> you remove all files removed from sets manually.
> 
> PS: This also prepares for the switch to xorgproto and xserver 1.20.
> 
> oks?

I've been running this on my x230 for a few days without any problems. I
don't use any of the impacted ports, though.

fwiw, I'm ok with this removal.

Re: vmd: def nitems() locally

[Mike Larkin]

On Mon, Apr 30, 2018 at 07:49:20PM -0300, Gleydson Soares wrote:
> hi,
> 
> following diff defines nitems locally and stop 
> including 

Has this been done elsewhere in the tree? Is this the new paradigm
we will be adopting? I don't have a strong opinion one way or the
other, just want to be consistent.

-ml


> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/control.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 control.c
> --- control.c 8 Sep 2017 06:24:31 -   1.22
> +++ control.c 30 Apr 2018 22:45:22 -
> @@ -17,7 +17,6 @@
>   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>   */
>  
> -#include/* nitems */
>  #include 
>  #include 
>  #include 
> Index: priv.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/priv.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 priv.c
> --- priv.c11 Nov 2017 02:50:07 -  1.13
> +++ priv.c30 Apr 2018 22:45:22 -
> @@ -16,7 +16,6 @@
>   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>   */
>  
> -#include/* nitems */
>  #include 
>  #include 
>  #include 
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 vmd.c
> --- vmd.c 25 Apr 2018 15:49:48 -  1.84
> +++ vmd.c 30 Apr 2018 22:45:22 -
> @@ -16,7 +16,6 @@
>   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>   */
>  
> -#include/* nitems */
>  #include 
>  #include 
>  #include 
> Index: vmd.h
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
> retrieving revision 1.68
> diff -u -p -r1.68 vmd.h
> --- vmd.h 27 Apr 2018 12:15:10 -  1.68
> +++ vmd.h 30 Apr 2018 22:45:22 -
> @@ -35,6 +35,10 @@
>  #ifndef VMD_H
>  #define VMD_H
>  
> +#ifndef nitems
> +#define nitems(_a)(sizeof((_a)) / sizeof((_a)[0]))
> +#endif
> +
>  #define SET(_v, _m)  ((_v) |= (_m))
>  #define CLR(_v, _m)  ((_v) &= ~(_m))
>  #define ISSET(_v, _m)((_v) & (_m))
> Index: vmm.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 vmm.c
> --- vmm.c 13 Apr 2018 17:12:44 -  1.81
> +++ vmm.c 30 Apr 2018 22:45:22 -
> @@ -16,7 +16,6 @@
>   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>   */
>  
> -#include/* nitems */
>  #include 
>  #include 
>  #include 

Re: vmd: def nitems() locally

[Theo de Raadt]

Mike Larkin  wrote:

> On Mon, Apr 30, 2018 at 07:49:20PM -0300, Gleydson Soares wrote:
> > hi,
> > 
> > following diff defines nitems locally and stop 
> > including 
> 
> Has this been done elsewhere in the tree? Is this the new paradigm
> we will be adopting? I don't have a strong opinion one way or the
> other, just want to be consistent.

I prefer the current "document the problem on #include "
line approach, until we make a decision about making this available in
userland.  Which would require a robust assessment of impact on the
entire ports ecosystem, and consider how it may bleed into other
projects.

I don't believe we have reached that point yet, so I prefer if it
remains as-is.

Re: 5GHz AP RSSI measurement problem

[Stefan Sperling]

On Tue, May 01, 2018 at 10:22:22AM +0100, Stuart Henderson wrote:
> On 2018/05/01 10:48, Stefan Sperling wrote:
> > > On 2018/04/30 11:08, Stefan Sperling wrote:
> > > > Derp. A dBm value of -10 would of course be better than -60.
> > > > 
> > > > Whatever the numbers shown by tcpdump really mean, the probe response's
> > > > one is better!!!
> > > 
> > > Better as in "more accurate". But as the reported value is ridiculously
> > > high rather than too low, why wasn't 5GHz selected anyway?
> > 
> > What I wrote in my first mail was not very clear (I wrote it in
> > a hurry before leaving to catch a train).
> > 
> > The kernel compares the RSSI values which are shown in debug output.
> > For reference, the scan debug printfs below again show a 2GHz beacon
> > vs.  a 5GHz "low power" beacon, where measured RSSI on 2Ghz is represented
> > as "58" and on 5Ghz is represented as "6":
> > 
> >   + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
> >   + b8:ee:0e:cb:b3:09  112+6 54M   ess  privacy   rsn  "ESSID"
> > 
> > The "non-reduced" Tx power frames, i.e. probe responses or beacons while
> > a client is associated, closely matched Tx power seen on 2GHz:
> > 
> >   + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
> >   + b8:ee:0e:cb:b3:09  112   +61 54M   ess  privacy   rsn  "ESSID"
> 
> What do the values represent here?
>
> I've been reading them as sign-flipped dBm signal strength because
> (before the +6 outlier) the numbers all matched that (i.e. -61dBm for
> 5GHz and -58dBm for 2GHz looks right for an access point at typical
> power levels about 30m away).

I've done some quick digging:

The values come from iwm(4). Which means they are derived as dBm from
values reported by hardware and then converted into a percentage.
See iwm_calc_rssi() and its caller iwm_rx_rx_mpdu().

I've been wondering if we should make iwm(4) report values in dBm instead
of as a percentage. But a percentage may be more intuitive to users.
I'll leave this as it is -- since pirofti@ told me he might be working
on fixing the RSSI situation I'd rather leave such decisions to him.

> I didn't interpret them as % in this case because it would be unusual
> for the 5GHz signal to be stronger than the 2GHz though that is
> possible and if that's the case, your diff makes sense.

It seems % would be the correct interpretation.

I was sitting right next to the AP and I'm not surprised that
both bands show similar values at such close range.

Re: rssi comparison threshold

[Stefan Sperling]

On Mon, Apr 30, 2018 at 08:57:23AM +0200, Peter Hessler wrote:
> On 2018 Apr 29 (Sun) at 11:51:26 +0200 (+0200), Stefan Sperling wrote:
> :This diff tries to avoid situations where background scans play
> :ping-pong between different APs with nearly equal RSSI, as
> :observed by phessler.
> :
> :Not all drivers represent RSSI values in dBm or percentage, so the
> :diff includes the possibility for drivers to override the new RSSI
> :comparison function. However, since the threshold is rather low
> :applying this to all drivers for now should not do any harm, unless
> :there is a driver where the RSSI value range is ridiculously small.
> :I'm not aware of any such driver at present.
> :
> 
> I'm concerned about two things.
> 
> The usage is confusing, because you pass two incompatible things to be
> compared.  I would prefer ieee80211_node_cmprssi(ic, rssi_one, rssi_two).

Agreed. Updated diff below. I've chosen to make it compare two nodes
since the function lives in the nodes namespace. It makes the function
a bit less flexible as it cannot be used to compare naked rssi values.
But that's not really needed for the problem at hand; after all, we
want to compare APs.

> Also, in the case where ni->ni_rssi has a very weak signal, the second
> comparison can underflow.

Indeed. Thanks for catching this!

Index: ieee80211_node.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
retrieving revision 1.129
diff -u -p -r1.129 ieee80211_node.c
--- ieee80211_node.c28 Apr 2018 14:49:07 -  1.129
+++ ieee80211_node.c30 Apr 2018 07:30:43 -
@@ -67,6 +67,8 @@ u_int8_t ieee80211_node_getrssi(struct i
 const struct ieee80211_node *);
 int ieee80211_node_checkrssi(struct ieee80211com *,
 const struct ieee80211_node *);
+int ieee80211_node_cmprssi(struct ieee80211com *,
+const struct ieee80211_node *, const struct ieee80211_node *);
 void ieee80211_setup_node(struct ieee80211com *, struct ieee80211_node *,
 const u_int8_t *);
 void ieee80211_free_node(struct ieee80211com *, struct ieee80211_node *);
@@ -133,6 +135,7 @@ ieee80211_node_attach(struct ifnet *ifp)
ic->ic_node_copy = ieee80211_node_copy;
ic->ic_node_getrssi = ieee80211_node_getrssi;
ic->ic_node_checkrssi = ieee80211_node_checkrssi;
+   ic->ic_node_cmprssi = ieee80211_node_cmprssi;
ic->ic_scangen = 1;
ic->ic_max_nnodes = ieee80211_cache_size;
 
@@ -761,12 +764,15 @@ ieee80211_end_scan(struct ifnet *ifp)
 
if (ic->ic_caps & IEEE80211_C_SCANALLBAND) {
if (IEEE80211_IS_CHAN_2GHZ(ni->ni_chan) &&
-   (selbs2 == NULL || ni->ni_rssi > selbs2->ni_rssi))
+   (selbs2 == NULL ||
+   ic->ic_node_cmprssi(ic, ni, selbs2) > 0))
selbs2 = ni;
else if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) &&
-   (selbs5 == NULL || ni->ni_rssi > selbs5->ni_rssi))
+   (selbs5 == NULL ||
+   ic->ic_node_cmprssi(ic, ni, selbs5) > 0))
selbs5 = ni;
-   } else if (selbs == NULL || ni->ni_rssi > selbs->ni_rssi)
+   } else if (selbs == NULL ||
+   ic->ic_node_cmprssi(ic, ni, selbs) > 0)
selbs = ni;
}
 
@@ -782,9 +788,12 @@ ieee80211_end_scan(struct ifnet *ifp)
 */
if (selbs5 && selbs5->ni_rssi > min_5ghz_rssi)
selbs = selbs5;
-   else if (selbs5 && selbs2)
-   selbs = (selbs5->ni_rssi >= selbs2->ni_rssi ? selbs5 : selbs2);
-   else if (selbs2)
+   else if (selbs5 && selbs2) {
+   if (ic->ic_node_cmprssi(ic, selbs5, selbs2) >= 0)
+   selbs = selbs5;
+   else
+   selbs = selbs2;
+   } else if (selbs2)
selbs = selbs2;
else if (selbs5)
selbs = selbs5;
@@ -989,6 +998,38 @@ ieee80211_node_checkrssi(struct ieee8021
IEEE80211_RSSI_THRES_2GHZ :
IEEE80211_RSSI_THRES_5GHZ;
return (ni->ni_rssi >= (u_int8_t)thres);
+}
+
+/*
+ * Determine if RSSI values of two nodes are significantly different.
+ * This function assumes RSSI values are represented either in dBm or
+ * as a percentage of ic_max_rssi. Drivers should override this function
+ * in case their RSSI values use a different representation.
+ */
+int
+ieee80211_node_cmprssi(struct ieee80211com *ic,
+const struct ieee80211_node *ni1, const struct ieee80211_node *ni2)
+{
+   uint8_t thres;
+
+   if (ic->ic_max_rssi)
+   thres = IEEE80211_RSSI_CMP_THRES_RATIO;
+   else
+   thres = IEEE80211_RSSI_CMP_THRES_DBM;
+
+   /* cap threshold to prevent overflow in calculations below */
+   while (thres > 0) {
+   if (ni1->ni_rssi + thres >= 

Re: 5GHz AP RSSI measurement problem

[Stefan Sperling]

On Mon, Apr 30, 2018 at 01:35:59PM +0100, Stuart Henderson wrote:
> On 2018/04/30 10:55, Stefan Sperling wrote:
> > The AP sends 5GHz beacons with a ridicously low RSSI while no client
> > is connected, and OpenBSD prefers the 2GHz band...
> 
> Surely it has to be the receiver that is adding the RSSI infornation?
> The AP can't know.

Indeed. What I meant is that this AP seems to send 5Ghz beacons
with reduced Tx power for some reason.

> Seems it would either have to be the client side measuring incorrectly,
> or the AP is really sending beacons at low power. If the latter, that
> _could_ be a mechanism to reduce power consumption in idle mode...
> Let them connect at 2GHz then whack up the power and steer them towards
> 5GHz after association...

Yes, something like that seems to be happening.
It could also be related to radar since low power beacons would carry
less risk of causing interference with radar on channel 112.

There is a huge amount of material in the 802.11 specs about tx power
management and spectrum management but I haven't read any of that.

> Anyway it would be interesting to see how a spectrum analyser looks with
> this access point :)

For all intents and purposes, trusting the RSSI reported by Intel
hardware seems good enough to me :)

> On 2018/04/30 11:08, Stefan Sperling wrote:
> > Derp. A dBm value of -10 would of course be better than -60.
> > 
> > Whatever the numbers shown by tcpdump really mean, the probe response's
> > one is better!!!
> 
> Better as in "more accurate". But as the reported value is ridiculously
> high rather than too low, why wasn't 5GHz selected anyway?

What I wrote in my first mail was not very clear (I wrote it in
a hurry before leaving to catch a train).

The kernel compares the RSSI values which are shown in debug output.
For reference, the scan debug printfs below again show a 2GHz beacon
vs.  a 5GHz "low power" beacon, where measured RSSI on 2Ghz is represented
as "58" and on 5Ghz is represented as "6":

  + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
  + b8:ee:0e:cb:b3:09  112+6 54M   ess  privacy   rsn  "ESSID"

The "non-reduced" Tx power frames, i.e. probe responses or beacons while
a client is associated, closely matched Tx power seen on 2GHz:

  + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
  + b8:ee:0e:cb:b3:09  112   +61 54M   ess  privacy   rsn  "ESSID"

In the above cases, the RSSI value comparisons which happened in the
kernel were 6 < 58 without my diff and 61 < 58 with my diff.

We never pick the 5GHz AP in the former situation. Of course, there
is a race: We would pick the 5GHz AP if its node record was created
due to a probe response and no subsequent beacon had reset ni_rssi
to a low value before we choose an AP. This occasionally happened
in my observations but most of the time we picked the 2GHz AP.

In my first mail, values I've quoted from scan debug output and
values I've shown from tcpdump were derived from different frames.
Debug output for those frames captured by tcpdump would have been
10 and 60 rather than 6 and 61, I suppose.

Re: 5GHz AP RSSI measurement problem

[Stuart Henderson]

On 2018/05/01 10:48, Stefan Sperling wrote:
> > On 2018/04/30 11:08, Stefan Sperling wrote:
> > > Derp. A dBm value of -10 would of course be better than -60.
> > > 
> > > Whatever the numbers shown by tcpdump really mean, the probe response's
> > > one is better!!!
> > 
> > Better as in "more accurate". But as the reported value is ridiculously
> > high rather than too low, why wasn't 5GHz selected anyway?
> 
> What I wrote in my first mail was not very clear (I wrote it in
> a hurry before leaving to catch a train).
> 
> The kernel compares the RSSI values which are shown in debug output.
> For reference, the scan debug printfs below again show a 2GHz beacon
> vs.  a 5GHz "low power" beacon, where measured RSSI on 2Ghz is represented
> as "58" and on 5Ghz is represented as "6":
> 
>   + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
>   + b8:ee:0e:cb:b3:09  112+6 54M   ess  privacy   rsn  "ESSID"
> 
> The "non-reduced" Tx power frames, i.e. probe responses or beacons while
> a client is associated, closely matched Tx power seen on 2GHz:
> 
>   + b8:ee:0e:cb:b3:081   +58 54M   ess  privacy   rsn  "ESSID"
>   + b8:ee:0e:cb:b3:09  112   +61 54M   ess  privacy   rsn  "ESSID"

What do the values represent here?

I've been reading them as sign-flipped dBm signal strength because
(before the +6 outlier) the numbers all matched that (i.e. -61dBm for
5GHz and -58dBm for 2GHz looks right for an access point at typical
power levels about 30m away).

I didn't interpret them as % in this case because it would be unusual
for the 5GHz signal to be stronger than the 2GHz though that is
possible and if that's the case, your diff makes sense.


> There is a huge amount of material in the 802.11 specs about tx power
> management and spectrum management but I haven't read any of that.

And even then more reading is needed to understand it all, the
specs just provide the framework for implementation, but the actual
requirements are in local regulations which aren't in the specs.
But fortunately for us (at least in client mode ;) this is mostly
AP-led.

Re: route(8): stop debugging route monitor

[Claudio Jeker]

On Tue, May 01, 2018 at 11:53:16AM +0200, Sebastian Benoit wrote:
> Florian Obser(flor...@openbsd.org) on 2018.04.30 18:27:52 +0200:
> > The -d flag should be a no-op in monitor mode since it does not modify
> > the routing table.
> > 
> > However, if -d is provided route monitor lists all interfaces and
> > their associated addresses and exits. This is confusing, unexpected
> > and no longer needed (if ever).
> > 
> > Make -d a proper no-op for route monitor and get rid of the interfaces
> > function which didn't use the correct sysctl idiom anyway.
> > 
> > OK?
> 
> if nobody complains about loosing that "feature", ok benno@
>  

I didn't know about it but I could have used this from time to time.
Still it feels overloading the debug flag with that is not the right way.
It is useful since this is what getifaddrs(3) uses in the background.

> > diff --git route.c route.c
> > index 85e76621dd3..738de3a8cde 100644
> > --- route.c
> > +++ route.c
> > @@ -115,7 +115,6 @@ int  rtmsg(int, int, int, uint8_t);
> >  __dead void usage(char *);
> >  voidset_metric(char *, int);
> >  voidinet_makenetandmask(u_int32_t, struct sockaddr_in *, int);
> > -voidinterfaces(void);
> >  voidgetlabel(char *);
> >  int gettable(const char *);
> >  int rdomain(int, char **);
> > @@ -1069,36 +1068,6 @@ prefixlen(int af, char *s)
> > return (len == max);
> >  }
> >  
> > -void
> > -interfaces(void)
> > -{
> > -   size_t needed;
> > -   int mib[6];
> > -   char *buf = NULL, *lim, *next;
> > -   struct rt_msghdr *rtm;
> > -
> > -   mib[0] = CTL_NET;
> > -   mib[1] = PF_ROUTE;
> > -   mib[2] = 0; /* protocol */
> > -   mib[3] = 0; /* wildcard address family */
> > -   mib[4] = NET_RT_IFLIST;
> > -   mib[5] = 0; /* no flags */
> > -   if (sysctl(mib, 6, NULL, , NULL, 0) < 0)
> > -   err(1, "route-sysctl-estimate");
> > -   if (needed) {
> > -   if ((buf = malloc(needed)) == NULL)
> > -   err(1, "malloc");
> > -   if (sysctl(mib, 6, buf, , NULL, 0) < 0)
> > -   err(1, "actual retrieval of interface table");
> > -   lim = buf + needed;
> > -   for (next = buf; next < lim; next += rtm->rtm_msglen) {
> > -   rtm = (struct rt_msghdr *)next;
> > -   print_rtmsg(rtm, rtm->rtm_msglen);
> > -   }
> > -   free(buf);
> > -   }
> > -}
> > -
> >  void
> >  monitor(int argc, char *argv[])
> >  {
> > @@ -1107,10 +1076,6 @@ monitor(int argc, char *argv[])
> > time_t now;
> >  
> > verbose = 1;
> > -   if (debugonly) {
> > -   interfaces();
> > -   exit(0);
> > -   }
> > for (;;) {
> > if ((n = read(s, msg, sizeof(msg))) == -1) {
> > if (errno == EINTR)
> > 
> > -- 
> > I'm not entirely sure you are real.
> > 
> 

-- 
:wq Claudio

Re: route(8): sync p_rttables to netstat(1) version

[Sebastian Benoit]

ok

Florian Obser(flor...@openbsd.org) on 2018.04.30 18:26:46 +0200:
> Sync p_rttables() to netstat(1) version. Pointed out by claudio and
> mpi.
> 
> Remaining differences are pledge and priority handling which only
> route(8) has.
> 
> While here switch flushroutes to get_sysctl() function.
> 
> OK?
> 
> diff --git route.c route.c
> index d93374578c5..85e76621dd3 100644
> --- route.c
> +++ route.c
> @@ -281,7 +281,7 @@ int
>  flushroutes(int argc, char **argv)
>  {
>   size_t needed;
> - int mib[7], rlen, seqno, af = AF_UNSPEC;
> + int mib[7], mcnt, rlen, seqno, af = AF_UNSPEC;
>   char *buf = NULL, *next, *lim = NULL;
>   struct rt_msghdr *rtm;
>   struct sockaddr *sa;
> @@ -333,21 +333,10 @@ flushroutes(int argc, char **argv)
>   mib[4] = NET_RT_DUMP;
>   mib[5] = prio;
>   mib[6] = tableid;
> - while (1) {
> - if (sysctl(mib, 7, NULL, , NULL, 0) == -1)
> - err(1, "route-sysctl-estimate");
> - if (needed == 0)
> - break;
> - if ((buf = realloc(buf, needed)) == NULL)
> - err(1, "realloc");
> - if (sysctl(mib, 7, buf, , NULL, 0) == -1) {
> - if (errno == ENOMEM)
> - continue;
> - err(1, "actual retrieval of routing table");
> - }
> - lim = buf + needed;
> - break;
> - }
> + mcnt = 7;
> +
> + needed = get_sysctl(mib, mcnt, );
> + lim = buf + needed;
>  
>   if (pledge("stdio dns", NULL) == -1)
>   err(1, "pledge");
> diff --git show.c show.c
> index 5898a19cd45..68731c78591 100644
> --- show.c
> +++ show.c
> @@ -107,6 +107,29 @@ char *routename6(struct sockaddr_in6 *);
>  char *netname4(in_addr_t, struct sockaddr_in *);
>  char *netname6(struct sockaddr_in6 *, struct sockaddr_in6 *);
>  
> +size_t
> +get_sysctl(const int *mib, u_int mcnt, char **buf)
> +{
> + size_t needed;
> +
> + while (1) {
> + if (sysctl(mib, mcnt, NULL, , NULL, 0) == -1)
> + err(1, "sysctl-estimate");
> + if (needed == 0)
> + break;
> + if ((*buf = realloc(*buf, needed)) == NULL)
> + err(1, NULL);
> + if (sysctl(mib, mcnt, *buf, , NULL, 0) == -1) {
> + if (errno == ENOMEM)
> + continue;
> + err(1, "sysctl");
> + }
> + break;
> + }
> +
> + return needed;
> +}
> +
>  /*
>   * Print routing tables.
>   */
> @@ -116,7 +139,7 @@ p_rttables(int af, u_int tableid, char prio)
>   struct rt_msghdr *rtm;
>   char *buf = NULL, *next, *lim = NULL;
>   size_t needed;
> - int mib[7];
> + int mib[7], mcnt;
>   struct sockaddr *sa;
>  
>   mib[0] = CTL_NET;
> @@ -126,22 +149,10 @@ p_rttables(int af, u_int tableid, char prio)
>   mib[4] = NET_RT_DUMP;
>   mib[5] = prio;
>   mib[6] = tableid;
> + mcnt = 7;
>  
> - while (1) {
> - if (sysctl(mib, 7, NULL, , NULL, 0) == -1)
> - err(1, "route-sysctl-estimate");
> - if (needed == 0)
> - break;
> - if ((buf = realloc(buf, needed)) == NULL)
> - err(1, NULL);
> - if (sysctl(mib, 7, buf, , NULL, 0) == -1) {
> - if (errno == ENOMEM)
> - continue;
> - err(1, "sysctl of routing table");
> - }
> - lim = buf + needed;
> - break;
> - }
> + needed = get_sysctl(mib, mcnt, );
> + lim = buf + needed;
>  
>   if (pledge("stdio dns", NULL) == -1)
>   err(1, "pledge");
> @@ -156,9 +167,8 @@ p_rttables(int af, u_int tableid, char prio)
>   sa = (struct sockaddr *)(next + rtm->rtm_hdrlen);
>   p_rtentry(rtm);
>   }
> - free(buf);
> - buf = NULL;
>   }
> + free(buf);
>  }
>  
>  /*
> diff --git show.h show.h
> index 03999b7fdd7..461d5a39c5e 100644
> --- show.h
> +++ show.h
> @@ -34,6 +34,7 @@ void p_sockaddr(struct sockaddr *, struct sockaddr 
> *, int, int);
>  char *routename(struct sockaddr *);
>  char *netname(struct sockaddr *, struct sockaddr *);
>  char *mpls_op(u_int32_t);
> +size_tget_sysctl(const int *, u_int, char **);
>  
>  extern int nflag;
>  extern int Fflag;
> 
> 
> -- 
> I'm not entirely sure you are real.
> 

SMCCC 1.1 support for psci(4)

[Mark Kettenis]

So after adding a quick hack to mitigate Spectre variant 2 to ARM
Trusted Firmware (ATF), ARM actually designed a proper solution that
minimizes the performance loss and makes the presence of the
workaround detectable.  This is all documented in an update of the SMC
Calling Convention (SMCCC) standard.

The diff below implements support for this solution while keeping
support for the hack.  While ARM strongly suggests vendors to update
to a version of ATF that implements SMCCC 1.1 the current ATF for the
Marvell ARMADA 8040 hasn't been updated yet (but does include the
initial hack).

Unfortunately the SMCCC 1.1 implementation in ATF doesn't quite
implement the spec.  As a result we have to check whether the
workaround is implemented by issuing the relevant calls on each of the
CPUs that might be affected.  This is important for big.LITTLE designs
such as the RK3399 that include both Cortex-A53 cores that aren't
vulnerable and Cortex-A72 cores that are.

ok?


Index: dev/fdt/psci.c
===
RCS file: /cvs/src/sys/dev/fdt/psci.c,v
retrieving revision 1.6
diff -u -p -r1.6 psci.c
--- dev/fdt/psci.c  23 Feb 2018 19:08:56 -  1.6
+++ dev/fdt/psci.c  1 May 2018 09:35:14 -
@@ -31,14 +31,19 @@
 extern void (*cpuresetfn)(void);
 extern void (*powerdownfn)(void);
 
-#define PSCI_VERSION   0x8400
-#define SYSTEM_OFF 0x8408
-#define SYSTEM_RESET   0x8409
+#define SMCCC_VERSION  0x8000
+#define SMCCC_ARCH_FEATURES0x8001
+#define SMCCC_ARCH_WORKAROUND_10x80008000
+
+#define PSCI_VERSION   0x8400
 #ifdef __LP64__
-#define CPU_ON 0xc403
+#define CPU_ON 0xc403
 #else
-#define CPU_ON 0x8403
+#define CPU_ON 0x8403
 #endif
+#define SYSTEM_OFF 0x8408
+#define SYSTEM_RESET   0x8409
+#define PSCI_FEATURES  0x840a
 
 struct psci_softc {
struct devicesc_dev;
@@ -48,6 +53,9 @@ struct psci_softc {
uint32_t sc_system_off;
uint32_t sc_system_reset;
uint32_t sc_cpu_on;
+
+   uint32_t sc_version;
+   uint32_t sc_smccc_version;
 };
 
 struct psci_softc *psci_sc;
@@ -60,6 +68,12 @@ void psci_powerdown(void);
 extern register_t hvc_call(register_t, register_t, register_t, register_t);
 extern register_t smc_call(register_t, register_t, register_t, register_t);
 
+int32_t smccc_version(void);
+int32_t smccc_arch_features(uint32_t);
+
+uint32_t psci_version(void);
+int32_t psci_features(uint32_t);
+
 struct cfattach psci_ca = {
sizeof(struct psci_softc), psci_match, psci_attach
 };
@@ -84,7 +98,6 @@ psci_attach(struct device *parent, struc
struct psci_softc *sc = (struct psci_softc *)self;
struct fdt_attach_args *faa = aux;
char method[128];
-   uint32_t version;
 
if (OF_getprop(faa->fa_node, "method", method, sizeof(method))) {
if (strcmp(method, "hvc") == 0)
@@ -114,8 +127,18 @@ psci_attach(struct device *parent, struc
 
psci_sc = sc;
 
-   version = psci_version();
-   printf(": PSCI %d.%d\n", version >> 16, version & 0x);
+   sc->sc_version = psci_version();
+   printf(": PSCI %d.%d", sc->sc_version >> 16, sc->sc_version & 0x);
+
+   if (sc->sc_version >= 0x1) {
+   if (psci_features(SMCCC_VERSION) == PSCI_SUCCESS) {
+   sc->sc_smccc_version = smccc_version();
+   printf(", SMCCC %d.%d", sc->sc_smccc_version >> 16,
+   sc->sc_smccc_version & 0x);
+   }
+   }
+
+   printf("\n");
 
if (sc->sc_system_off != 0)
powerdownfn = psci_powerdown;
@@ -123,22 +146,11 @@ psci_attach(struct device *parent, struc
cpuresetfn = psci_reset;
 }
 
-uint32_t
-psci_version(void)
-{
-   struct psci_softc *sc = psci_sc;
-
-   if (sc && sc->sc_callfn && sc->sc_psci_version != 0)
-   return (*sc->sc_callfn)(sc->sc_psci_version, 0, 0, 0);
-
-   /* No version support; return 0.0. */
-   return 0;
-}
-
 void
 psci_reset(void)
 {
struct psci_softc *sc = psci_sc;
+
if (sc->sc_callfn)
(*sc->sc_callfn)(sc->sc_system_reset, 0, 0, 0);
 }
@@ -147,10 +159,111 @@ void
 psci_powerdown(void)
 {
struct psci_softc *sc = psci_sc;
+
if (sc->sc_callfn)
(*sc->sc_callfn)(sc->sc_system_off, 0, 0, 0);
 }
 
+/*
+ * Firmware-based workaround for CVE-2017-5715.  We pick the
+ * appropriate mechanism based on the PSCI and SMCCC versions.  Note
+ * that ARM Trusted Firmware violates the SMCCC 1.1 specification and
+ * only reports the presence of the appropriate workaround on CPUs
+ * that are actually vulnerable.  That's why we determine the
+ * appropriate workaround the first time around.
+ */
+
+void
+psci_flush_bp_none(void)
+{
+}
+
+void

Re: route(8): stop debugging route monitor

[Sebastian Benoit]

Florian Obser(flor...@openbsd.org) on 2018.04.30 18:27:52 +0200:
> The -d flag should be a no-op in monitor mode since it does not modify
> the routing table.
> 
> However, if -d is provided route monitor lists all interfaces and
> their associated addresses and exits. This is confusing, unexpected
> and no longer needed (if ever).
> 
> Make -d a proper no-op for route monitor and get rid of the interfaces
> function which didn't use the correct sysctl idiom anyway.
> 
> OK?

if nobody complains about loosing that "feature", ok benno@
 
> diff --git route.c route.c
> index 85e76621dd3..738de3a8cde 100644
> --- route.c
> +++ route.c
> @@ -115,7 +115,6 @@ intrtmsg(int, int, int, uint8_t);
>  __dead void usage(char *);
>  void  set_metric(char *, int);
>  void  inet_makenetandmask(u_int32_t, struct sockaddr_in *, int);
> -void  interfaces(void);
>  void  getlabel(char *);
>  int   gettable(const char *);
>  int   rdomain(int, char **);
> @@ -1069,36 +1068,6 @@ prefixlen(int af, char *s)
>   return (len == max);
>  }
>  
> -void
> -interfaces(void)
> -{
> - size_t needed;
> - int mib[6];
> - char *buf = NULL, *lim, *next;
> - struct rt_msghdr *rtm;
> -
> - mib[0] = CTL_NET;
> - mib[1] = PF_ROUTE;
> - mib[2] = 0; /* protocol */
> - mib[3] = 0; /* wildcard address family */
> - mib[4] = NET_RT_IFLIST;
> - mib[5] = 0; /* no flags */
> - if (sysctl(mib, 6, NULL, , NULL, 0) < 0)
> - err(1, "route-sysctl-estimate");
> - if (needed) {
> - if ((buf = malloc(needed)) == NULL)
> - err(1, "malloc");
> - if (sysctl(mib, 6, buf, , NULL, 0) < 0)
> - err(1, "actual retrieval of interface table");
> - lim = buf + needed;
> - for (next = buf; next < lim; next += rtm->rtm_msglen) {
> - rtm = (struct rt_msghdr *)next;
> - print_rtmsg(rtm, rtm->rtm_msglen);
> - }
> - free(buf);
> - }
> -}
> -
>  void
>  monitor(int argc, char *argv[])
>  {
> @@ -1107,10 +1076,6 @@ monitor(int argc, char *argv[])
>   time_t now;
>  
>   verbose = 1;
> - if (debugonly) {
> - interfaces();
> - exit(0);
> - }
>   for (;;) {
>   if ((n = read(s, msg, sizeof(msg))) == -1) {
>   if (errno == EINTR)
> 
> -- 
> I'm not entirely sure you are real.
> 

Re: pf pledge and ioctls

[Sebastian Benoit]

Nick Owens(misch...@offblast.org) on 2018.04.30 18:40:34 -0700:
> hi tech@,
> 
> i've written a program in go which gathers information from pf for
> exporting to prometheus. my go program invokes several ioctls on /dev/pf to
> do this. however, i recently looked at pledge'ing my program, but i've
> noticed that not all of the pf ioctls that i use are included in the pf
> pledge. currently the missing ioctls my program needs are DIOCGETQUEUES,
> and DIOCGETQSTATS, but i wonder if there is a reason why these and other
> ioctls are not included in the pf pledge.
> 
> so some questions - would there be a problem with adding these ioctls? what
> about adding the rest of pf ioctls to the pf pledge? and last, would it
> make sense to split the pf pledge to "pf" for full control and "rpf" for
> read only access such as what my metrics gathering program needs?

Not all features are useable when pledged. Features exposed through pledge
are always balanced by what programms need, what still is understandable by
users of pledge, what is considered safe and by how programs should be
designed.

In this case, you can use priveledge seperation and move the functions that
cannot be pledged into a seperate process.

Re: rssi comparison threshold

[Peter Hessler]

On 2018 May 01 (Tue) at 11:20:54 +0200 (+0200), Stefan Sperling wrote:
:On Mon, Apr 30, 2018 at 08:57:23AM +0200, Peter Hessler wrote:
:> On 2018 Apr 29 (Sun) at 11:51:26 +0200 (+0200), Stefan Sperling wrote:
:> :This diff tries to avoid situations where background scans play
:> :ping-pong between different APs with nearly equal RSSI, as
:> :observed by phessler.
:> :
:> :Not all drivers represent RSSI values in dBm or percentage, so the
:> :diff includes the possibility for drivers to override the new RSSI
:> :comparison function. However, since the threshold is rather low
:> :applying this to all drivers for now should not do any harm, unless
:> :there is a driver where the RSSI value range is ridiculously small.
:> :I'm not aware of any such driver at present.
:> :
:> 
:> I'm concerned about two things.
:> 
:> The usage is confusing, because you pass two incompatible things to be
:> compared.  I would prefer ieee80211_node_cmprssi(ic, rssi_one, rssi_two).
:
:Agreed. Updated diff below. I've chosen to make it compare two nodes
:since the function lives in the nodes namespace. It makes the function
:a bit less flexible as it cannot be used to compare naked rssi values.
:But that's not really needed for the problem at hand; after all, we
:want to compare APs.
:
:> Also, in the case where ni->ni_rssi has a very weak signal, the second
:> comparison can underflow.
:
:Indeed. Thanks for catching this!
:

OK

:Index: ieee80211_node.c
:===
:RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
:retrieving revision 1.129
:diff -u -p -r1.129 ieee80211_node.c
:--- ieee80211_node.c   28 Apr 2018 14:49:07 -  1.129
:+++ ieee80211_node.c   30 Apr 2018 07:30:43 -
:@@ -67,6 +67,8 @@ u_int8_t ieee80211_node_getrssi(struct i
: const struct ieee80211_node *);
: int ieee80211_node_checkrssi(struct ieee80211com *,
: const struct ieee80211_node *);
:+int ieee80211_node_cmprssi(struct ieee80211com *,
:+const struct ieee80211_node *, const struct ieee80211_node *);
: void ieee80211_setup_node(struct ieee80211com *, struct ieee80211_node *,
: const u_int8_t *);
: void ieee80211_free_node(struct ieee80211com *, struct ieee80211_node *);
:@@ -133,6 +135,7 @@ ieee80211_node_attach(struct ifnet *ifp)
:   ic->ic_node_copy = ieee80211_node_copy;
:   ic->ic_node_getrssi = ieee80211_node_getrssi;
:   ic->ic_node_checkrssi = ieee80211_node_checkrssi;
:+  ic->ic_node_cmprssi = ieee80211_node_cmprssi;
:   ic->ic_scangen = 1;
:   ic->ic_max_nnodes = ieee80211_cache_size;
: 
:@@ -761,12 +764,15 @@ ieee80211_end_scan(struct ifnet *ifp)
: 
:   if (ic->ic_caps & IEEE80211_C_SCANALLBAND) {
:   if (IEEE80211_IS_CHAN_2GHZ(ni->ni_chan) &&
:-  (selbs2 == NULL || ni->ni_rssi > selbs2->ni_rssi))
:+  (selbs2 == NULL ||
:+  ic->ic_node_cmprssi(ic, ni, selbs2) > 0))
:   selbs2 = ni;
:   else if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) &&
:-  (selbs5 == NULL || ni->ni_rssi > selbs5->ni_rssi))
:+  (selbs5 == NULL ||
:+  ic->ic_node_cmprssi(ic, ni, selbs5) > 0))
:   selbs5 = ni;
:-  } else if (selbs == NULL || ni->ni_rssi > selbs->ni_rssi)
:+  } else if (selbs == NULL ||
:+  ic->ic_node_cmprssi(ic, ni, selbs) > 0)
:   selbs = ni;
:   }
: 
:@@ -782,9 +788,12 @@ ieee80211_end_scan(struct ifnet *ifp)
:*/
:   if (selbs5 && selbs5->ni_rssi > min_5ghz_rssi)
:   selbs = selbs5;
:-  else if (selbs5 && selbs2)
:-  selbs = (selbs5->ni_rssi >= selbs2->ni_rssi ? selbs5 : selbs2);
:-  else if (selbs2)
:+  else if (selbs5 && selbs2) {
:+  if (ic->ic_node_cmprssi(ic, selbs5, selbs2) >= 0)
:+  selbs = selbs5;
:+  else
:+  selbs = selbs2;
:+  } else if (selbs2)
:   selbs = selbs2;
:   else if (selbs5)
:   selbs = selbs5;
:@@ -989,6 +998,38 @@ ieee80211_node_checkrssi(struct ieee8021
:   IEEE80211_RSSI_THRES_2GHZ :
:   IEEE80211_RSSI_THRES_5GHZ;
:   return (ni->ni_rssi >= (u_int8_t)thres);
:+}
:+
:+/*
:+ * Determine if RSSI values of two nodes are significantly different.
:+ * This function assumes RSSI values are represented either in dBm or
:+ * as a percentage of ic_max_rssi. Drivers should override this function
:+ * in case their RSSI values use a different representation.
:+ */
:+int
:+ieee80211_node_cmprssi(struct ieee80211com *ic,
:+const struct ieee80211_node *ni1, const struct ieee80211_node *ni2)
:+{
:+  uint8_t thres;
:+
:+  if (ic->ic_max_rssi)
:+  thres = IEEE80211_RSSI_CMP_THRES_RATIO;
:+  else
:+  thres = IEEE80211_RSSI_CMP_THRES_DBM;
:+
:+  

a first batch of const for x509

[Theo Buehler]

Here's a few X509_* functions with added const. This was part of a bulk
by sthen with no fallout. One minor difference: the diff I sent to
Stuart contained a mistake in X509_signature_print().

Most of these are straightforward.  X509_ALGOR_get0() required a bit of
churn in *{pub,priv}_decode* functions. Among those, gost stands out
with an ugly cast, but I'd rather not touch these more than necessary.

Index: asn1/t_x509.c
===
RCS file: /cvs/src/lib/libcrypto/asn1/t_x509.c,v
retrieving revision 1.29
diff -u -p -r1.29 t_x509.c
--- asn1/t_x509.c   25 Apr 2018 19:58:53 -  1.29
+++ asn1/t_x509.c   1 May 2018 16:18:46 -
@@ -321,7 +321,7 @@ X509_signature_dump(BIO *bp, const ASN1_
 }
 
 int
-X509_signature_print(BIO *bp, X509_ALGOR *sigalg, ASN1_STRING *sig)
+X509_signature_print(BIO *bp, const X509_ALGOR *sigalg, const ASN1_STRING *sig)
 {
int sig_nid;
if (BIO_puts(bp, "Signature Algorithm: ") <= 0)
Index: asn1/x_algor.c
===
RCS file: /cvs/src/lib/libcrypto/asn1/x_algor.c,v
retrieving revision 1.21
diff -u -p -r1.21 x_algor.c
--- asn1/x_algor.c  24 Jul 2015 15:09:52 -  1.21
+++ asn1/x_algor.c  1 May 2018 16:18:46 -
@@ -176,8 +176,8 @@ X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OB
 }
 
 void
-X509_ALGOR_get0(ASN1_OBJECT **paobj, int *pptype, void **ppval,
-X509_ALGOR *algor)
+X509_ALGOR_get0(const ASN1_OBJECT **paobj, int *pptype, const void **ppval,
+const X509_ALGOR *algor)
 {
if (paobj)
*paobj = algor->algorithm;
Index: asn1/x_x509a.c
===
RCS file: /cvs/src/lib/libcrypto/asn1/x_x509a.c,v
retrieving revision 1.14
diff -u -p -r1.14 x_x509a.c
--- asn1/x_x509a.c  14 Feb 2015 15:28:39 -  1.14
+++ asn1/x_x509a.c  1 May 2018 16:18:46 -
@@ -154,7 +154,7 @@ aux_get(X509 *x)
 }
 
 int
-X509_alias_set1(X509 *x, unsigned char *name, int len)
+X509_alias_set1(X509 *x, const unsigned char *name, int len)
 {
X509_CERT_AUX *aux;
if (!name) {
@@ -172,7 +172,7 @@ X509_alias_set1(X509 *x, unsigned char *
 }
 
 int
-X509_keyid_set1(X509 *x, unsigned char *id, int len)
+X509_keyid_set1(X509 *x, const unsigned char *id, int len)
 {
X509_CERT_AUX *aux;
if (!id) {
@@ -210,7 +210,7 @@ X509_keyid_get0(X509 *x, int *len)
 }
 
 int
-X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
+X509_add1_trust_object(X509 *x, const ASN1_OBJECT *obj)
 {
X509_CERT_AUX *aux;
ASN1_OBJECT *objtmp;
@@ -232,7 +232,7 @@ err:
 }
 
 int
-X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj)
+X509_add1_reject_object(X509 *x, const ASN1_OBJECT *obj)
 {
X509_CERT_AUX *aux;
ASN1_OBJECT *objtmp;
Index: dh/dh_ameth.c
===
RCS file: /cvs/src/lib/libcrypto/dh/dh_ameth.c,v
retrieving revision 1.14
diff -u -p -r1.14 dh_ameth.c
--- dh/dh_ameth.c   29 Jan 2017 17:49:22 -  1.14
+++ dh/dh_ameth.c   1 May 2018 16:18:46 -
@@ -78,8 +78,8 @@ dh_pub_decode(EVP_PKEY *pkey, X509_PUBKE
const unsigned char *p, *pm;
int pklen, pmlen;
int ptype;
-   void *pval;
-   ASN1_STRING *pstr;
+   const void *pval;
+   const ASN1_STRING *pstr;
X509_ALGOR *palg;
ASN1_INTEGER *public_key = NULL;
DH *dh = NULL;
@@ -185,8 +185,8 @@ dh_priv_decode(EVP_PKEY *pkey, PKCS8_PRI
const unsigned char *p, *pm;
int pklen, pmlen;
int ptype;
-   void *pval;
-   ASN1_STRING *pstr;
+   const void *pval;
+   const ASN1_STRING *pstr;
X509_ALGOR *palg;
ASN1_INTEGER *privkey = NULL;
DH *dh = NULL;
Index: dsa/dsa_ameth.c
===
RCS file: /cvs/src/lib/libcrypto/dsa/dsa_ameth.c,v
retrieving revision 1.23
diff -u -p -r1.23 dsa_ameth.c
--- dsa/dsa_ameth.c 29 Jan 2017 17:49:22 -  1.23
+++ dsa/dsa_ameth.c 1 May 2018 16:18:46 -
@@ -75,8 +75,8 @@ dsa_pub_decode(EVP_PKEY *pkey, X509_PUBK
const unsigned char *p, *pm;
int pklen, pmlen;
int ptype;
-   void *pval;
-   ASN1_STRING *pstr;
+   const void *pval;
+   const ASN1_STRING *pstr;
X509_ALGOR *palg;
ASN1_INTEGER *public_key = NULL;
 
@@ -184,8 +184,8 @@ dsa_priv_decode(EVP_PKEY *pkey, PKCS8_PR
const unsigned char *p, *pm;
int pklen, pmlen;
int ptype;
-   void *pval;
-   ASN1_STRING *pstr;
+   const void *pval;
+   const ASN1_STRING *pstr;
X509_ALGOR *palg;
ASN1_INTEGER *privkey = NULL;
BN_CTX *ctx = NULL;
Index: ec/ec_ameth.c
===
RCS file: /cvs/src/lib/libcrypto/ec/ec_ameth.c,v
retrieving revision 1.19
diff -u -p -r1.19 ec_ameth.c
--- ec/ec_ameth.c 

Re: avoid uninitialised attr in rasops_scrollback()

[Miod Vallat]

> scrollback isn't part of wsdisplay_emulops but rather wsdisplay_accessops.
> It isn't clear how to properly get an attr in wsscrollback()
> GETCHAR/getchar is part of wsmoused and sc->sc_accessops->getchar
> may be NULL.

Ok, ignore that. There is a simpler solution.

Index: rasops.c
===
RCS file: /OpenBSD/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.53
diff -u -p -r1.53 rasops.c
--- rasops.c27 Apr 2018 21:36:12 -  1.53
+++ rasops.c1 May 2018 17:02:43 -
@@ -1373,6 +1373,7 @@ struct rasops_screen {
int rs_visible;
int rs_crow;
int rs_ccol;
+   long rs_defattr;
 
int rs_sbscreens;
 #define RS_SCROLLBACK_SCREENS 5
@@ -1412,10 +1413,11 @@ rasops_alloc_screen(void *v, void **cook
scr->rs_visible = (ri->ri_nscreens == 0);
scr->rs_crow = -1;
scr->rs_ccol = -1;
+   scr->rs_defattr = *attrp;
 
for (i = 0; i < scr->rs_dispoffset; i++) {
scr->rs_bs[i].uc = ' ';
-   scr->rs_bs[i].attr = *attrp;
+   scr->rs_bs[i].attr = scr->rs_defattr;
}
 
if (ri->ri_bs && scr->rs_visible) {
@@ -1425,7 +1427,8 @@ rasops_alloc_screen(void *v, void **cook
} else {
for (i = 0; i < ri->ri_rows * ri->ri_cols; i++) {
scr->rs_bs[scr->rs_dispoffset + i].uc = ' ';
-   scr->rs_bs[scr->rs_dispoffset + i].attr = *attrp;
+   scr->rs_bs[scr->rs_dispoffset + i].attr =
+   scr->rs_defattr;
}
}
 
@@ -1906,7 +1909,6 @@ rasops_scrollback(void *v, void *cookie,
struct rasops_info *ri = v;
struct rasops_screen *scr = cookie;
int row, col, oldvoff;
-   long attr;
 
oldvoff = scr->rs_visibleoffset;
 
@@ -1927,7 +1929,7 @@ rasops_scrollback(void *v, void *cookie,
return;
 
rasops_cursor(ri, 0, 0, 0);
-   ri->ri_eraserows(ri, 0, ri->ri_rows, attr);
+   ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr);
for (row = 0; row < ri->ri_rows; row++) {
for (col = 0; col < ri->ri_cols; col++) {
int off = row * scr->rs_ri->ri_cols + col +

Re: Push the netlock down in in_control()

[Theo Buehler]

On Mon, Apr 30, 2018 at 02:55:21PM +0200, Martin Pieuchot wrote:
> On 30/04/18(Mon) 12:00, Theo Buehler wrote:
> > With mpi's encouragement and guidance, here's a diff that reduces the
> > scope of the NET_LOCK() a bit.
> > 
> > in_control() is the only caller of mrt_ioctl() and the latter is a
> > simple function only requiring a read lock.
> > 
> > There are only a handful callers of in_ioctl(). The two switches create
> > relatively tangled codepaths, so this will need some refactoring before
> > we can push the lock further down and split the read and write cases.
> > For now, establish a single exit point, grab the netlock at the
> > beginning and release it at the end.
> 
> That makes sense, the same could be done for in6_ioctl().

Here's the corresponding diff for in6_control().

Push the diff down into mrt6_ioctl() and in6_ioctl(). Much like the IPv4
version, mrt6_ioctl() only needs a read lock.

in6_ioctl() is a bit messier. It starts off with a call to nd6_ioctl(),
which only needs a read lock. In the diff, I pushed the NET_RLOCK() down
into nd6_ioctl(), but perhaps it would be better to keep the locking
inside in6_ioctl(), like this:

switch (cmd) {
case SIOCGIFINFO_IN6:
case SIOCGNBRINFO_IN6:
NET_RLOCK();
error = nd6_ioctl(cmd, data, ifp);
NET_RUNLOCK();
return error;
}

I tested this as well as I could, but I don't usually use IPv6, so I
dabbled a bit with it on my home network and I tried to use the netinet6
regress tests, only with moderate success.

A test from people who actually use IPv6 would be welcome.

Index: sys/netinet6/in6.c
===
RCS file: /var/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.222
diff -u -p -r1.222 in6.c
--- sys/netinet6/in6.c  24 Apr 2018 19:53:38 -  1.222
+++ sys/netinet6/in6.c  1 May 2018 09:14:07 -
@@ -183,7 +183,6 @@ in6_control(struct socket *so, u_long cm
int privileged;
int error;
 
-   NET_LOCK();
privileged = 0;
if ((so->so_state & SS_PRIV) != 0)
privileged++;
@@ -200,7 +199,6 @@ in6_control(struct socket *so, u_long cm
break;
}
 
-   NET_UNLOCK();
return error;
 }
 
@@ -211,12 +209,11 @@ in6_ioctl(u_long cmd, caddr_t data, stru
struct  in6_ifaddr *ia6 = NULL;
struct  in6_aliasreq *ifra = (struct in6_aliasreq *)data;
struct sockaddr_in6 *sa6;
+   int error = 0;
 
if (ifp == NULL)
return (ENXIO);
 
-   NET_ASSERT_LOCKED();
-
switch (cmd) {
case SIOCGIFINFO_IN6:
case SIOCGNBRINFO_IN6:
@@ -252,13 +249,16 @@ in6_ioctl(u_long cmd, caddr_t data, stru
case SIOCSIFNETMASK:
/*
 * Do not pass those ioctl to driver handler since they are not
-* properly setup. Instead just error out.
+* properly set up. Instead just error out.
 */
return (EINVAL);
default:
sa6 = NULL;
break;
}
+
+   NET_LOCK();
+
if (sa6 && sa6->sin6_family == AF_INET6) {
if (IN6_IS_ADDR_LINKLOCAL(>sin6_addr)) {
if (sa6->sin6_addr.s6_addr16[1] == 0) {
@@ -267,12 +267,15 @@ in6_ioctl(u_long cmd, caddr_t data, stru
htons(ifp->if_index);
} else if (sa6->sin6_addr.s6_addr16[1] !=
htons(ifp->if_index)) {
-   return (EINVAL);/* link ID contradicts 
*/
+   error = EINVAL; /* link ID contradicts */
+   goto err;
}
if (sa6->sin6_scope_id) {
if (sa6->sin6_scope_id !=
-   (u_int32_t)ifp->if_index)
-   return (EINVAL);
+   (u_int32_t)ifp->if_index) {
+   error = EINVAL;
+   goto err;
+   }
sa6->sin6_scope_id = 0; /* XXX: good way? */
}
}
@@ -289,8 +292,10 @@ in6_ioctl(u_long cmd, caddr_t data, stru
 * interface address from the day one, we consider "remove the
 * first one" semantics to be not preferable.
 */
-   if (ia6 == NULL)
-   return (EADDRNOTAVAIL);
+   if (ia6 == NULL) {
+   error = EADDRNOTAVAIL;
+   goto err;
+   }
/* FALLTHROUGH */
case SIOCAIFADDR_IN6:
/*
@@ -298,11 +303,14 @@ in6_ioctl(u_long cmd, caddr_t data, stru
 * the corresponding 

Re: const for BIO_new, BIO_set and BIO_{f,s}_*

[Theo Buehler]

On Mon, Apr 30, 2018 at 09:39:18PM +0200, Theo Buehler wrote:
> Here is a straightforward diff that converts BIO_new(), BIO_set() and the
> BIO_{f,s}_*() functions to match OpenSSL wrt const. This was part of a
> larger diff that was run through a bulk by sthen and produced no fallout.
> 

There were a few more of these in sthen's bulk:

Index: evp/bio_b64.c
===
RCS file: /var/cvs/src/lib/libcrypto/evp/bio_b64.c,v
retrieving revision 1.20
diff -u -p -r1.20 bio_b64.c
--- evp/bio_b64.c   7 Feb 2015 13:19:15 -   1.20
+++ evp/bio_b64.c   1 May 2018 15:52:07 -
@@ -91,7 +91,7 @@ typedef struct b64_struct {
char tmp[B64_BLOCK_SIZE];
 } BIO_B64_CTX;
 
-static BIO_METHOD methods_b64 = {
+static const BIO_METHOD methods_b64 = {
.type = BIO_TYPE_BASE64,
.name = "base64 encoding",
.bwrite = b64_write,
@@ -103,7 +103,7 @@ static BIO_METHOD methods_b64 = {
.callback_ctrl = b64_callback_ctrl
 };
 
-BIO_METHOD *
+const BIO_METHOD *
 BIO_f_base64(void)
 {
return (_b64);
Index: evp/bio_enc.c
===
RCS file: /var/cvs/src/lib/libcrypto/evp/bio_enc.c,v
retrieving revision 1.20
diff -u -p -r1.20 bio_enc.c
--- evp/bio_enc.c   2 May 2017 03:59:44 -   1.20
+++ evp/bio_enc.c   1 May 2018 15:52:07 -
@@ -87,7 +87,7 @@ typedef struct enc_struct {
char buf[ENC_BLOCK_SIZE + BUF_OFFSET + 2];
 } BIO_ENC_CTX;
 
-static BIO_METHOD methods_enc = {
+static const BIO_METHOD methods_enc = {
.type = BIO_TYPE_CIPHER,
.name = "cipher",
.bwrite = enc_write,
@@ -98,7 +98,7 @@ static BIO_METHOD methods_enc = {
.callback_ctrl = enc_callback_ctrl
 };
 
-BIO_METHOD *
+const BIO_METHOD *
 BIO_f_cipher(void)
 {
return (_enc);
Index: evp/bio_md.c
===
RCS file: /var/cvs/src/lib/libcrypto/evp/bio_md.c,v
retrieving revision 1.14
diff -u -p -r1.14 bio_md.c
--- evp/bio_md.c11 Jul 2014 08:44:48 -  1.14
+++ evp/bio_md.c1 May 2018 15:52:07 -
@@ -74,7 +74,7 @@ static int md_new(BIO *h);
 static int md_free(BIO *data);
 static long md_callback_ctrl(BIO *h, int cmd, bio_info_cb *fp);
 
-static BIO_METHOD methods_md = {
+static const BIO_METHOD methods_md = {
.type = BIO_TYPE_MD,
.name = "message digest",
.bwrite = md_write,
@@ -86,7 +86,7 @@ static BIO_METHOD methods_md = {
.callback_ctrl = md_callback_ctrl
 };
 
-BIO_METHOD *
+const BIO_METHOD *
 BIO_f_md(void)
 {
return (_md);
Index: evp/evp.h
===
RCS file: /var/cvs/src/lib/libcrypto/evp/evp.h,v
retrieving revision 1.58
diff -u -p -r1.58 evp.h
--- evp/evp.h   20 Feb 2018 18:05:28 -  1.58
+++ evp/evp.h   1 May 2018 15:52:07 -
@@ -651,9 +651,9 @@ int EVP_CIPHER_CTX_ctrl(EVP_CIPHER_CTX *
 int EVP_CIPHER_CTX_rand_key(EVP_CIPHER_CTX *ctx, unsigned char *key);
 
 #ifndef OPENSSL_NO_BIO
-BIO_METHOD *BIO_f_md(void);
-BIO_METHOD *BIO_f_base64(void);
-BIO_METHOD *BIO_f_cipher(void);
+const BIO_METHOD *BIO_f_md(void);
+const BIO_METHOD *BIO_f_base64(void);
+const BIO_METHOD *BIO_f_cipher(void);
 void BIO_set_cipher(BIO *b, const EVP_CIPHER *c, const unsigned char *k,
 const unsigned char *i, int enc);
 #endif