Re: parse.y: strndup() in cmdline_symset()

2018-11-06 Thread Claudio Jeker
On Wed, Nov 07, 2018 at 12:55:34PM +0800, Michael Mikonos wrote:
> ping?

OK claudio@
 
> On Thu, Nov 01, 2018 at 04:14:53PM +0800, Michael Mikonos wrote:
> > Hello,
> > 
> > When I updated cmdline_symset() in parse.y in the following commit
> > I missed src/sbin/{iked,ipsecctl,pfctl}/parse.y. OK to update them?
> > 
> > https://marc.info/?l=openbsd-cvs=153631079505256=2
> > 
> > 
> > Index: iked/parse.y
> > ===
> > RCS file: /cvs/src/sbin/iked/parse.y,v
> > retrieving revision 1.76
> > diff -u -p -u -r1.76 parse.y
> > --- iked/parse.y1 Nov 2018 00:18:44 -   1.76
> > +++ iked/parse.y1 Nov 2018 07:03:31 -
> > @@ -1660,17 +1660,13 @@ cmdline_symset(char *s)
> >  {
> > char*sym, *val;
> > int ret;
> > -   size_t  len;
> >  
> > if ((val = strrchr(s, '=')) == NULL)
> > return (-1);
> >  
> > -   len = strlen(s) - strlen(val) + 1;
> > -   if ((sym = malloc(len)) == NULL)
> > +   sym = strndup(s, val - s);
> > +   if (sym == NULL)
> > err(1, "%s", __func__);
> > -
> > -   strlcpy(sym, s, len);
> > -
> > ret = symset(sym, val + 1, 1);
> > free(sym);
> >  
> > Index: ipsecctl/parse.y
> > ===
> > RCS file: /cvs/src/sbin/ipsecctl/parse.y,v
> > retrieving revision 1.174
> > diff -u -p -u -r1.174 parse.y
> > --- ipsecctl/parse.y1 Nov 2018 00:18:44 -   1.174
> > +++ ipsecctl/parse.y1 Nov 2018 07:03:32 -
> > @@ -1426,17 +1426,13 @@ cmdline_symset(char *s)
> >  {
> > char*sym, *val;
> > int ret;
> > -   size_t  len;
> >  
> > if ((val = strrchr(s, '=')) == NULL)
> > return (-1);
> >  
> > -   len = strlen(s) - strlen(val) + 1;
> > -   if ((sym = malloc(len)) == NULL)
> > +   sym = strndup(s, val - s);
> > +   if (sym == NULL)
> > err(1, "%s", __func__);
> > -
> > -   strlcpy(sym, s, len);
> > -
> > ret = symset(sym, val + 1, 1);
> > free(sym);
> >  
> > Index: pfctl/parse.y
> > ===
> > RCS file: /cvs/src/sbin/pfctl/parse.y,v
> > retrieving revision 1.685
> > diff -u -p -u -r1.685 parse.y
> > --- pfctl/parse.y   1 Nov 2018 00:18:44 -   1.685
> > +++ pfctl/parse.y   1 Nov 2018 07:03:34 -
> > @@ -5564,11 +5564,9 @@ pfctl_cmdline_symset(char *s)
> > if ((val = strrchr(s, '=')) == NULL)
> > return (-1);
> >  
> > -   if ((sym = malloc(strlen(s) - strlen(val) + 1)) == NULL)
> > +   sym = strndup(s, val - s);  
> > +   if (sym == NULL);
> > err(1, "%s", __func__);
> > -
> > -   strlcpy(sym, s, strlen(s) - strlen(val) + 1);
> > -
> > ret = symset(sym, val + 1, 1);
> > free(sym);
> >  
> 

-- 
:wq Claudio



Re: parse.y: strndup() in cmdline_symset()

2018-11-06 Thread Alexandr Nedvedicky
Hello,

the change looks good to me. (I was delaying my OK hoping
someone else will chip-in with OK for iked & ipsecctl stuff).

IMO It makes sense to keep parse.y code in sync where possible,
so go for it.

OK sashan.


On Thu, Nov 01, 2018 at 04:14:53PM +0800, Michael Mikonos wrote:
> Hello,
> 
> When I updated cmdline_symset() in parse.y in the following commit
> I missed src/sbin/{iked,ipsecctl,pfctl}/parse.y. OK to update them?
> 
> https://marc.info/?l=openbsd-cvs=153631079505256=2
> 
> 
> Index: iked/parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.76
> diff -u -p -u -r1.76 parse.y
> --- iked/parse.y  1 Nov 2018 00:18:44 -   1.76
> +++ iked/parse.y  1 Nov 2018 07:03:31 -
> @@ -1660,17 +1660,13 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
>  
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> + sym = strndup(s, val - s);
> + if (sym == NULL)
>   err(1, "%s", __func__);
> -
> - strlcpy(sym, s, len);
> -
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: ipsecctl/parse.y
> ===
> RCS file: /cvs/src/sbin/ipsecctl/parse.y,v
> retrieving revision 1.174
> diff -u -p -u -r1.174 parse.y
> --- ipsecctl/parse.y  1 Nov 2018 00:18:44 -   1.174
> +++ ipsecctl/parse.y  1 Nov 2018 07:03:32 -
> @@ -1426,17 +1426,13 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
>  
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> + sym = strndup(s, val - s);
> + if (sym == NULL)
>   err(1, "%s", __func__);
> -
> - strlcpy(sym, s, len);
> -
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: pfctl/parse.y
> ===
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.685
> diff -u -p -u -r1.685 parse.y
> --- pfctl/parse.y 1 Nov 2018 00:18:44 -   1.685
> +++ pfctl/parse.y 1 Nov 2018 07:03:34 -
> @@ -5564,11 +5564,9 @@ pfctl_cmdline_symset(char *s)
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
>  
> - if ((sym = malloc(strlen(s) - strlen(val) + 1)) == NULL)
> + sym = strndup(s, val - s);  
> + if (sym == NULL);
>   err(1, "%s", __func__);
> -
> - strlcpy(sym, s, strlen(s) - strlen(val) + 1);
> -
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> 



Re: AMD64 buffer cache 4GB cap anything new, multiqueueing plans? ("64bit DMA on amd64" cont)

2018-11-06 Thread Philip Guenther
On Tue, Nov 6, 2018 at 9:51 PM Joseph Mayer 
wrote:

> Previously there was a years-long thread about a 4GB (32bit) buffer
> cache constraint on AMD64, ref
> https://marc.info/?t=14682443664=1=2 .
>
> What I gather is,
>
>  * The problematique is that on AMD64, DMA is limited to 32bit
>addressing, I guess because unlike AMD64 arch CPU:s which all have
>64bit DMA support, popular PCI accessories and supporting hardware
>out there like bridges, have DMA functionality limited to 32bit
>addressing.
>

My read of that thread, particularly Theo's comments, is that no one
actually demonstrated a case where lack of 64bit DMA caused any problems or
limitations.

If you have a system and use where lack of 64bit DMA creates a performance
limitation, then describe it and, *more importantly*, *why* you think the
DMA limit is involved.


Philip Guenther


switchd(8): change default listen port to the standardized OpenFlow port

2018-11-06 Thread Ayaka Koshibe
Hi all,

Currently, switchd(8) defaults to listening on port 6633, which was
the defacto port value used by OpenFlow. A decent chunk of OpenFlow
controllers have switched over to the IANA standardized OpenFlow port,
6653. While testing the change, I noticed that switchd(8) will listen on
random ports if one isn't specified in switchd.conf(5), e.g:

listen on 127.0.0.1

The following consolidates the #defines for port values, and also sets a
default listen port when it isn't specified in switchd.conf(5).
 
Comments/OKs?


Thanks,
Ayaka

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/switchd/parse.y,v
retrieving revision 1.12
diff -u -p -u -r1.12 parse.y
--- parse.y 1 Nov 2018 00:18:44 -   1.12
+++ parse.y 7 Nov 2018 06:35:03 -
@@ -146,6 +146,8 @@ listen  : LISTEN ON STRING opttls port {
YYERROR;
}
free($3);
+   ((struct sockaddr_in *)>sc_server.srv_addr)
+   ->sin_port = htons(SWITCHD_CTLR_PORT);
}
;
 
@@ -627,7 +629,7 @@ parse_config(const char *filename, struc
 
conf = sc;
 
-   /* Set the default 0.0.0.0 6633/tcp */
+   /* Set the default 0.0.0.0 6653/tcp */
memset(>sc_server.srv_addr, 0, sizeof(conf->sc_server.srv_addr));
sin4 = (struct sockaddr_in *)>sc_server.srv_addr;
sin4->sin_family = AF_INET;
Index: types.h
===
RCS file: /cvs/src/usr.sbin/switchd/types.h,v
retrieving revision 1.10
diff -u -p -u -r1.10 types.h
--- types.h 18 Nov 2016 16:49:35 -  1.10
+++ types.h 7 Nov 2018 06:35:03 -
@@ -37,8 +37,7 @@
 #define SWITCHD_MAX_TAP256
 #define SWITCHD_MAX_SESSIONS   0x
 
-#define SWITCHD_CTLR_PORT  6633/* Previously used by OpenFlow */
-#define SWITCHD_CTLR_IANA_PORT 6653/* Assigned by IANA for OpenFlow */
+#define SWITCHD_CTLR_PORT  6653/* Assigned by IANA for OpenFlow */
 
 #define SWITCHD_CACHE_MAX  4096/* Default MAC address cache limit */
 #define SWITCHD_CACHE_TIMEOUT  240 /* t/o in seconds for learned MACs */



Re: malloc.conf heads up

2018-11-06 Thread Otto Moerbeek
On Wed, Nov 07, 2018 at 07:23:35AM +0100, Otto Moerbeek wrote:

> Hi,
> 
> We are moving away from the /etc/malloc.conf symbolic link to a new sysctl:
> 
>   $ sysctl vm.malloc_conf   
>   vm.malloc_conf=C
> 
> This will allow unveiled and chrooted processes to access the malloc
> options without having to do anything special in the code or chroot
> dir.
> 
> As I often get this question: for some extra protection, use C or CJ,

I meant to type C or CF!

> accept some performance impact.  For development, bug hunting and/or
> extra securty use S, with more performance impact.
> 
> Note that with default options, malloc already has quite some
> protection features.
> 
> Upcoming snapshots will contain this.
> 
>   -Otto
> 



malloc.conf heads up

2018-11-06 Thread Otto Moerbeek
Hi,

We are moving away from the /etc/malloc.conf symbolic link to a new sysctl:

$ sysctl vm.malloc_conf   
vm.malloc_conf=C

This will allow unveiled and chrooted processes to access the malloc
options without having to do anything special in the code or chroot
dir.

As I often get this question: for some extra protection, use C or CJ,
accept some performance impact.  For development, bug hunting and/or
extra securty use S, with more performance impact.

Note that with default options, malloc already has quite some
protection features.

Upcoming snapshots will contain this.

-Otto



AMD64 buffer cache 4GB cap anything new, multiqueueing plans? ("64bit DMA on amd64" cont)

2018-11-06 Thread Joseph Mayer
Hi,

Previously there was a years-long thread about a 4GB (32bit) buffer
cache constraint on AMD64, ref
https://marc.info/?t=14682443664=1=2 .

What I gather is,

 * The problematique is that on AMD64, DMA is limited to 32bit
   addressing, I guess because unlike AMD64 arch CPU:s which all have
   64bit DMA support, popular PCI accessories and supporting hardware
   out there like bridges, have DMA functionality limited to 32bit
   addressing.

   (Is this a feature of lower-quality hardware, or for very old PCI
   devices, or is it systemic to the whole AMD64 ecosystem today?

   Could a system be configured to use 64bit DMA on AMD64 and be
   expected to work presuming recent or higher-quality / well-selected
   hardware?)

 * The OS asks the disk hardware to load disk data to give memory
   locations via DMA, and then userland fread() and mmap() is fed with
   that data - no need for further data moving or mapping. This is the
   dynamics leading to the 4GB cap.

   And, the 4GB cap is kind of constraining for any computer with much
   RAM and lots of disk reading, as it means lots of reads that
   wouldn't need to hit the disk (as it could be cached using all this
   free memory) isn't cached and is directed to disk anyhow which takes
   a lot of time, yes?

 * This was recognized a long time ago and Bob wrote a solution in
   the form of a "buffer cache flipper" that would push buffer cache
   data out of the 32bit area (to "high memory" as in >32bit) hence
   lifting the limit, via a "(generic) backpressure" mechanism that as
   a bonus used the DMA engine to do the memory moving, I guess this
   means that the buffer cache would be pretty much zero-cost to the
   CPU - sounds incredibly neat!

   And then, it didn't really work, malfunctioned and irritated people
   (was "busted" - for unknown reasons, actually why was it?) and Theo
   wrote it will be fixed in the future.


Has it been fixed since?


Also - when fixed, fread() and mmap() reads to data that's in the
buffer cache will be incredibly fast right, as, in optimal conditions
the mmap:ed addresses will be already-mapped to the buffer cache data
and hence in optimal conditions mmap:ed buffer cache data reads will
have the speed of any memory access, right?


(The ML thread also mentioned an undeadly.org post discussing this
topic, however both searching and browsing I can't find it, the closest
i find is 5 words here
https://undeadly.org/cgi?action=article;sid=20170815171854 - do you
have any URL?)


Last, OpenBSD's biggest limit as an OS seems to be that the disk/file
subsystem is sequential. A modern SSD can read at 2.8GB/sec but that
requires parallellism, without multiqueueing and with small reads e.g.
4KB or smaller, speeds stay around 70-120MB/sec = ~3.5% of the
hardware's potential performance. This would be really worthy goal to
donate to for instance, in particular as OpenBSD leads the way in many
other areas.

Are there any thoughts about implementing this in the future?

Thanks,
Joseph



On ARM64, does OpenBSD have a 32/4GB cap on the buffer cache like on AMD64?

2018-11-06 Thread Joseph Mayer
Hi,

On ARM64, does OpenBSD have a 4GB cap on the buffer cache like on
AMD64?

Thanks,
Joseph



Re: parse.y: strndup() in cmdline_symset()

2018-11-06 Thread Michael Mikonos
ping?

On Thu, Nov 01, 2018 at 04:14:53PM +0800, Michael Mikonos wrote:
> Hello,
> 
> When I updated cmdline_symset() in parse.y in the following commit
> I missed src/sbin/{iked,ipsecctl,pfctl}/parse.y. OK to update them?
> 
> https://marc.info/?l=openbsd-cvs=153631079505256=2
> 
> 
> Index: iked/parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.76
> diff -u -p -u -r1.76 parse.y
> --- iked/parse.y  1 Nov 2018 00:18:44 -   1.76
> +++ iked/parse.y  1 Nov 2018 07:03:31 -
> @@ -1660,17 +1660,13 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
>  
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> + sym = strndup(s, val - s);
> + if (sym == NULL)
>   err(1, "%s", __func__);
> -
> - strlcpy(sym, s, len);
> -
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: ipsecctl/parse.y
> ===
> RCS file: /cvs/src/sbin/ipsecctl/parse.y,v
> retrieving revision 1.174
> diff -u -p -u -r1.174 parse.y
> --- ipsecctl/parse.y  1 Nov 2018 00:18:44 -   1.174
> +++ ipsecctl/parse.y  1 Nov 2018 07:03:32 -
> @@ -1426,17 +1426,13 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
>  
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> + sym = strndup(s, val - s);
> + if (sym == NULL)
>   err(1, "%s", __func__);
> -
> - strlcpy(sym, s, len);
> -
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: pfctl/parse.y
> ===
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.685
> diff -u -p -u -r1.685 parse.y
> --- pfctl/parse.y 1 Nov 2018 00:18:44 -   1.685
> +++ pfctl/parse.y 1 Nov 2018 07:03:34 -
> @@ -5564,11 +5564,9 @@ pfctl_cmdline_symset(char *s)
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
>  
> - if ((sym = malloc(strlen(s) - strlen(val) + 1)) == NULL)
> + sym = strndup(s, val - s);  
> + if (sym == NULL);
>   err(1, "%s", __func__);
> -
> - strlcpy(sym, s, strlen(s) - strlen(val) + 1);
> -
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  



wsmouse/wstpad: remove the strong hysteresis filter

2018-11-06 Thread Ulf Brosziewski
For a while now, wsmouse activates compatibility mode for touchpads by
default.  There have been no complaints about wobbling pointers or
unstable pointer paths.  I conclude that the default filtering is
sufficient and that the type of touchpad which could profit from the
"strong" variant of the hysteresis filter is dying out.  This patch
removes it (and somewhat simplifies related code of the default method).

The patch doesn't change the list of wsmouse parameters in wsconsio.h.
Reading the parameter value for strong hysteresis returns 0, an attempt
to set it results in an error (should someone actually stumble over the
change, I'd like to know that).

OK?


Index: dev/wscons/wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.89
diff -u -p -r1.89 wsconsio.h
--- dev/wscons/wsconsio.h   30 Jul 2018 15:56:30 -  1.89
+++ dev/wscons/wsconsio.h   6 Nov 2018 19:03:49 -
@@ -295,7 +295,8 @@ enum wsmousecfg {
WSMOUSECFG_X_HYSTERESIS,/* retard value for X coordinates */
WSMOUSECFG_Y_HYSTERESIS,/* retard value for Y coordinates */
WSMOUSECFG_DECELERATION,/* threshold (distance) for deceleration */
-   WSMOUSECFG_STRONG_HYSTERESIS,   /* apply the filter continuously */
+   WSMOUSECFG_STRONG_HYSTERESIS,   /* FALSE and read-only, the fea-
+  ture is not supported anymore. */
WSMOUSECFG_SMOOTHING,   /* smoothing factor (0-7) */
 
/*
Index: dev/wscons/wsmouse.c
===
RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
retrieving revision 1.45
diff -u -p -r1.45 wsmouse.c
--- dev/wscons/wsmouse.c7 May 2018 21:58:42 -   1.45
+++ dev/wscons/wsmouse.c6 Nov 2018 19:03:49 -
@@ -625,7 +625,10 @@ set_x(struct position *pos, int x, u_int
}
if ((pos->dx = x - pos->x)) {
pos->x = x;
-   pos->acc_dx += pos->dx;
+   if (pos->dx > 0 == pos->acc_dx > 0)
+   pos->acc_dx += pos->dx;
+   else
+   pos->acc_dx = pos->dx;
*sync |= mask;
}
 }
@@ -641,7 +644,10 @@ set_y(struct position *pos, int y, u_int
}
if ((pos->dy = y - pos->y)) {
pos->y = y;
-   pos->acc_dy += pos->dy;
+   if (pos->dy > 0 == pos->acc_dy > 0)
+   pos->acc_dy += pos->dy;
+   else
+   pos->acc_dy = pos->dy;
*sync |= mask;
}
 }
@@ -860,18 +866,6 @@ wsmouse_mt_update(struct wsmouseinput *i
 int
 wsmouse_hysteresis(struct wsmouseinput *input, struct position *pos)
 {
-
-   if (!(input->filter.h.hysteresis && input->filter.v.hysteresis))
-   return (0);
-
-   if ((pos->dx > 0 && pos->dx > pos->acc_dx)
-  || (pos->dx < 0 && pos->dx < pos->acc_dx))
-   pos->acc_dx = pos->dx;
-
-   if ((pos->dy > 0 && pos->dy > pos->acc_dy)
-  || (pos->dy < 0 && pos->dy < pos->acc_dy))
-   pos->acc_dy = pos->dy;
-
return (abs(pos->acc_dx) < input->filter.h.hysteresis
&& abs(pos->acc_dy) < input->filter.v.hysteresis);
 }
@@ -1489,8 +1483,7 @@ wsmouse_get_params(struct device *sc,
params[i].value = input->filter.dclr;
break;
case WSMOUSECFG_STRONG_HYSTERESIS:
-   params[i].value =
-   !!(input->filter.mode & STRONG_HYSTERESIS);
+   params[i].value = 0; /* The feature has been removed. */
break;
case WSMOUSECFG_SMOOTHING:
params[i].value =
@@ -1569,12 +1562,6 @@ wsmouse_set_params(struct device *sc,
break;
case WSMOUSECFG_DY_MAX:
input->filter.v.dmax = val;
-   break;
-   case WSMOUSECFG_STRONG_HYSTERESIS:
-   if (val)
-   input->filter.mode |= STRONG_HYSTERESIS;
-   else
-   input->filter.mode &= ~STRONG_HYSTERESIS;
break;
case WSMOUSECFG_SMOOTHING:
input->filter.mode &= ~SMOOTHING_MASK;
Index: dev/wscons/wsmouseinput.h
===
RCS file: /cvs/src/sys/dev/wscons/wsmouseinput.h,v
retrieving revision 1.11
diff -u -p -r1.11 wsmouseinput.h
--- dev/wscons/wsmouseinput.h   7 May 2018 21:58:42 -   1.11
+++ dev/wscons/wsmouseinput.h   6 Nov 2018 19:03:49 -
@@ -167,11 +167,9 @@ struct wsmouseinput {
 #define LOG_INPUT  (1 << 19)
 #define LOG_EVENTS (1 << 20)
 
-/* filter.mode (bit 0-2: smoothing factor, bit 3: hysteresis type) */
-#define 

Re: amd64/boot.8: document machine gop

2018-11-06 Thread Jason McIntyre
On Tue, Nov 06, 2018 at 08:49:33PM +0100, Klemens Nanni wrote:
> On Tue, Nov 06, 2018 at 08:28:17PM +0100, Klemens Nanni wrote:
> > Back in 2017 when sending the diff[0] to implement `machine gop' I did
> > not know better and forgot about manual bits - here they are.
> > 
> > I tried adding some details to make the difference between `video' and
> > `gop' a tad more clearer without mentioning too much implementation
> > details.
> Missing double colon after Mdocdate spotted by Larry Hynes.
> 

hi.

you shouldn;t touch Mdocdate - cvs updates this when you commit.

the diff reads ok. i wonder if there's a better way to distinguish
between EFI mode and EFI mode with efifb? i'm unclear on the terminology
(the whole efi thing actually) but i presumed you would be in efi mode
only when using efi framebuffer. so it seems (to me) misleading or
unclear just to talk about efi mode. perhaps you could just replace the
"On EFI systems" in the gop (what a name!) description with "On efifb(4)
systems", and save yourself a line of text?

finally the page is sorted alphabetically, so i think the gop text
should be sorted too.

jmc

> Index: arch/amd64/stand/boot/boot.8
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/boot/boot.8,v
> retrieving revision 1.27
> diff -u -p -r1.27 boot.8
> --- arch/amd64/stand/boot/boot.8  18 Feb 2018 07:45:39 -  1.27
> +++ arch/amd64/stand/boot/boot.8  6 Nov 2018 19:49:10 -
> @@ -25,7 +25,7 @@
>  .\" THE POSSIBILITY OF SUCH DAMAGE.
>  .\"
>  .\"
> -.Dd $Mdocdate: February 18 2018 $
> +.Dd $Mdocdate: November 5 2018 $
>  .Dt BOOT 8 amd64
>  .Os
>  .Sh NAME
> @@ -283,12 +283,22 @@ Prints contents of processor registers i
>  .Em DEBUG .
>  .It Nm video Op Ar mode
>  On EFI systems,
> -sets the video resolution to
> +sets the video resolution in columns and rows to
>  .Ar mode .
>  If
>  .Ar mode
>  is not given,
> -a list of available resolutions is shown.
> +a list of available modes is shown.
> +.It Nm gop Op Ar mode
> +On EFI systems,
> +sets the video resolution in pixels to
> +.Ar mode .
> +If
> +.Ar mode
> +is not given,
> +a list of available modes is shown.
> +This mode is used by
> +.Xr efifb 4 .
>  .El
>  .It reboot
>  Reboots the machine by initiating a warm boot procedure.
> 



Re: amd64/boot.8: document machine gop

2018-11-06 Thread Klemens Nanni
On Tue, Nov 06, 2018 at 08:28:17PM +0100, Klemens Nanni wrote:
> Back in 2017 when sending the diff[0] to implement `machine gop' I did
> not know better and forgot about manual bits - here they are.
> 
> I tried adding some details to make the difference between `video' and
> `gop' a tad more clearer without mentioning too much implementation
> details.
Missing double colon after Mdocdate spotted by Larry Hynes.

Index: arch/amd64/stand/boot/boot.8
===
RCS file: /cvs/src/sys/arch/amd64/stand/boot/boot.8,v
retrieving revision 1.27
diff -u -p -r1.27 boot.8
--- arch/amd64/stand/boot/boot.818 Feb 2018 07:45:39 -  1.27
+++ arch/amd64/stand/boot/boot.86 Nov 2018 19:49:10 -
@@ -25,7 +25,7 @@
 .\" THE POSSIBILITY OF SUCH DAMAGE.
 .\"
 .\"
-.Dd $Mdocdate: February 18 2018 $
+.Dd $Mdocdate: November 5 2018 $
 .Dt BOOT 8 amd64
 .Os
 .Sh NAME
@@ -283,12 +283,22 @@ Prints contents of processor registers i
 .Em DEBUG .
 .It Nm video Op Ar mode
 On EFI systems,
-sets the video resolution to
+sets the video resolution in columns and rows to
 .Ar mode .
 If
 .Ar mode
 is not given,
-a list of available resolutions is shown.
+a list of available modes is shown.
+.It Nm gop Op Ar mode
+On EFI systems,
+sets the video resolution in pixels to
+.Ar mode .
+If
+.Ar mode
+is not given,
+a list of available modes is shown.
+This mode is used by
+.Xr efifb 4 .
 .El
 .It reboot
 Reboots the machine by initiating a warm boot procedure.



amd64/boot.8: document machine gop

2018-11-06 Thread Klemens Nanni
Back in 2017 when sending the diff[0] to implement `machine gop' I did
not know better and forgot about manual bits - here they are.

I tried adding some details to make the difference between `video' and
`gop' a tad more clearer without mentioning too much implementation
details.

Feedback? OK?

0: https://marc.info/?l=openbsd-tech=150557217729175

Index: arch/amd64/stand/boot/boot.8
===
RCS file: /cvs/src/sys/arch/amd64/stand/boot/boot.8,v
retrieving revision 1.27
diff -u -p -r1.27 boot.8
--- arch/amd64/stand/boot/boot.818 Feb 2018 07:45:39 -  1.27
+++ arch/amd64/stand/boot/boot.86 Nov 2018 19:22:42 -
@@ -25,7 +25,7 @@
 .\" THE POSSIBILITY OF SUCH DAMAGE.
 .\"
 .\"
-.Dd $Mdocdate: February 18 2018 $
+.Dd $Mdocdate November 5 2018 $
 .Dt BOOT 8 amd64
 .Os
 .Sh NAME
@@ -283,12 +283,22 @@ Prints contents of processor registers i
 .Em DEBUG .
 .It Nm video Op Ar mode
 On EFI systems,
-sets the video resolution to
+sets the video resolution in columns and rows to
 .Ar mode .
 If
 .Ar mode
 is not given,
-a list of available resolutions is shown.
+a list of available modes is shown.
+.It Nm gop Op Ar mode
+On EFI systems,
+sets the video resolution in pixels to
+.Ar mode .
+If
+.Ar mode
+is not given,
+a list of available modes is shown.
+This mode is used by
+.Xr efifb 4 .
 .El
 .It reboot
 Reboots the machine by initiating a warm boot procedure.



Re: ktrace: struct flock

2018-11-06 Thread Todd C. Miller
On Mon, 05 Nov 2018 08:12:57 +0100, Anton Lindqvist wrote:

> Start tracing struct flock. I've been using this diff during lockf
> development.

OK millert@

 - todd



Re: OpenBSD on AMD Ryzen7 2700 Asrock B450 chipset

2018-11-06 Thread Chris Cappuccio
Denis [den...@mindall.org] wrote:
> 
> Hardware is relatively new. Can test any compatibility issues/fixes on it.
> 
> There are a lot of "unconfigured" hardware is present in dmesg:
> 
> OpenBSD 6.4 (RAMDISK_CD) #348: Thu Oct 11 13:36:16 MDT 2018
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/RAMDISK_CD

The RAMDISK kernels are stripped down. The GENERIC.MP output is more
interesting. Please send it to dm...@openbsd.org (not tech@)



Re: installboot: double free

2018-11-06 Thread Otto Moerbeek
On Tue, Nov 06, 2018 at 05:32:47PM +0800, Michael Mikonos wrote:

> On Tue, Nov 06, 2018 at 10:20:34AM +0100, Otto Moerbeek wrote:
> > On Tue, Nov 06, 2018 at 04:35:05PM +0800, Michael Mikonos wrote:
> > 
> > > Hello,
> > > 
> > > In installboot's fileprefix() function r is the return value
> > > of realpath(). If snprintf() fails free(r) happens twice---
> > > the second time is at label "err". From what I see the behavior
> > > was introduced in util.c revision 1.12.
> > > Does this fix look OK?
> > 
> > Yes, but I perfer to move the free call to just before the no-error return.
> > 
> 
> Sure, updated diff.

ok,

-Otto

> 
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/util.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 util.c
> --- util.c3 Jul 2018 20:14:41 -   1.12
> +++ util.c6 Nov 2018 09:29:04 -
> @@ -124,11 +124,11 @@ fileprefix(const char *base, const char 
>   goto err;
>   }
>   n = snprintf(s, PATH_MAX, "%s/%s", r, b);
> - free(r);
>   if (n < 1 || n >= PATH_MAX) {
>   warn("snprintf");
>   goto err;
>   }
> + free(r);
>   return (s);
>  
>  err:
> 



Re: installboot: double free

2018-11-06 Thread Michael Mikonos
On Tue, Nov 06, 2018 at 10:20:34AM +0100, Otto Moerbeek wrote:
> On Tue, Nov 06, 2018 at 04:35:05PM +0800, Michael Mikonos wrote:
> 
> > Hello,
> > 
> > In installboot's fileprefix() function r is the return value
> > of realpath(). If snprintf() fails free(r) happens twice---
> > the second time is at label "err". From what I see the behavior
> > was introduced in util.c revision 1.12.
> > Does this fix look OK?
> 
> Yes, but I perfer to move the free call to just before the no-error return.
> 

Sure, updated diff.

Index: util.c
===
RCS file: /cvs/src/usr.sbin/installboot/util.c,v
retrieving revision 1.12
diff -u -p -r1.12 util.c
--- util.c  3 Jul 2018 20:14:41 -   1.12
+++ util.c  6 Nov 2018 09:29:04 -
@@ -124,11 +124,11 @@ fileprefix(const char *base, const char 
goto err;
}
n = snprintf(s, PATH_MAX, "%s/%s", r, b);
-   free(r);
if (n < 1 || n >= PATH_MAX) {
warn("snprintf");
goto err;
}
+   free(r);
return (s);
 
 err:



Re: installboot: double free

2018-11-06 Thread Otto Moerbeek
On Tue, Nov 06, 2018 at 04:35:05PM +0800, Michael Mikonos wrote:

> Hello,
> 
> In installboot's fileprefix() function r is the return value
> of realpath(). If snprintf() fails free(r) happens twice---
> the second time is at label "err". From what I see the behavior
> was introduced in util.c revision 1.12.
> Does this fix look OK?

Yes, but I perfer to move the free call to just before the no-error return.

-Otto

> 
> - Michael
> 
> 
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/util.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 util.c
> --- util.c3 Jul 2018 20:14:41 -   1.12
> +++ util.c6 Nov 2018 08:26:45 -
> @@ -125,6 +125,7 @@ fileprefix(const char *base, const char 
>   }
>   n = snprintf(s, PATH_MAX, "%s/%s", r, b);
>   free(r);
> + r = NULL;
>   if (n < 1 || n >= PATH_MAX) {
>   warn("snprintf");
>   goto err;
> 



Re: bgpd: deny redefinition of default RIBs

2018-11-06 Thread Claudio Jeker
On Sun, Nov 04, 2018 at 06:51:39PM +0100, Denis Fondras wrote:
> Redefining a default RIB is not desirable.
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.352
> diff -u -p -r1.352 bgpd.h
> --- bgpd.h4 Nov 2018 14:34:00 -   1.352
> +++ bgpd.h4 Nov 2018 17:49:38 -
> @@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
>  #define F_RIB_NOEVALUATE 0x0002
>  #define F_RIB_NOFIB  0x0004
>  #define F_RIB_NOFIBSYNC  0x0008
> +#define F_RIB_DEFAULT0x0010
>  #define F_RIB_HASNOFIB   (F_RIB_NOFIB | F_RIB_NOEVALUATE)
>  
>  /* 4-byte magic AS number */
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.362
> diff -u -p -r1.362 parse.y
> --- parse.y   1 Nov 2018 00:18:44 -   1.362
> +++ parse.y   4 Nov 2018 17:49:38 -
> @@ -3319,10 +3319,10 @@ parse_config(char *filename, struct bgpd
>   netconf = >networks;
>  
>   add_rib("Adj-RIB-In", conf->default_tableid,
> - F_RIB_NOFIB | F_RIB_NOEVALUATE);
> + F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
>   add_rib("Adj-RIB-Out", conf->default_tableid,
> - F_RIB_NOFIB | F_RIB_NOEVALUATE);
> - add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL);
> + F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
> + add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL | F_RIB_DEFAULT);
>  
>   if ((file = pushfile(filename, 1)) == NULL) {
>   free(conf);
> @@ -3876,6 +3876,11 @@ add_rib(char *name, u_int rtableid, u_in
>   return (-1);
>   }
>   }
> + if (rr->flags & F_RIB_DEFAULT) {
> + yyerror("redefinition of %s not permitted", rr->name);
> + return (-1);
> + }
> +
>   if (strlcpy(rr->name, name, sizeof(rr->name)) >= sizeof(rr->name)) {
>   yyerror("rib name \"%s\" too long: max %zu",
>  name, sizeof(rr->name) - 1);
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.445
> diff -u -p -r1.445 rde.c
> --- rde.c 4 Nov 2018 12:34:54 -   1.445
> +++ rde.c 4 Nov 2018 17:49:38 -
> @@ -217,8 +217,10 @@ rde_main(int debug, int verbose)
>   peer_init(peerhashsize);
>  
>   /* make sure the default RIBs are setup */
> - rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> - rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> + rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> + F_RIB_DEFAULT);
> + rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> + F_RIB_DEFAULT);
>  
>   out_rules = calloc(1, sizeof(struct filter_head));
>   if (out_rules == NULL)
> 

Unsure if we need an extra flag, shouldn't it be enough to not allow
re-adding of a RIB in the conf? e.g. having 'rde rib foo' twice in the
config with different options could also be considered a
miss-configuration. Also it may be necessary to skip the default ribs in
printconf.c else bgpd -nv output will not work as input (like in the
regress tests).

-- 
:wq Claudio



installboot: double free

2018-11-06 Thread Michael Mikonos
Hello,

In installboot's fileprefix() function r is the return value
of realpath(). If snprintf() fails free(r) happens twice---
the second time is at label "err". From what I see the behavior
was introduced in util.c revision 1.12.
Does this fix look OK?

- Michael


Index: util.c
===
RCS file: /cvs/src/usr.sbin/installboot/util.c,v
retrieving revision 1.12
diff -u -p -r1.12 util.c
--- util.c  3 Jul 2018 20:14:41 -   1.12
+++ util.c  6 Nov 2018 08:26:45 -
@@ -125,6 +125,7 @@ fileprefix(const char *base, const char 
}
n = snprintf(s, PATH_MAX, "%s/%s", r, b);
free(r);
+   r = NULL;
if (n < 1 || n >= PATH_MAX) {
warn("snprintf");
goto err;



Re: ldap(1) add SAFE-INIT-CHAR

2018-11-06 Thread Martijn van Duren
On 11/6/18 8:59 AM, Claudio Jeker wrote:
> On Tue, Nov 06, 2018 at 08:21:57AM +0100, Martijn van Duren wrote:
>> ping
>>
>> On 10/24/18 10:27 AM, Martijn van Duren wrote:
>>> In my previous ldap mail I proclaimed that we should encode whitespace. 
>>> Reading rfc2849 a bit further, encoding a string with leading space is  
>>> mandatory by SAFE-INIT-CHAR. This is needed because of the definition
>>> of value-spec, which allows additional space, colon, and less-than
>>> after the colon separating the AttributeDescription.
>>>
>>> The code below adds these definitions. I also changed the outlen
>>> calculation because it at least fails b64_ntop on inlen == 1.
>>>
>>> OK?
> 
> See inline.
> 
>>> martijn@
>>>
>>> Index: ldapclient.c
>>> ===
>>> RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
>>> retrieving revision 1.5
>>> diff -u -p -r1.5 ldapclient.c
>>> --- ldapclient.c23 Oct 2018 08:28:34 -  1.5
>>> +++ ldapclient.c24 Oct 2018 08:21:27 -
>>> @@ -404,8 +404,13 @@ ldapc_printattr(struct ldapc *ldap, cons
>>>  * in SAFE-STRINGs. String value that do not match the
>>>  * criteria must be encoded as Base64.
>>>  */
>>> -   for (cp = (const unsigned char *)value;
>>> -   encode == 0 &&*cp != '\0'; cp++) {
>>> +   cp = (const unsigned char *)value;
>>> +   /* !SAFE-INIT-CHAR: SAFE-CHAR minus %x20 %x3A %x3C */
>>> +   if (*cp == ' ' ||
>>> +   *cp == ':' ||
>>> +   *cp == '<')
>>> +   encode = 1;
>>> +   for (; encode == 0 &&*cp != '\0'; cp++) {
>  ^ missing space
> 
>>> /* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
>>> if (*cp > 127 ||
>>> *cp == '\0' ||
> 
> The *cp == '\0' check here is strange since that is part of the for loop
> termination check.

See my other diff. The check for '\0' in the for-loop is incorrect,
since a attribute is transferred as an OCTETSTRING, but isn't required
to be an LDAPSTRING, AKA an UTF-8 string. (RFC4511 4.1.5)
> 
>>> @@ -421,7 +426,7 @@ ldapc_printattr(struct ldapc *ldap, cons
>>> }
>>> } else {
>>> inlen = strlen(value);
>>> -   outlen = inlen * 2 + 1;
>>> +   outlen = (((inlen + 2) / 3) * 4) + 1;
> 
> I'm surprised that there is no macro to calculate the length of a base64
> string. Anyway math is right.
> 
>>>  
>>> if ((out = calloc(1, outlen)) == NULL ||
>>> b64_ntop(value, inlen, out, outlen) == -1) {
>>>
>>
> 
> OK claudio
>