Re: patch: tighten make parser and fix bug in makefile

[Philip Guenther]

On Sun, Jun 18, 2017 at 3:17 AM, Marc Espie  wrote:
> While reading thru gnu/usr.bin/cc, I couldn't figure out where DEPENDFILE
> comes from.
>
> Turns out it comes from FreeBSD, which currently defines it in bsd.own.mk.
>
> The variable is empty on OpenBSD, thus yielding an always-false condition,
> which was definitely not the intent.
>
> This does put the right condition (probably not for long) and also fixes
> make's parser to reject this kind of problem directly.
>
> (currently running a src/xenocara make build/release to make sure this
> doesn't affect other directories).
>
> Once this is done, okay ?

ok guenther@

Re: Trapsleds

[Bryan Steele]

On Mon, Jun 19, 2017 at 09:22:57PM -0400, Todd Mortimer wrote:
> Hello tech,
> 
> I have attached a patch that converts NOP padding from the assembler
> into INT3 padding on amd64. The idea is to remove potentially conveinent
> NOP sleds from programs and libraries, which makes it harder for an
> attacker to hit any ROP gadgets or other instructions after a NOP sled. 
> 
> NOP sleds are used for text alignment in order to get jump targets onto
> 16 byte boundaries. They can appear both in the middle of a function
> and at the end. The trapsleds implemented in this diff convert NOP sleds
> longer than 2 bytes from a series of 0x6690 instructions to a 2 byte
> short JMP over a series of INT3 instructions that fill the rest of the
> gap. Programs that would have normally just slid through the NOP sled
> will now jump over. An attacker trying to hit the NOP sled will now get
> a core dump.
> 
> I have been running this on my system for over a week without any
> apparent ill effects. Specifically, there don't appear to be any
> performance penalties associated with doing this. A full base build
> on a system completely converted over to this took slightly less time to
> complete than the same build on a normal system, and my synthetic
> testing shows trapsleds perform similarly to nopsleds (performance
> difference was <1%, which is within error over multiple runs).
> 
> If people like this, I can do up the equivalent diff for clang.
> 
> Things that could could be improved:
> 
> 1. For padding inserted at the end of a function, the JMP is
> unnecessary, and could also be a 0x. I am going to have a go at gcc
> to see if I can coerce it into distinguishing end-of-function padding
> from padding that is intended to be executed. If some kind soul with gcc
> experience knows where I should look, any pointers would be welcome - my
> previous attempt was not fruitful.
> 
> 2. This patch also hits NOP sleds > 8 bytes on i386. We could also hit
> the NOP sleds between 3 and 7 bytes if there are no objections.
> 
> Comments and suggestions are welcome. Thanks to Theo for suggesting it
> in the hallway track at BSDCan. 
> 
> Todd
> 

> Index: gas/config/tc-i386.c
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/gas/config/tc-i386.c,v
> retrieving revision 1.7
> diff -u -p -u -p -r1.7 tc-i386.c
> --- gas/config/tc-i386.c  4 Jun 2017 20:26:18 -   1.7
> +++ gas/config/tc-i386.c  20 Jun 2017 00:36:27 -
> @@ -538,8 +538,8 @@ i386_align_code (fragP, count)
>  {0x8d,0xb4,0x26,0x00,0x00,0x00,0x00, /* leal 0L(%esi,1),%esi */
>   0x8d,0xbc,0x27,0x00,0x00,0x00,0x00};/* leal 0L(%edi,1),%edi */
>static const char f32_15[] =
> -{0xeb,0x0d,0x90,0x90,0x90,0x90,0x90, /* jmp .+15; lotsa nops */
> - 0x90,0x90,0x90,0x90,0x90,0x90,0x90,0x90};
> +{0xeb,0x0d,0xCC,0xCC,0xCC,0xCC,0xCC, /* jmp .+15; lotsa int3 */
> + 0xCC,0xCC,0xCC,0xCC,0xCC,0xCC,0xCC,0xCC};
>static const char f16_3[] =
>  {0x8d,0x74,0x00};/* lea 0(%esi),%esi 
> */
>static const char f16_4[] =
> @@ -556,6 +556,8 @@ i386_align_code (fragP, count)
>static const char f16_8[] =
>  {0x8d,0xb4,0x00,0x00,/* lea 0w(%si),%si  */
>   0x8d,0xbd,0x00,0x00};   /* lea 0w(%di),%di  */
> +  static const char f64_2[] =
> +{0x66,0x90};/* data16, nop*/
>static const char *const f32_patt[] = {
>  f32_1, f32_2, f32_3, f32_4, f32_5, f32_6, f32_7, f32_8,
>  f32_9, f32_10, f32_11, f32_12, f32_13, f32_14, f32_15
> @@ -564,32 +566,21 @@ i386_align_code (fragP, count)
>  f32_1, f32_2, f16_3, f16_4, f16_5, f16_6, f16_7, f16_8,
>  f32_15, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15
>};
> +  static const char *const f64_patt[] = {
> +f32_1, f64_2, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15,
> +f32_15, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15
> +  };
>  
>if (count <= 0 || count > 15)
>  return;
>  
> -  /* The recommended way to pad 64bit code is to use NOPs preceded by
> - maximally four 0x66 prefixes.  Balance the size of nops.  */
>if (flag_code == CODE_64BIT)
>  {
> -  int i;
> -  int nnops = (count + 3) / 4;
> -  int len = count / nnops;
> -  int remains = count - nnops * len;
> -  int pos = 0;
> -
> -  for (i = 0; i < remains; i++)
> - {
> -   memset (fragP->fr_literal + fragP->fr_fix + pos, 0x66, len);
> -   fragP->fr_literal[fragP->fr_fix + pos + len] = 0x90;
> -   pos += len + 1;
> - }
> -  for (; i < nnops; i++)
> - {
> -   memset (fragP->fr_literal + fragP->fr_fix + pos, 0x66, len - 1);
> -   fragP->fr_literal[fragP->fr_fix + pos + len - 1] = 0x90;
> -   pos += len;
> - }
> +  memcpy(fragP->fr_literal + fragP->fr_fix,
> +  f64_patt[count -1], count);
> +if (count > 2)
> +  /* Adjust jump 

Re: Trapsleds

[Mike Larkin]

On Mon, Jun 19, 2017 at 09:22:57PM -0400, Todd Mortimer wrote:
> Hello tech,
> 
> I have attached a patch that converts NOP padding from the assembler
> into INT3 padding on amd64. The idea is to remove potentially conveinent
> NOP sleds from programs and libraries, which makes it harder for an
> attacker to hit any ROP gadgets or other instructions after a NOP sled. 
> 
> NOP sleds are used for text alignment in order to get jump targets onto
> 16 byte boundaries. They can appear both in the middle of a function
> and at the end. The trapsleds implemented in this diff convert NOP sleds
> longer than 2 bytes from a series of 0x6690 instructions to a 2 byte
> short JMP over a series of INT3 instructions that fill the rest of the
> gap. Programs that would have normally just slid through the NOP sled
> will now jump over. An attacker trying to hit the NOP sled will now get
> a core dump.
> 
> I have been running this on my system for over a week without any
> apparent ill effects. Specifically, there don't appear to be any
> performance penalties associated with doing this. A full base build
> on a system completely converted over to this took slightly less time to
> complete than the same build on a normal system, and my synthetic
> testing shows trapsleds perform similarly to nopsleds (performance
> difference was <1%, which is within error over multiple runs).
> 
> If people like this, I can do up the equivalent diff for clang.
> 
> Things that could could be improved:
> 
> 1. For padding inserted at the end of a function, the JMP is
> unnecessary, and could also be a 0x. I am going to have a go at gcc
> to see if I can coerce it into distinguishing end-of-function padding
> from padding that is intended to be executed. If some kind soul with gcc
> experience knows where I should look, any pointers would be welcome - my
> previous attempt was not fruitful.
> 
> 2. This patch also hits NOP sleds > 8 bytes on i386. We could also hit
> the NOP sleds between 3 and 7 bytes if there are no objections.
> 
> Comments and suggestions are welcome. Thanks to Theo for suggesting it
> in the hallway track at BSDCan. 
> 
> Todd
> 

Nice, well done! I had this on my to do list for a while now and I'm happy
to see someone beat me to it.

-ml

> Index: gas/config/tc-i386.c
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/gas/config/tc-i386.c,v
> retrieving revision 1.7
> diff -u -p -u -p -r1.7 tc-i386.c
> --- gas/config/tc-i386.c  4 Jun 2017 20:26:18 -   1.7
> +++ gas/config/tc-i386.c  20 Jun 2017 00:36:27 -
> @@ -538,8 +538,8 @@ i386_align_code (fragP, count)
>  {0x8d,0xb4,0x26,0x00,0x00,0x00,0x00, /* leal 0L(%esi,1),%esi */
>   0x8d,0xbc,0x27,0x00,0x00,0x00,0x00};/* leal 0L(%edi,1),%edi */
>static const char f32_15[] =
> -{0xeb,0x0d,0x90,0x90,0x90,0x90,0x90, /* jmp .+15; lotsa nops */
> - 0x90,0x90,0x90,0x90,0x90,0x90,0x90,0x90};
> +{0xeb,0x0d,0xCC,0xCC,0xCC,0xCC,0xCC, /* jmp .+15; lotsa int3 */
> + 0xCC,0xCC,0xCC,0xCC,0xCC,0xCC,0xCC,0xCC};
>static const char f16_3[] =
>  {0x8d,0x74,0x00};/* lea 0(%esi),%esi 
> */
>static const char f16_4[] =
> @@ -556,6 +556,8 @@ i386_align_code (fragP, count)
>static const char f16_8[] =
>  {0x8d,0xb4,0x00,0x00,/* lea 0w(%si),%si  */
>   0x8d,0xbd,0x00,0x00};   /* lea 0w(%di),%di  */
> +  static const char f64_2[] =
> +{0x66,0x90};/* data16, nop*/
>static const char *const f32_patt[] = {
>  f32_1, f32_2, f32_3, f32_4, f32_5, f32_6, f32_7, f32_8,
>  f32_9, f32_10, f32_11, f32_12, f32_13, f32_14, f32_15
> @@ -564,32 +566,21 @@ i386_align_code (fragP, count)
>  f32_1, f32_2, f16_3, f16_4, f16_5, f16_6, f16_7, f16_8,
>  f32_15, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15
>};
> +  static const char *const f64_patt[] = {
> +f32_1, f64_2, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15,
> +f32_15, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15
> +  };
>  
>if (count <= 0 || count > 15)
>  return;
>  
> -  /* The recommended way to pad 64bit code is to use NOPs preceded by
> - maximally four 0x66 prefixes.  Balance the size of nops.  */
>if (flag_code == CODE_64BIT)
>  {
> -  int i;
> -  int nnops = (count + 3) / 4;
> -  int len = count / nnops;
> -  int remains = count - nnops * len;
> -  int pos = 0;
> -
> -  for (i = 0; i < remains; i++)
> - {
> -   memset (fragP->fr_literal + fragP->fr_fix + pos, 0x66, len);
> -   fragP->fr_literal[fragP->fr_fix + pos + len] = 0x90;
> -   pos += len + 1;
> - }
> -  for (; i < nnops; i++)
> - {
> -   memset (fragP->fr_literal + fragP->fr_fix + pos, 0x66, len - 1);
> -   fragP->fr_literal[fragP->fr_fix + pos + len - 1] = 0x90;
> -   pos += len;
> - }
> +  memcpy(fragP->fr_literal + 

Trapsleds

[Todd Mortimer]

Hello tech,

I have attached a patch that converts NOP padding from the assembler
into INT3 padding on amd64. The idea is to remove potentially conveinent
NOP sleds from programs and libraries, which makes it harder for an
attacker to hit any ROP gadgets or other instructions after a NOP sled. 

NOP sleds are used for text alignment in order to get jump targets onto
16 byte boundaries. They can appear both in the middle of a function
and at the end. The trapsleds implemented in this diff convert NOP sleds
longer than 2 bytes from a series of 0x6690 instructions to a 2 byte
short JMP over a series of INT3 instructions that fill the rest of the
gap. Programs that would have normally just slid through the NOP sled
will now jump over. An attacker trying to hit the NOP sled will now get
a core dump.

I have been running this on my system for over a week without any
apparent ill effects. Specifically, there don't appear to be any
performance penalties associated with doing this. A full base build
on a system completely converted over to this took slightly less time to
complete than the same build on a normal system, and my synthetic
testing shows trapsleds perform similarly to nopsleds (performance
difference was <1%, which is within error over multiple runs).

If people like this, I can do up the equivalent diff for clang.

Things that could could be improved:

1. For padding inserted at the end of a function, the JMP is
unnecessary, and could also be a 0x. I am going to have a go at gcc
to see if I can coerce it into distinguishing end-of-function padding
from padding that is intended to be executed. If some kind soul with gcc
experience knows where I should look, any pointers would be welcome - my
previous attempt was not fruitful.

2. This patch also hits NOP sleds > 8 bytes on i386. We could also hit
the NOP sleds between 3 and 7 bytes if there are no objections.

Comments and suggestions are welcome. Thanks to Theo for suggesting it
in the hallway track at BSDCan. 

Todd

Index: gas/config/tc-i386.c
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/gas/config/tc-i386.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 tc-i386.c
--- gas/config/tc-i386.c4 Jun 2017 20:26:18 -   1.7
+++ gas/config/tc-i386.c20 Jun 2017 00:36:27 -
@@ -538,8 +538,8 @@ i386_align_code (fragP, count)
 {0x8d,0xb4,0x26,0x00,0x00,0x00,0x00,   /* leal 0L(%esi,1),%esi */
  0x8d,0xbc,0x27,0x00,0x00,0x00,0x00};  /* leal 0L(%edi,1),%edi */
   static const char f32_15[] =
-{0xeb,0x0d,0x90,0x90,0x90,0x90,0x90,   /* jmp .+15; lotsa nops */
- 0x90,0x90,0x90,0x90,0x90,0x90,0x90,0x90};
+{0xeb,0x0d,0xCC,0xCC,0xCC,0xCC,0xCC,   /* jmp .+15; lotsa int3 */
+ 0xCC,0xCC,0xCC,0xCC,0xCC,0xCC,0xCC,0xCC};
   static const char f16_3[] =
 {0x8d,0x74,0x00};  /* lea 0(%esi),%esi */
   static const char f16_4[] =
@@ -556,6 +556,8 @@ i386_align_code (fragP, count)
   static const char f16_8[] =
 {0x8d,0xb4,0x00,0x00,  /* lea 0w(%si),%si  */
  0x8d,0xbd,0x00,0x00}; /* lea 0w(%di),%di  */
+  static const char f64_2[] =
+{0x66,0x90};/* data16, nop*/
   static const char *const f32_patt[] = {
 f32_1, f32_2, f32_3, f32_4, f32_5, f32_6, f32_7, f32_8,
 f32_9, f32_10, f32_11, f32_12, f32_13, f32_14, f32_15
@@ -564,32 +566,21 @@ i386_align_code (fragP, count)
 f32_1, f32_2, f16_3, f16_4, f16_5, f16_6, f16_7, f16_8,
 f32_15, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15
   };
+  static const char *const f64_patt[] = {
+f32_1, f64_2, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15,
+f32_15, f32_15, f32_15, f32_15, f32_15, f32_15, f32_15
+  };
 
   if (count <= 0 || count > 15)
 return;
 
-  /* The recommended way to pad 64bit code is to use NOPs preceded by
- maximally four 0x66 prefixes.  Balance the size of nops.  */
   if (flag_code == CODE_64BIT)
 {
-  int i;
-  int nnops = (count + 3) / 4;
-  int len = count / nnops;
-  int remains = count - nnops * len;
-  int pos = 0;
-
-  for (i = 0; i < remains; i++)
-   {
- memset (fragP->fr_literal + fragP->fr_fix + pos, 0x66, len);
- fragP->fr_literal[fragP->fr_fix + pos + len] = 0x90;
- pos += len + 1;
-   }
-  for (; i < nnops; i++)
-   {
- memset (fragP->fr_literal + fragP->fr_fix + pos, 0x66, len - 1);
- fragP->fr_literal[fragP->fr_fix + pos + len - 1] = 0x90;
- pos += len;
-   }
+  memcpy(fragP->fr_literal + fragP->fr_fix,
+  f64_patt[count -1], count);
+if (count > 2)
+  /* Adjust jump offset */
+  fragP->fr_literal[fragP->fr_fix + 1] = count - 2;
 }
   else
 if (flag_code == CODE_16BIT)

switch libkvm bits from hand rolled RBT updates to proper RBT api use

[David Gwynne]

now RBT is in userland, we can actually use the API instead of hand
rolling the copying of the tree topology.

ok?

Index: lib/libkvm/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
--- lib/libkvm/kvm_proc.c   7 Nov 2016 00:26:33 -   1.58
+++ lib/libkvm/kvm_proc.c   20 Jun 2017 01:03:05 -
@@ -77,6 +77,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -108,6 +109,9 @@ static int  proc_verify(kvm_t *, const st
 static voidps_str_a(struct ps_strings *, u_long *, int *);
 static voidps_str_e(struct ps_strings *, u_long *, int *);
 
+RBT_PROTOTYPE(uvm_map_addr, vm_map_entry, daddrs.addr_entry,
+uvm_mapentry_addrcmp);
+
 static struct vm_anon *
 _kvm_findanon(kvm_t *kd, struct vm_amap *amapp, int slot)
 {
@@ -166,7 +170,6 @@ _kvm_ureadm(kvm_t *kd, const struct kinf
struct vm_anon *anonp, anon;
struct vm_map_entry vme;
struct vm_page pg;
-   unsigned long rboff;
 
if (kd->swapspc == 0) {
kd->swapspc = _kvm_malloc(kd, kd->nbpg);
@@ -174,26 +177,23 @@ _kvm_ureadm(kvm_t *kd, const struct kinf
return (NULL);
}
 
-   rboff = (unsigned long)_entry - (unsigned long)
-
/*
 * Look through the address map for the memory object
 * that corresponds to the given virtual address.
 */
if (KREAD(kd, (u_long)p->p_vmspace, ))
return (NULL);
-   addr = (u_long)_map.addr.rbh_root.rbt_root;
+   addr = (u_long)RBT_ROOT(uvm_map_addr, _map.addr);
while (1) {
if (addr == 0)
return (NULL);
-   addr -= rboff;
if (KREAD(kd, addr, ))
return (NULL);
 
if (va < vme.start)
-   addr = (u_long)vme.daddrs.addr_entry.rbt_left;
+   addr = (u_long)RBT_LEFT(uvm_map_addr, );
else if (va >= vme.end + vme.guard + vme.fspace)
-   addr = (u_long)vme.daddrs.addr_entry.rbt_right;
+   addr = (u_long)RBT_RIGHT(uvm_map_addr, );
else if (va >= vme.end)
return (NULL);
else
@@ -543,3 +543,13 @@ kvm_ureadm(kvm_t *kd, const struct kinfo
}
return (ssize_t)(cp - buf);
 }
+
+static inline int
+uvm_mapentry_addrcmp(const struct vm_map_entry *e1,
+const struct vm_map_entry *e2)
+{
+   return e1->start < e2->start ? -1 : e1->start > e2->start;
+}
+
+RBT_GENERATE(uvm_map_addr, vm_map_entry, daddrs.addr_entry,
+uvm_mapentry_addrcmp);
Index: lib/libkvm/shlib_version
===
RCS file: /cvs/src/lib/libkvm/shlib_version,v
retrieving revision 1.19
diff -u -p -r1.19 shlib_version
--- lib/libkvm/shlib_version2 Oct 2016 23:11:55 -   1.19
+++ lib/libkvm/shlib_version20 Jun 2017 01:03:05 -
@@ -1,2 +1,2 @@
 major=16
-minor=2
+minor=3
Index: usr.sbin/procmap/procmap.c
===
RCS file: /cvs/src/usr.sbin/procmap/procmap.c,v
retrieving revision 1.63
diff -u -p -r1.63 procmap.c
--- usr.sbin/procmap/procmap.c  16 Sep 2016 04:45:35 -  1.63
+++ usr.sbin/procmap/procmap.c  20 Jun 2017 01:03:05 -
@@ -580,10 +580,10 @@ load_vm_map_entries(kvm_t *kd, struct vm
 
/* RBTs point at rb_entries inside nodes */
ld = load_vm_map_entries(kd, RBT_LEFT(uvm_map_addr, result), result);
-   result->daddrs.addr_entry.rbt_left = >daddrs.addr_entry;
+   RBT_SET_LEFT(uvm_map_addr, result, ld);
ld = load_vm_map_entries(kd, RBT_RIGHT(uvm_map_addr, result), result);
-   result->daddrs.addr_entry.rbt_right = >daddrs.addr_entry;
-   result->daddrs.addr_entry.rbt_parent = >daddrs.addr_entry;
+   RBT_SET_RIGHT(uvm_map_addr, result, ld);
+   RBT_SET_PARENT(uvm_map_addr, result, parent);
 
return result;
 }

optimise uvm_map_fill_vmmap with a binary search

[David Gwynne]

userland can ask for an array of a processes memory mappings via
sysctl, starting from a specified point. currently that starting
point is found by iterating over the mappings from start to finish,
but because the mappings are in an RBT we can do a binary search
instead.

ok?

Index: uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.231
diff -u -p -r1.231 uvm_map.c
--- uvm_map.c   17 May 2017 08:13:33 -  1.231
+++ uvm_map.c   20 Jun 2017 00:35:50 -
@@ -5211,7 +5211,7 @@ int
 uvm_map_fill_vmmap(struct vm_map *map, struct kinfo_vmentry *kve,
 size_t *lenp)
 {
-   struct vm_map_entry *entry;
+   struct vm_map_entry *entry = NULL, *tmp;
vaddr_t start;
int cnt, maxcnt, error = 0;
 
@@ -5229,13 +5229,21 @@ uvm_map_fill_vmmap(struct vm_map *map, s
start = (vaddr_t)kve[0].kve_start;
 
vm_map_lock(map);
-   RBT_FOREACH(entry, uvm_map_addr, >addr) {
-   if (cnt == maxcnt) {
-   error = ENOMEM;
+   /* look for the starting address by doing nfind (by hand) */
+   tmp = RBT_ROOT(uvm_map_addr, >addr);
+   while (tmp != NULL) {
+   if (start < tmp->start) {
+   entry = tmp;
+   tmp = RBT_LEFT(uvm_map_addr, tmp);
+   } else if (start > tmp->start) {
+   tmp = RBT_RIGHT(uvm_map_addr, tmp);
+   } else { /* exact match */
+   entry = tmp;
break;
}
-   if (start != 0 && entry->start < start)
-   continue;
+   }
+
+   while (entry != NULL) {
kve->kve_start = entry->start;
kve->kve_end = entry->end;
kve->kve_guard = entry->guard;
@@ -5249,8 +5257,14 @@ uvm_map_fill_vmmap(struct vm_map *map, s
kve->kve_advice = entry->advice;
kve->kve_inheritance = entry->inheritance;
kve->kve_flags = entry->flags;
+
+   if (++cnt == maxcnt) {
+   error = ENOMEM;
+   break;
+   }
+
kve++;
-   cnt++;
+   entry = RBT_NEXT(uvm_map_addr, entry);
}
vm_map_unlock(map);
 

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

[Alexander Bluhm]

On Mon, Jun 19, 2017 at 03:53:21PM +0200, Martin Pieuchot wrote:
> New version including a fix for kqueue filters.

OK bluhm@

> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.186
> diff -u -p -r1.186 uipc_socket.c
> --- kern/uipc_socket.c31 May 2017 08:55:10 -  1.186
> +++ kern/uipc_socket.c19 Jun 2017 13:43:27 -
> @@ -216,7 +216,7 @@ sofree(struct socket *so)
>   so->so_sp = NULL;
>   }
>  #endif /* SOCKET_SPLICE */
> - sbrelease(>so_snd);
> + sbrelease(so, >so_snd);
>   sorflush(so);
>   pool_put(_pool, so);
>  }
> @@ -440,7 +440,7 @@ restart:
>   } else if (addr == 0)
>   snderr(EDESTADDRREQ);
>   }
> - space = sbspace(>so_snd);
> + space = sbspace(so, >so_snd);
>   if (flags & MSG_OOB)
>   space += 1024;
>   if ((atomic && resid > so->so_snd.sb_hiwat) ||
> @@ -1041,7 +1041,7 @@ sorflush(struct socket *so)
>   struct sockbuf *sb = >so_rcv;
>   struct protosw *pr = so->so_proto;
>   sa_family_t af = pr->pr_domain->dom_family;
> - struct sockbuf asb;
> + struct socket aso;
>  
>   sb->sb_flags |= SB_NOINTR;
>   sblock(sb, M_WAITOK,
> @@ -1049,16 +1049,16 @@ sorflush(struct socket *so)
>: NULL);
>   socantrcvmore(so);
>   sbunlock(sb);
> - asb = *sb;
> + aso.so_rcv = *sb;
>   memset(sb, 0, sizeof (*sb));
>   /* XXX - the memset stomps all over so_rcv */
> - if (asb.sb_flags & SB_KNOTE) {
> - sb->sb_sel.si_note = asb.sb_sel.si_note;
> + if (aso.so_rcv.sb_flags & SB_KNOTE) {
> + sb->sb_sel.si_note = aso.so_rcv.sb_sel.si_note;
>   sb->sb_flags = SB_KNOTE;
>   }
>   if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose)
> - (*pr->pr_domain->dom_dispose)(asb.sb_mb);
> - sbrelease();
> + (*pr->pr_domain->dom_dispose)(aso.so_rcv.sb_mb);
> + sbrelease(, _rcv);
>  }
>  
>  #ifdef SOCKET_SPLICE
> @@ -1157,7 +1157,7 @@ sosplice(struct socket *so, int fd, off_
>   so->so_idletv = *tv;
>   else
>   timerclear(>so_idletv);
> - timeout_set(>so_idleto, soidle, so);
> + timeout_set_proc(>so_idleto, soidle, so);
>   task_set(>so_splicetask, sotask, so);
>  
>   /*
> @@ -1270,7 +1270,7 @@ somove(struct socket *so, int wait)
>   maxreached = 1;
>   }
>   }
> - space = sbspace(>so_snd);
> + space = sbspace(sosp, >so_snd);
>   if (so->so_oobmark && so->so_oobmark < len &&
>   so->so_oobmark < space + 1024)
>   space += 1024;
> @@ -1635,7 +1635,7 @@ sosetopt(struct socket *so, int level, i
>   goto bad;
>   }
>   if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
> - sbreserve(>so_snd, cnt)) {
> + sbreserve(so, >so_snd, cnt)) {
>   error = ENOBUFS;
>   goto bad;
>   }
> @@ -1648,7 +1648,7 @@ sosetopt(struct socket *so, int level, i
>   goto bad;
>   }
>   if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
> - sbreserve(>so_rcv, cnt)) {
> + sbreserve(so, >so_rcv, cnt)) {
>   error = ENOBUFS;
>   goto bad;
>   }
> @@ -1990,8 +1990,13 @@ int
>  filt_sowrite(struct knote *kn, long hint)
>  {
>   struct socket *so = kn->kn_fp->f_data;
> + int s;
>  
> - kn->kn_data = sbspace(>so_snd);
> + if (!(hint & NOTE_SUBMIT))
> + s = solock(so);
> + kn->kn_data = sbspace(so, >so_snd);
> + if (!(hint & NOTE_SUBMIT))
> + sounlock(s);
>   if (so->so_state & SS_CANTSENDMORE) {
>   kn->kn_flags |= EV_EOF;
>   kn->kn_fflags = so->so_error;
> Index: kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.117
> diff -u -p -r1.117 uipc_usrreq.c
> --- kern/uipc_usrreq.c13 Mar 2017 20:18:21 -  1.117
> +++ kern/uipc_usrreq.c19 Jun 2017 10:28:00 -
> @@ -222,7 +222,7 @@ uipc_usrreq(struct socket *so, int req, 
>   from = mtod(unp->unp_addr, struct sockaddr *);
>   else
>   from = _noname;
> - if (sbappendaddr(>so_rcv, from, m, control)) {
> + if (sbappendaddr(so2, >so_rcv, from, m, 

Re: Remove conditionals around crypto based free functions on relayd(8)

[Ricardo Mestre]

Hi,

After being prodded by benno@ please find below a new diff for relayd(8) due to
claudio@'s migration to libtls:

Comments? OK?

Index: relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.221
diff -u -p -u -r1.221 relay.c
--- relay.c 28 May 2017 10:39:15 -  1.221
+++ relay.c 19 Jun 2017 22:30:56 -
@@ -2130,10 +2130,8 @@ relay_tls_ctx_create(struct relay *rlay)
purge_key(>rl_tls_cert, rlay->rl_conf.tls_cert_len);
purge_key(>rl_tls_cacert, rlay->rl_conf.tls_cacert_len);
 
-   if (rlay->rl_tls_client_cfg == NULL)
-   tls_config_free(tls_client_cfg);
-   if (rlay->rl_tls_cfg == NULL)
-   tls_config_free(tls_cfg);
+   tls_config_free(tls_client_cfg);
+   tls_config_free(tls_cfg);
 
return (0);
  err:
@@ -2212,8 +2210,7 @@ relay_tls_transaction(struct rsession *c
errstr = "could not accept the TLS connection";
goto err;
}
-   if (cre->tlscert != NULL)
-   tls_free(tls_server);
+   tls_free(tls_server);
flag = EV_READ;
} else {
cre->tls = tls_client();
Index: relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.169
diff -u -p -u -r1.169 relayd.c
--- relayd.c31 May 2017 04:14:34 -  1.169
+++ relayd.c19 Jun 2017 22:30:56 -
@@ -576,18 +576,12 @@ purge_relay(struct relayd *env, struct r
purge_key(>rl_tls_ca, rlay->rl_conf.tls_ca_len);
purge_key(>rl_tls_cakey, rlay->rl_conf.tls_cakey_len);
 
-   if (rlay->rl_tls_pkey != NULL) {
-   EVP_PKEY_free(rlay->rl_tls_pkey);
-   rlay->rl_tls_pkey = NULL;
-   }
-   if (rlay->rl_tls_cacertx509 != NULL) {
-   X509_free(rlay->rl_tls_cacertx509);
-   rlay->rl_tls_cacertx509 = NULL;
-   }
-   if (rlay->rl_tls_capkey != NULL) {
-   EVP_PKEY_free(rlay->rl_tls_capkey);
-   rlay->rl_tls_capkey = NULL;
-   }
+   EVP_PKEY_free(rlay->rl_tls_pkey);
+   rlay->rl_tls_pkey = NULL;
+   X509_free(rlay->rl_tls_cacertx509);
+   rlay->rl_tls_cacertx509 = NULL;
+   EVP_PKEY_free(rlay->rl_tls_capkey);
+   rlay->rl_tls_capkey = NULL;
 
tls_free(rlay->rl_tls_ctx);
tls_config_free(rlay->rl_tls_cfg);
Index: ssl.c
===
RCS file: /cvs/src/usr.sbin/relayd/ssl.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 ssl.c
--- ssl.c   28 May 2017 10:39:15 -  1.33
+++ ssl.c   19 Jun 2017 22:30:56 -
@@ -109,10 +109,8 @@ ssl_load_key(struct relayd *env, const c
 
  fail:
free(buf);
-   if (bio != NULL)
-   BIO_free_all(bio);
-   if (key != NULL)
-   EVP_PKEY_free(key);
+   BIO_free_all(bio);
+   EVP_PKEY_free(key);
return (NULL);
 }
 
@@ -195,12 +193,9 @@ ssl_update_certificate(const uint8_t *ol
 
 done:
free(foo);
-   if (in)
-   BIO_free(in);
-   if (out)
-   BIO_free(out);
-   if (cert)
-   X509_free(cert);
+   BIO_free(in);
+   BIO_free(out);
+   X509_free(cert);
return (newcert);
 }
 
@@ -252,14 +247,10 @@ ssl_load_pkey(void *data, char *buf, off
return (1);
 
  fail:
-   if (rsa != NULL)
-   RSA_free(rsa);
-   if (in != NULL)
-   BIO_free(in);
-   if (pkey != NULL)
-   EVP_PKEY_free(pkey);
-   if (x509 != NULL)
-   X509_free(x509);
+   RSA_free(rsa);
+   BIO_free(in);
+   EVP_PKEY_free(pkey);
+   X509_free(x509);
free(exdata);
 
return (0);
@@ -318,10 +309,8 @@ ssl_ctx_fake_private_key(char *buf, off_
 fail:
BIO_free(in);
 
-   if (pkey != NULL)
-   EVP_PKEY_free(pkey);
-   if (x509 != NULL)
-   X509_free(x509);
+   EVP_PKEY_free(pkey);
+   X509_free(x509);
 
return (ret);
 }

Re: Route priority support for ospf6d

[Florian Riehm]

Thanks,

I commited it to move forward.

On 06/09/17 15:41, Claudio Jeker wrote:
> On Fri, Jun 09, 2017 at 03:28:07PM +0200, Alexander Bluhm wrote:
>> On Wed, May 31, 2017 at 02:29:03PM +0200, Florian Riehm wrote:
>>> @@ -359,6 +333,7 @@ kr_fib_decouple(void)
>>>  void
>>>  kr_dispatch_msg(int fd, short event, void *bula)
>>>  {
>>> +   /* XXX this is stupid */
>>> dispatch_rtmsg();
>>>  }
>>
>> I guess this comment refers to the event_loopexit(NULL) in ospfd code.
>> So I would not add it here.
>>
> 
> This this is from my side, because it is dumb to have a void function that
> only does on thing calling another void function.

I thought it referred to the exit on error approach.
I think it is the right approach (especially for a routing daemon)
but usually our customers tell me "ospf(6)d exited" and there is no 
chance for debugging. I am thinking about a solution for this problem.
My idea is do dump 'helpful' data in case of error, but 'helpful'
has to be precised.

>>> @@ -1377,10 +1376,10 @@ dispatch_rtmsg(void)
>>> if (rtm->rtm_flags & (RTF_LLINFO|RTF_BROADCAST))
>>> continue;
>>>  
>>> -#ifdef RTF_MPATH
>>> if (rtm->rtm_flags & RTF_MPATH)
>>> mpath = 1;
>>> -#endif
>>> +   prio = rtm->rtm_priority;
>>> +
>>
>> In ospfd we have at this location
>> flags = (prio == RTP_OSPF) ?
>> F_OSPFD_INSERTED : F_KERNEL;
>> Should we add that here?
> 
> Hmmm... this seems to be to detect from where routes are originating.

Yes, the next change will unify fetchtable and the kroute dispatch handler.
This will fix it.

Re: freetype-config.in

[David Coppa]

Il 19 giu 2017 22:32, "Stuart Henderson"  ha scritto:

On 2017/06/19 22:18, David Coppa wrote:
> Il 19 giu 2017 21:23, "Stuart Henderson"  ha scritto:
>
> On 2017/06/19 17:46, David Coppa wrote:
> >
> >
> > Il 19 giu 2017 16:19, "Stuart Henderson"  ha
> > scritto:
> >
> > I'm looking at updating freetype, mostly to unbreak PCF font names
> > which are currently different on different machines (e.g. I have
> > "xos4
> > Terminus" on a more recently installed laptop but just "Terminus"
> > on
> > other machines that have been upgraded from older versions). This
> > is a
> > problem for ports which hardcode the font name like dwm and dmenu
> > as
> > there isn't any one name which will work on all machines.
> >
> > It looks mostly straightforward to update (new functions -> minor
> > bump),
> > but it looks like there's something odd with builds/unix/
> > freetype-config.in,
> > our file isn't the same as upstream's last release, any suggestions
> > on what
> > to do with this?
> >
> >
> > I've already posted an update to freetype-2.8, asking for a bulk build,
> > and Matthieu already tested it... Have you missed it for some reasons?
> >
> >
> >
>
> Ah, found it now. I'll do an i386 bulk after my current one is done.
>
> There's a difference in include/freetype/config/ftoption.h compared to
> upstream's; they has PCF_CONFIG_OPTION_LONG_FAMILY_NAMES commented-out,
> your diff has it enabled. I'd really like to get this disabled and
> back to the normal names for PCF fonts, unless someone can fix it so
> that it's consistent between newly installed and upgraded machines.
>
>
> Here' a bit of rationale:
>
> The PCF change to show more "colourful" family names (introduced
> in version 2.7.1) was too radical; it can now be configured with
> PCF_CONFIG_OPTION_LONG_FAMILY_NAMES at compile time.
> If activated, it can be switched off at run time with the new pcf
> property 'no-long-family-names':
>
> export FREETYPE_PROPERTIES="pcf:no-long-family-names=1"
>
> Some users think the long family names are useful, some other not.
> I think, by enabling the option, we can leave users the freedom of choice.

Several ports (including dwm, dmenu) have "Terminus" hardcoded and won't
work correctly on the machines which use the new naming.

But only some machines use the new names, some use the old ones, and we
don't know why. It's not just me, others confirmed on my ports@ post
about it.

So if we change those ports to use the new naming like "xos4 Terminus"
then they break on other machines.

> Arch Linux also enabled the option.

And they had to make an faq telling people to add an XML file to create
an alias - and then the only way to get consistent behaviour is to use
the old name "Terminus" in configs and rely on it being picked up via
the alias on newer systems


Convincing enough. I'll keep it disabled.

Thanks,
David

Re: freetype-config.in

[Stuart Henderson]

On 2017/06/19 22:18, David Coppa wrote:
> Il 19 giu 2017 21:23, "Stuart Henderson"  ha scritto:
> 
> On 2017/06/19 17:46, David Coppa wrote:
> >
> >
> > Il 19 giu 2017 16:19, "Stuart Henderson"  ha
> > scritto:
> >
> > I'm looking at updating freetype, mostly to unbreak PCF font names
> > which are currently different on different machines (e.g. I have
> > "xos4
> > Terminus" on a more recently installed laptop but just "Terminus"
> > on
> > other machines that have been upgraded from older versions). This
> > is a
> > problem for ports which hardcode the font name like dwm and dmenu
> > as
> > there isn't any one name which will work on all machines.
> >
> > It looks mostly straightforward to update (new functions -> minor
> > bump),
> > but it looks like there's something odd with builds/unix/
> > freetype-config.in,
> > our file isn't the same as upstream's last release, any suggestions
> > on what
> > to do with this?
> >
> >
> > I've already posted an update to freetype-2.8, asking for a bulk build,
> > and Matthieu already tested it... Have you missed it for some reasons?
> >
> >
> >
> 
> Ah, found it now. I'll do an i386 bulk after my current one is done.
> 
> There's a difference in include/freetype/config/ftoption.h compared to
> upstream's; they has PCF_CONFIG_OPTION_LONG_FAMILY_NAMES commented-out,
> your diff has it enabled. I'd really like to get this disabled and
> back to the normal names for PCF fonts, unless someone can fix it so
> that it's consistent between newly installed and upgraded machines.
> 
> 
> Here' a bit of rationale:
> 
> The PCF change to show more "colourful" family names (introduced
> in version 2.7.1) was too radical; it can now be configured with
> PCF_CONFIG_OPTION_LONG_FAMILY_NAMES at compile time.
> If activated, it can be switched off at run time with the new pcf
> property 'no-long-family-names':
> 
> export FREETYPE_PROPERTIES="pcf:no-long-family-names=1"
> 
> Some users think the long family names are useful, some other not.
> I think, by enabling the option, we can leave users the freedom of choice.

Several ports (including dwm, dmenu) have "Terminus" hardcoded and won't
work correctly on the machines which use the new naming.

But only some machines use the new names, some use the old ones, and we
don't know why. It's not just me, others confirmed on my ports@ post
about it.

So if we change those ports to use the new naming like "xos4 Terminus"
then they break on other machines.

> Arch Linux also enabled the option.

And they had to make an faq telling people to add an XML file to create
an alias - and then the only way to get consistent behaviour is to use
the old name "Terminus" in configs and rely on it being picked up via
the alias on newer systems.

crypt_checkpass.3: clarify cetrain points

[Michal Mazurek]

I lost the old thread.

tedu@ said that we musn't recommend a buffer size of 61 (current
underlying code), but instead suggested using _PASSWORD_LEN or 128.

_PASSWORD_LEN is used in /usr/src and /usr/ports.

Index: lib/libc/crypt/crypt_checkpass.3
===
RCS file: /cvs/src/lib/libc/crypt/crypt_checkpass.3,v
retrieving revision 1.9
diff -u -p -r1.9 crypt_checkpass.3
--- lib/libc/crypt/crypt_checkpass.323 Jul 2015 22:20:02 -  1.9
+++ lib/libc/crypt/crypt_checkpass.319 Jun 2017 20:18:30 -
@@ -58,17 +58,32 @@ The provided
 .Fa password
 is randomly salted and hashed and stored in
 .Fa hash .
+.Fa hash
+must already be allocated, and
+.Fa hashsize
+must contain its size, which depends on the underlying algorithm.
+The constant 
+.Dv _PASSWORD_LEN
+is recommended for that.
 The
 .Fa pref
 argument identifies the preferred hashing algorithm and parameters.
+If set to
+.Dv NULL
+it defaults to 
+.Dq bcrypt,8 .
 Possible values are:
 .Bl -tag -width Ds
-.It Dq bcrypt,
+.It Dq bcrypt[,]
 The bcrypt algorithm, where the value of rounds can be between 4 and 31 and
 specifies the base 2 logarithm of the number of rounds.
 The special rounds value
 .Sq a
 automatically selects rounds based on system performance.
+This is the default if rounds is omitted.
+.Dq blowfish
+can be used as an alias for
+.Dq bcrypt .
 .El
 .Sh RETURN VALUES
 .Rv -std crypt_checkpass crypt_newhash
@@ -89,7 +104,9 @@ to
 .Er EINVAL
 if
 .Fa pref
-is unsupported.
+is unsupported, or the value of 
+.Fa hashsize
+is insufficient.
 .Sh SEE ALSO
 .Xr crypt 3 ,
 .Xr login.conf 5 ,

-- 
Michal Mazurek

Re: freetype-config.in

[David Coppa]

Il 19 giu 2017 21:23, "Stuart Henderson"  ha scritto:

On 2017/06/19 17:46, David Coppa wrote:
>
>
> Il 19 giu 2017 16:19, "Stuart Henderson"  ha
> scritto:
>
> I'm looking at updating freetype, mostly to unbreak PCF font names
> which are currently different on different machines (e.g. I have
> "xos4
> Terminus" on a more recently installed laptop but just "Terminus"
> on
> other machines that have been upgraded from older versions). This
> is a
> problem for ports which hardcode the font name like dwm and dmenu
> as
> there isn't any one name which will work on all machines.
>
> It looks mostly straightforward to update (new functions -> minor
> bump),
> but it looks like there's something odd with builds/unix/
> freetype-config.in,
> our file isn't the same as upstream's last release, any suggestions
> on what
> to do with this?
>
>
> I've already posted an update to freetype-2.8, asking for a bulk build,
> and Matthieu already tested it... Have you missed it for some reasons?
>
>
>

Ah, found it now. I'll do an i386 bulk after my current one is done.

There's a difference in include/freetype/config/ftoption.h compared to
upstream's; they has PCF_CONFIG_OPTION_LONG_FAMILY_NAMES commented-out,
your diff has it enabled. I'd really like to get this disabled and
back to the normal names for PCF fonts, unless someone can fix it so
that it's consistent between newly installed and upgraded machines.


Here' a bit of rationale:

The PCF change to show more "colourful" family names (introduced
in version 2.7.1) was too radical; it can now be configured with
PCF_CONFIG_OPTION_LONG_FAMILY_NAMES at compile time.
If activated, it can be switched off at run time with the new pcf
property 'no-long-family-names':

export FREETYPE_PROPERTIES="pcf:no-long-family-names=1"

Some users think the long family names are useful, some other not.
I think, by enabling the option, we can leave users the freedom of choice.
Arch Linux also enabled the option.

Ciao!
David

Re: freetype-config.in

[Stuart Henderson]

On 2017/06/19 17:46, David Coppa wrote:
> 
> 
> Il 19 giu 2017 16:19, "Stuart Henderson"  ha
> scritto:
> 
> I'm looking at updating freetype, mostly to unbreak PCF font names
> which are currently different on different machines (e.g. I have
> "xos4
> Terminus" on a more recently installed laptop but just "Terminus"
> on
> other machines that have been upgraded from older versions). This
> is a
> problem for ports which hardcode the font name like dwm and dmenu
> as
> there isn't any one name which will work on all machines.
> 
> It looks mostly straightforward to update (new functions -> minor
> bump),
> but it looks like there's something odd with builds/unix/
> freetype-config.in,
> our file isn't the same as upstream's last release, any suggestions
> on what
> to do with this?
> 
> 
> I've already posted an update to freetype-2.8, asking for a bulk build,
> and Matthieu already tested it... Have you missed it for some reasons?
> 
> 
> 

Ah, found it now. I'll do an i386 bulk after my current one is done.

There's a difference in include/freetype/config/ftoption.h compared to
upstream's; they has PCF_CONFIG_OPTION_LONG_FAMILY_NAMES commented-out,
your diff has it enabled. I'd really like to get this disabled and
back to the normal names for PCF fonts, unless someone can fix it so
that it's consistent between newly installed and upgraded machines.

Re: arp: Fix synopsis

[Theo de Raadt]

> On Mon, Jun 19, 2017 at 12:15:49PM +0200, Klemens Nanni wrote:
> > 'arp -[d]a' do not require hostname as told in the manual page.
> > 
> > A single fprintf() is both shorter and cleaner.
> > 
> 
> this diff suggest that running arp without arguments should work, but it
> does not.
> 
> there are a few approaches i guess. you could try and have SYNOPSIS show
> every usage trickery, or assume the reader can work out that
> this format of arp requires a hostname but, for example, the flag which
> displays all current arp entries does not.
> 
> i prefer the latter (that is, how it is now). if i thought it was not
> obvious how that worked, i'd say the fault lies in DESCRIPTION. but i
> think it's clear here.

absolutely right.

SYNOPSIS are not a grammar, it is not proscriptive.

The text is simply declarative, to help readers understand the scope
of operation; allowing them to determine valid use cases.

Everyone might as well get used to that.  I fear any solution to this
-- if taken the full extreme -- would lead to a number of manual pages
having 50 pages of SYNOPSIS at the start.  That would be very
unhelpful, so let's not go there.

Or, we could outlaw behaviours in commands which don't rigorously
abide to what SYNOPSIS describes.  Also an unachievable goal which
can create great harm once the fanatics get involved.

https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good

> > Index: arp.8
> > ===
> > RCS file: /cvs/src/usr.sbin/arp/arp.8,v
> > retrieving revision 1.39
> > diff -u -p -r1.39 arp.8
> > --- arp.8   5 Apr 2016 18:18:42 -   1.39
> > +++ arp.8   19 Jun 2017 10:15:10 -
> > @@ -40,7 +40,7 @@
> > .Nm arp
> > .Op Fl adn
> > .Op Fl V Ar rdomain
> > -.Ar hostname
> > +.Op Ar hostname
> > .Nm arp
> > .Op Fl F
> > .Op Fl f Ar file
> > Index: arp.c
> > ===
> > RCS file: /cvs/src/usr.sbin/arp/arp.c,v
> > retrieving revision 1.79
> > diff -u -p -r1.79 arp.c
> > --- arp.c   19 Apr 2017 05:36:12 -  1.79
> > +++ arp.c   19 Jun 2017 10:15:10 -
> > @@ -601,11 +601,10 @@ ether_str(struct sockaddr_dl *sdl)
> > void
> > usage(void)
> > {
> > -   fprintf(stderr, "usage: arp [-adn] [-V rdomain] hostname\n");
> > -   fprintf(stderr, "   arp [-F] [-f file] [-V rdomain] "
> > -   "-s hostname ether_addr\n"
> > -   "   [temp | permanent] [pub]\n");
> > -   fprintf(stderr, "   arp -W ether_addr [iface]\n");
> > +   fprintf(stderr, "usage: arp [-adn] [-V rdomain] [hostname]\n"
> > +   "   arp [-F] [-f file] [-V rdomain] -s hostname ether_addr\n"
> > +   "   [temp | permanent] [pub]\n"
> > +   "   arp -W ether_addr [iface]\n");
> > exit(1);
> > }
> > 
> > 
> 

Re: arp: Fix synopsis

[Jason McIntyre]

On Mon, Jun 19, 2017 at 12:15:49PM +0200, Klemens Nanni wrote:
> 'arp -[d]a' do not require hostname as told in the manual page.
> 
> A single fprintf() is both shorter and cleaner.
> 

this diff suggest that running arp without arguments should work, but it
does not.

there are a few approaches i guess. you could try and have SYNOPSIS show
every usage trickery, or assume the reader can work out that
this format of arp requires a hostname but, for example, the flag which
displays all current arp entries does not.

i prefer the latter (that is, how it is now). if i thought it was not
obvious how that worked, i'd say the fault lies in DESCRIPTION. but i
think it's clear here.

jmc

> Index: arp.8
> ===
> RCS file: /cvs/src/usr.sbin/arp/arp.8,v
> retrieving revision 1.39
> diff -u -p -r1.39 arp.8
> --- arp.8 5 Apr 2016 18:18:42 -   1.39
> +++ arp.8 19 Jun 2017 10:15:10 -
> @@ -40,7 +40,7 @@
> .Nm arp
> .Op Fl adn
> .Op Fl V Ar rdomain
> -.Ar hostname
> +.Op Ar hostname
> .Nm arp
> .Op Fl F
> .Op Fl f Ar file
> Index: arp.c
> ===
> RCS file: /cvs/src/usr.sbin/arp/arp.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 arp.c
> --- arp.c 19 Apr 2017 05:36:12 -  1.79
> +++ arp.c 19 Jun 2017 10:15:10 -
> @@ -601,11 +601,10 @@ ether_str(struct sockaddr_dl *sdl)
> void
> usage(void)
> {
> - fprintf(stderr, "usage: arp [-adn] [-V rdomain] hostname\n");
> - fprintf(stderr, "   arp [-F] [-f file] [-V rdomain] "
> - "-s hostname ether_addr\n"
> - "   [temp | permanent] [pub]\n");
> - fprintf(stderr, "   arp -W ether_addr [iface]\n");
> + fprintf(stderr, "usage: arp [-adn] [-V rdomain] [hostname]\n"
> + "   arp [-F] [-f file] [-V rdomain] -s hostname ether_addr\n"
> + "   [temp | permanent] [pub]\n"
> + "   arp -W ether_addr [iface]\n");
>   exit(1);
> }
> 
> 

Re: vi(1): documenting :s

[Jason McIntyre]

On Mon, Jun 19, 2017 at 03:57:35AM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> Jason McIntyre writes:
> > ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
> > so i'm not sure whether we should support this or not.
> 
> Hm, so it does. I think I would prefer to follow POSIX in this case.
> Here's a diff to allow "substitute" to work.
> 
> Annoyingly, there's an ambiguity in the POSIX synopsis (and ours).
> 
>  [2addr] s[ubstitute][/pattern/repl/[options][count][flags]]
> 
> This implies that "su" could expand to "substitute". But it expands (and
> should expand) to "suspend" instead. Since in our vi the first match
> always wins, the simplest way to implement this is to use separate ex
> commands:
> 
>  [2addr] s[/pattern/repl/[options][count][flags]]
>  [2addr] sub[stitute][/pattern/repl/[options][count][flags]]
> 
> There are already plenty of duplicate commands (e.g., "#"/"nu",
> "cd"/"chd", "co"/"t"...) so I think it's an acceptable approach.
> 

evening.

shouldn;t it be that we should show the suspend command as

sus[pend]

the shortest "s" matches "substitute", right. so we show it as

s[ubstitute]

i cannot find any text that describes what "su" *should* match though,
so i'm not sure. logically i'd expect it to match "substitute", since
that is first alphabetically. that's why i think it should be sus[pend],
not sub[stitute].

i don;t have any other versions of vi to compare how other systems do
this.

jmc

> Index: docs/USD.doc/vi.man/vi.1
> ===
> RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
> retrieving revision 1.67
> diff -u -p -r1.67 vi.1
> --- docs/USD.doc/vi.man/vi.1  15 Jun 2017 06:44:47 -  1.67
> +++ docs/USD.doc/vi.man/vi.1  19 Jun 2017 09:19:23 -
> @@ -356,7 +356,7 @@ matches the end of the word.
>  .It
>  .Sq ~
>  matches the replacement part of the last
> -.Cm substitute
> +.Cm s
>  command.
>  .El
>  .Sh BUFFERS
> @@ -1996,52 +1996,65 @@ Grow or shrink the current screen.
>  Rewind the argument list.
>  .Pp
>  .It Xo
> -.Cm se Ns Op Cm t
> +.Op Ar range
>  .Sm off
> -.Op option Oo = Oo value Oc Oc \ \&...
> +.Cm s
> +.Oo Cm / Ar pattern Cm / Ar replace Cm /
> +.Op Ar options
> +.Op Ar count
> +.Op Ar flags
> +.Oc
>  .Sm on
> -.Pf \ \& Op nooption ...
> -.Op option? ...
> -.Op Ar all
> -.Xc
> -Display or set editor options.
> -.Pp
> -.It Cm sh Ns Op Cm ell
> -Run a shell program.
> -.Pp
> -.It Xo
> -.Cm so Ns Op Cm urce
> -.Ar file
>  .Xc
> -Read and execute
> -.Nm ex
> -commands from a file.
> -.Pp
>  .It Xo
>  .Op Ar range
> -.Cm s Ns Op Cm ubstitute
>  .Sm off
> -.Op / Ar pattern No / Ar replace  No /
> -.Sm on
> -.Pf \ \& Op Ar options
> +.Cm sub Op Cm stitute
> +.Oo Cm / Ar pattern Cm / Ar replace Cm /
> +.Op Ar options
>  .Op Ar count
>  .Op Ar flags
> +.Sm on
>  .Xc
>  .It Xo
>  .Op Ar range
> +.Sm off
>  .Cm &
>  .Op Ar options
>  .Op Ar count
>  .Op Ar flags
> +.Sm on
>  .Xc
>  .It Xo
>  .Op Ar range
> +.Sm off
>  .Cm ~
>  .Op Ar options
>  .Op Ar count
>  .Op Ar flags
> +.Sm on
>  .Xc
> -Make substitutions.
> +Substitute the regular expression
> +.Ar pattern
> +with
> +.Ar replace .
> +When invoked as
> +.Cm & ,
> +or if
> +.Cm / Ns Ar pattern Ns Cm / Ns Ar replace Ns Cm /
> +is omitted,
> +.Ar pattern
> +and
> +.Ar replace
> +from the most recent
> +.Cm s
> +command are used.
> +.Cm ~
> +behaves like
> +.Cm & ,
> +except the pattern used is the most recent regular expression used by any
> +command.
> +.Pp
>  The
>  .Ar replace
>  field may contain any of the following sequences:
> @@ -2051,13 +2064,13 @@ The text matched by
>  .Ar pattern .
>  .It Sq \(a~
>  The replacement part of the previous
> -.Cm substitute
> +.Cm s
>  command.
>  .It Sq %
>  If this is the entire
>  .Ar replace
>  pattern, the replacement part of the previous
> -.Cm substitute
> +.Cm s
>  command.
>  .It Sq \e#
>  Where
> @@ -2082,6 +2095,40 @@ to be converted to uppercase.
>  Causes the next character to be converted to uppercase.
>  .El
>  .Pp
> +The
> +.Ar options
> +field may contain any of the following characters:
> +.Bl -tag -width Ds
> +.It Sq c
> +Prompt for confirmation before each replacement is done.
> +.It Sq g
> +Replace all instances of
> +.Ar pattern
> +in a line, not just the first.
> +.El
> +.Pp
> +.It Xo
> +.Cm se Ns Op Cm t
> +.Sm off
> +.Op option Oo = Oo value Oc Oc \ \&...
> +.Sm on
> +.Pf \ \& Op nooption ...
> +.Op option? ...
> +.Op Ar all
> +.Xc
> +Display or set editor options.
> +.Pp
> +.It Cm sh Ns Op Cm ell
> +Run a shell program.
> +.Pp
> +.It Xo
> +.Cm so Ns Op Cm urce
> +.Ar file
> +.Xc
> +Read and execute
> +.Nm ex
> +commands from a file.
> +.Pp
>  .It Xo
>  .Cm su Ns Op Cm spend Ns
>  .Op Cm !\&
> @@ -2291,7 +2338,9 @@ Remember the values of the
>  and
>  .Sq g
>  suffixes to the
> -.Cm substitute
> +.Cm s , &
> +and
> +.Cm ~
>  commands, instead of initializing them as unset for each new command.
>  .It Cm escapetime Bq 1
>  

Re: freetype-config.in

[David Coppa]

Il 19 giu 2017 16:19, "Stuart Henderson"  ha scritto:

I'm looking at updating freetype, mostly to unbreak PCF font names
which are currently different on different machines (e.g. I have "xos4
Terminus" on a more recently installed laptop but just "Terminus" on
other machines that have been upgraded from older versions). This is a
problem for ports which hardcode the font name like dwm and dmenu as
there isn't any one name which will work on all machines.

It looks mostly straightforward to update (new functions -> minor bump),
but it looks like there's something odd with builds/unix/freetype-config.in,
our file isn't the same as upstream's last release, any suggestions on what
to do with this?


I've already posted an update to freetype-2.8, asking for a bulk build, and
Matthieu already tested it... Have you missed it for some reasons?

Re: vi(1): documenting :s

[Anthony J. Bentley]

Theo Buehler writes:
> This looks like a reasonable approach and it appears to work. When I
> looked at this after jmc's question, I was scared off by the comment
> 
> >   * Adding new commands starting with 's' may break the substitute command 
> code
> >   * in ex_cmd() (the ex parser).  Read through the comments there, first.
> 
> which is also visible in your diff. I'm not entirely sure what this
> is talking about. Thus, only a hesitant ok for the C-part of your patch.

That comment is referring to the fact that, e.g., "sg" is a legal
command equivalent to ""; see ex/ex.c:430. Adding a command starting
with "sub" won't affect this, because there's no 'u' flag (plus there's
a command starting with "su" already).

-- 
Anthony J. Bentley

Re: vi(1): documenting :s

[Theo Buehler]

On Mon, Jun 19, 2017 at 03:57:35AM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> Jason McIntyre writes:
> > ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
> > so i'm not sure whether we should support this or not.
> 
> Hm, so it does. I think I would prefer to follow POSIX in this case.
> Here's a diff to allow "substitute" to work.
> 
> Annoyingly, there's an ambiguity in the POSIX synopsis (and ours).
> 
>  [2addr] s[ubstitute][/pattern/repl/[options][count][flags]]
> 
> This implies that "su" could expand to "substitute". But it expands (and
> should expand) to "suspend" instead. Since in our vi the first match
> always wins, the simplest way to implement this is to use separate ex
> commands:
> 
>  [2addr] s[/pattern/repl/[options][count][flags]]
>  [2addr] sub[stitute][/pattern/repl/[options][count][flags]]
> 
> There are already plenty of duplicate commands (e.g., "#"/"nu",
> "cd"/"chd", "co"/"t"...) so I think it's an acceptable approach.

This looks like a reasonable approach and it appears to work. When I
looked at this after jmc's question, I was scared off by the comment

>   * Adding new commands starting with 's' may break the substitute command 
> code
>   * in ex_cmd() (the ex parser).  Read through the comments there, first.

which is also visible in your diff. I'm not entirely sure what this
is talking about. Thus, only a hesitant ok for the C-part of your patch.

I'll leave it to jmc to verify the vi.1 part in full detail, but it does
look good to me.

Thanks!

freetype-config.in

[Stuart Henderson]

I'm looking at updating freetype, mostly to unbreak PCF font names
which are currently different on different machines (e.g. I have "xos4
Terminus" on a more recently installed laptop but just "Terminus" on
other machines that have been upgraded from older versions). This is a
problem for ports which hardcode the font name like dwm and dmenu as
there isn't any one name which will work on all machines.

It looks mostly straightforward to update (new functions -> minor bump),
but it looks like there's something odd with builds/unix/freetype-config.in,
our file isn't the same as upstream's last release, any suggestions on what
to do with this?

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

[Martin Pieuchot]

On 09/06/17(Fri) 15:54, Martin Pieuchot wrote:
> On 09/06/17(Fri) 00:32, Alexander Bluhm wrote:
> > On Tue, Jun 06, 2017 at 05:15:40PM +0200, Martin Pieuchot wrote:
> > > TCP/UDP are almost ready to run without KERNEL_LOCK() because accesses
> > > to their sockets are serialized via the NET_LOCK().  On the other hand
> > > pfkey and routing sockets accesses still rely on the KERNEL_LOCK().
> > > 
> > > Since we're going to work at the socket layer, first to remove the
> > > KERNEL_LOCK() from routing/pfkey sockets then to split the NET_LOCK(),
> > > we need some tooling to move faster and avoid mistakes.
> > > 
> > > Currently all operations on socket buffers are protected by these
> > > locks.  I'd like to assert that, at least for all functions used in
> > > TCP/UDP layers.
> > > 
> > > The idea is to later change the lock asserted in soassertlocked(). 
> > > 
> > > Comments, ok?
> > 
> > Good idea, mostly OK.
> 
> Updated diff:
> 
>  - use a clever trick in sorflush() to make the new assert happy.
>  - convert sbappendrecord() and sbappend() for coherency
>  - fix other nits bluhm@ spotted
>  - add a missing solock()/sounlock() in filt_sowrite().  Note that
>this doesn't make the whole function mp-safe.

New version including a fix for kqueue filters.

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.186
diff -u -p -r1.186 uipc_socket.c
--- kern/uipc_socket.c  31 May 2017 08:55:10 -  1.186
+++ kern/uipc_socket.c  19 Jun 2017 13:43:27 -
@@ -216,7 +216,7 @@ sofree(struct socket *so)
so->so_sp = NULL;
}
 #endif /* SOCKET_SPLICE */
-   sbrelease(>so_snd);
+   sbrelease(so, >so_snd);
sorflush(so);
pool_put(_pool, so);
 }
@@ -440,7 +440,7 @@ restart:
} else if (addr == 0)
snderr(EDESTADDRREQ);
}
-   space = sbspace(>so_snd);
+   space = sbspace(so, >so_snd);
if (flags & MSG_OOB)
space += 1024;
if ((atomic && resid > so->so_snd.sb_hiwat) ||
@@ -1041,7 +1041,7 @@ sorflush(struct socket *so)
struct sockbuf *sb = >so_rcv;
struct protosw *pr = so->so_proto;
sa_family_t af = pr->pr_domain->dom_family;
-   struct sockbuf asb;
+   struct socket aso;
 
sb->sb_flags |= SB_NOINTR;
sblock(sb, M_WAITOK,
@@ -1049,16 +1049,16 @@ sorflush(struct socket *so)
 : NULL);
socantrcvmore(so);
sbunlock(sb);
-   asb = *sb;
+   aso.so_rcv = *sb;
memset(sb, 0, sizeof (*sb));
/* XXX - the memset stomps all over so_rcv */
-   if (asb.sb_flags & SB_KNOTE) {
-   sb->sb_sel.si_note = asb.sb_sel.si_note;
+   if (aso.so_rcv.sb_flags & SB_KNOTE) {
+   sb->sb_sel.si_note = aso.so_rcv.sb_sel.si_note;
sb->sb_flags = SB_KNOTE;
}
if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose)
-   (*pr->pr_domain->dom_dispose)(asb.sb_mb);
-   sbrelease();
+   (*pr->pr_domain->dom_dispose)(aso.so_rcv.sb_mb);
+   sbrelease(, _rcv);
 }
 
 #ifdef SOCKET_SPLICE
@@ -1157,7 +1157,7 @@ sosplice(struct socket *so, int fd, off_
so->so_idletv = *tv;
else
timerclear(>so_idletv);
-   timeout_set(>so_idleto, soidle, so);
+   timeout_set_proc(>so_idleto, soidle, so);
task_set(>so_splicetask, sotask, so);
 
/*
@@ -1270,7 +1270,7 @@ somove(struct socket *so, int wait)
maxreached = 1;
}
}
-   space = sbspace(>so_snd);
+   space = sbspace(sosp, >so_snd);
if (so->so_oobmark && so->so_oobmark < len &&
so->so_oobmark < space + 1024)
space += 1024;
@@ -1635,7 +1635,7 @@ sosetopt(struct socket *so, int level, i
goto bad;
}
if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
-   sbreserve(>so_snd, cnt)) {
+   sbreserve(so, >so_snd, cnt)) {
error = ENOBUFS;
goto bad;
}
@@ -1648,7 +1648,7 @@ sosetopt(struct socket *so, int level, i
goto bad;
}
if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
-   sbreserve(>so_rcv, cnt)) {
+   sbreserve(so, >so_rcv, cnt)) {
error = ENOBUFS;
goto bad;
}
@@ -1990,8 +1990,13 @@ int
 filt_sowrite(struct knote *kn, long hint)
 {
struct 

Re: remove redundant flag from iwm(4)

[Mark Kettenis]

> Date: Mon, 19 Jun 2017 13:02:58 +0200
> From: Stefan Sperling 
> 
> On Mon, Jun 19, 2017 at 11:57:36AM +0200, Mark Kettenis wrote:
> > > @@ -7450,7 +7444,7 @@ iwm_init_task(void *arg1)
> > >   }
> > >   s = splnet();
> > >  
> > > - if (sc->sc_flags & IWM_FLAG_HW_INITED)
> > > + if (sc->sc_flags & IFF_RUNNING)
> > >   iwm_stop(ifp, 0);
> > 
> > This looks wrong to me.
> 
> Why?

Because IFF_RUNNING is a flag for ifp->if_flags, not sc->sc_flags.

Re: radeondrm: add a handler for the WSDISPLAYIO_GINFO ioctl

[Mark Kettenis]

> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
> d=statdns-com.20150623.gappssmtp.com; s=20150623;
> h=date:from:to:subject:message-id:mime-version:content-disposition
>  :user-agent;
> bh=V9zHsQIE9vOgAyjMqPGAM0Ewu2IvCE/CHFNKt6wyxNQ=;
> b=MkKZ9WE9lfjpYBYBGZQcYysJKvC4UlYyXKTKQXwLENXaHB2NtgnY1SZMu2Re5bscAU
>  o+Yz1pvNbZJEA7TC2Kf4+bYa6GzDnwizm7Za7pTT6TWRXmHEFneSKkYJBi2gB+31eHD4
>  D9G3GStIoRkevvNnlZQlWVGKJDx6O9GBByGR7lAz3h/iowKOFtdDirAf+qAD1z7JEp7Q
>  3GmoaUzEtgaWxGC6Mumysf2e9az+Er9aFCPmOHqeKdw86pkivAqfZ9FGTdsl6C6wJZuV
>  uhDVbKAlLpu60GLUx0N1+2euiQKWJzXuNGAMf14E6CSVUoNk1/QoH8g3hYWuHVCxF8Gi
>  PrpA==
> X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
> d=1e100.net; s=20161025;
> h=x-gm-message-state:date:from:to:subject:message-id:mime-version
>  :content-disposition:user-agent;
> bh=V9zHsQIE9vOgAyjMqPGAM0Ewu2IvCE/CHFNKt6wyxNQ=;
> b=Gi4R3L5FMsK4T3THolCk17KPii4oOfnIDjPqiaS8e9XsKEJL2x+uTya0qr4oGwvdmi
>  srWbZjD5ndbGZ58ZU2uXO0IkAt8FvsxAmmQ8FhZ+SxLb1mEEW1fSdcDLIk9Lt/X8Sae6
>  4gD6fh+jryBfZ5NkgPx1xRNqmc+MLONSebdJzkmuPlY8VblmGnrH7gZyU0iqrXfyJehI
>  S7seY8Y140GL8OEUCNn2xH/NAaNKSgvocKbflLySuviKHkC9nNHPCz94xZY3cYH5DGkV
>  PaBLuQqrW3ZTQTqPt1eFbHqAKTTKDNCpwl9At57Zpy4Sk5GblcXdCgCLUCHxCW4yl1+a
>  sbXQ==
> X-Gm-Message-State: AKS2vOzQ73kydUD5+ZRSch3GAerNubZ4817sxD32l1uCx9kCVICyci/5
>   f9ua8h3ArQcsaydiDtg=
> X-Received: by 10.46.20.73 with SMTP id 9mr5321274lju.51.1497802116251;
> Sun, 18 Jun 2017 09:08:36 -0700 (PDT)
> Date: Sun, 18 Jun 2017 18:08:33 +0200
> From: Frederic Cambus 
> Content-Disposition: inline
> List-Owner: 
> X-Loop: tech@openbsd.org
> Sender: owner-t...@openbsd.org
> X-CNFS-Analysis: v=2.2 cv=eoad9chX c=1 sm=0 tr=0
>   a=A3duGc4wJ8K8BtNzzvyz4A==:117 a=A3duGc4wJ8K8BtNzzvyz4A==:17
>   a=kj9zAlcOel0A:10 a=LWSFodeU3zMA:10 a=XlCHCKF-FruV5rCmWl0A:9
>   a=CjuIK1q_8ugA:10
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: 2.3 (**) DKIM_SIGNED, DKIM_VALID, FSL_HELO_FAKE,
>   RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS,
>   T_HEADER_FROM_DIFFERENT_DOMAINS
> X-XS4ALL-Spam: NO
> Envelope-To: mark.kette...@xs4all.nl
> 
> Hi tech@,
> 
> Add a handler for the WSDISPLAYIO_GINFO ioctl in radeondrm, allowing
> to retrieve basic information about a framebuffer display.
> 
> Same rationale as the diff I sent for inteldrm, and it can also be
> tested with wsconsctl which will return new display values for width,
> height and depth.
> 
> Comments? OK?

ok kettenis@

> Index: sys/dev/pci/drm/radeon/radeon_kms.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 radeon_kms.c
> --- sys/dev/pci/drm/radeon/radeon_kms.c   8 Jan 2017 12:11:54 -   
> 1.49
> +++ sys/dev/pci/drm/radeon/radeon_kms.c   18 Jun 2017 11:09:53 -
> @@ -354,9 +354,19 @@ struct wsdisplay_accessops radeondrm_acc
>  int
>  radeondrm_wsioctl(void *v, u_long cmd, caddr_t data, int flag, struct proc 
> *p)
>  {
> + struct rasops_info *ri = v;
> + struct wsdisplay_fbinfo *wdf;
> +
>   switch (cmd) {
>   case WSDISPLAYIO_GTYPE:
>   *(int *)data = WSDISPLAY_TYPE_RADEONDRM;
> + return 0;
> + case WSDISPLAYIO_GINFO:
> + wdf = (struct wsdisplay_fbinfo *)data;
> + wdf->width = ri->ri_width;
> + wdf->height = ri->ri_height;
> + wdf->depth = ri->ri_depth;
> + wdf->cmsize = 0;
>   return 0;
>   default:
>   return -1;
> 
> 

Re: inteldrm: add a handler for the WSDISPLAYIO_GINFO ioctl

[Mark Kettenis]

> Date: Sat, 17 Jun 2017 21:57:08 +0200
> From: Frederic Cambus 
> 
> Hi tech@,
> 
> Add a handler for the WSDISPLAYIO_GINFO ioctl in inteldrm, allowing
> to retrieve basic information about a framebuffer display.
> 
> When running wsconsctl on a machine with inteldrm support, this now
> returns those additional fields:
> 
> display.width=1600
> display.height=900
> display.depth=32
> 
> Comments? OK?

I think I can handle the conflicts with my inteldrm update, so ok
kettenis@

> Index: sys/dev/pci/drm/i915/i915_drv.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_drv.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 i915_drv.c
> --- sys/dev/pci/drm/i915/i915_drv.c   8 Jan 2017 12:11:54 -   1.101
> +++ sys/dev/pci/drm/i915/i915_drv.c   17 Jun 2017 19:15:04 -
> @@ -1037,11 +1037,20 @@ inteldrm_wsioctl(void *v, u_long cmd, ca
>  {
>   struct inteldrm_softc *dev_priv = v;
>   struct drm_device *dev = dev_priv->dev;
> + struct rasops_info *ri = _priv->ro;
> + struct wsdisplay_fbinfo *wdf;
>   struct wsdisplay_param *dp = (struct wsdisplay_param *)data;
>  
>   switch (cmd) {
>   case WSDISPLAYIO_GTYPE:
>   *(int *)data = WSDISPLAY_TYPE_INTELDRM;
> + return 0;
> + case WSDISPLAYIO_GINFO:
> + wdf = (struct wsdisplay_fbinfo *)data;
> + wdf->width = ri->ri_width;
> + wdf->height = ri->ri_height;
> + wdf->depth = ri->ri_depth;
> + wdf->cmsize = 0;
>   return 0;
>   case WSDISPLAYIO_GETPARAM:
>   if (ws_get_param && ws_get_param(dp) == 0)
> 
> 

Re: remove redundant flag from iwm(4)

[Stefan Sperling]

On Mon, Jun 19, 2017 at 11:57:36AM +0200, Mark Kettenis wrote:
> > @@ -7450,7 +7444,7 @@ iwm_init_task(void *arg1)
> > }
> > s = splnet();
> >  
> > -   if (sc->sc_flags & IWM_FLAG_HW_INITED)
> > +   if (sc->sc_flags & IFF_RUNNING)
> > iwm_stop(ifp, 0);
> 
> This looks wrong to me.

Why?

The idea is not to call iwm_stop() if we're resuming.

Re: net80211: initialize link state during attach

[Mark Kettenis]

> X-Envelope-From: s...@stsp.name
> X-Envelope-To: 
> Date: Fri, 16 Jun 2017 15:50:24 +0200
> From: Stefan Sperling 
> Mail-Followup-To: tech@openbsd.org
> Content-Disposition: inline
> List-Owner: 
> X-Loop: tech@openbsd.org
> Sender: owner-t...@openbsd.org
> X-CNFS-Analysis: v=2.2 cv=X8AQEybe c=1 sm=0 tr=0
>   a=A3duGc4wJ8K8BtNzzvyz4A==:117 a=A3duGc4wJ8K8BtNzzvyz4A==:17
>   a=kj9zAlcOel0A:10 a=MKtGQD3n3ToA:10 a=1oJP67jkp3AA:10
>   a=LWSFodeU3zMA:10 a=9vQTC7IF3B0A:10 a=ZZnuYtJkoWoA:10
>   a=FejnlcmcWjXD4WlFsZAA:9 a=CjuIK1q_8ugA:10
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: -0.5 () RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS,
>   T_HEADER_FROM_DIFFERENT_DOMAINS, UNPARSEABLE_RELAY
> X-XS4ALL-Spam: NO
> Envelope-To: mark.kette...@xs4all.nl
> 
> Set the link state of wifi interfaces to DOWN at attach time
> instead of leaving it as UNKNOWN which userland cannot really
> make use of (e.g. dhclient interprets UNKNOWN as UP).
> 
> Link state is also reset by the ieee80211_newstate() function.
> However, that function is usually called by the driver-specific newstate
> function stored in ic->ic_newstate. During the very first time ic_newstate()
> runs, without the diff below the link state is UNKNOWN. And if a driver
> decides not to call ieee80211_newstate() for some reason (such as iwm(4)
> does during INIT->SCAN) the link state will still be UNKNOWN.
> 
> In my opinion the link state of wifi interfaces should at any time
> be either DOWN or UP, and never UNKNOWN.
> 
> ok?

Makes sense to me.  Even if the firmware would initialize the hardware
I don't think we'd ever want to take over that state.

> Index: sys/net80211/ieee80211.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 ieee80211.c
> --- sys/net80211/ieee80211.c  31 May 2017 09:17:39 -  1.61
> +++ sys/net80211/ieee80211.c  7 Jun 2017 15:08:55 -
> @@ -156,6 +156,8 @@ ieee80211_ifattach(struct ifnet *ifp)
>  
>   if_addgroup(ifp, "wlan");
>   ifp->if_priority = IF_WIRELESS_DEFAULT_PRIORITY;
> +
> + ieee80211_set_link_state(ic, LINK_STATE_DOWN);
>  }
>  
>  void
> 
> 

arp: Fix synopsis

[Klemens Nanni]

'arp -[d]a' do not require hostname as told in the manual page.

A single fprintf() is both shorter and cleaner.

Index: arp.8
===
RCS file: /cvs/src/usr.sbin/arp/arp.8,v
retrieving revision 1.39
diff -u -p -r1.39 arp.8
--- arp.8   5 Apr 2016 18:18:42 -   1.39
+++ arp.8   19 Jun 2017 10:15:10 -
@@ -40,7 +40,7 @@
.Nm arp
.Op Fl adn
.Op Fl V Ar rdomain
-.Ar hostname
+.Op Ar hostname
.Nm arp
.Op Fl F
.Op Fl f Ar file
Index: arp.c
===
RCS file: /cvs/src/usr.sbin/arp/arp.c,v
retrieving revision 1.79
diff -u -p -r1.79 arp.c
--- arp.c   19 Apr 2017 05:36:12 -  1.79
+++ arp.c   19 Jun 2017 10:15:10 -
@@ -601,11 +601,10 @@ ether_str(struct sockaddr_dl *sdl)
void
usage(void)
{
-   fprintf(stderr, "usage: arp [-adn] [-V rdomain] hostname\n");
-   fprintf(stderr, "   arp [-F] [-f file] [-V rdomain] "
-   "-s hostname ether_addr\n"
-   "   [temp | permanent] [pub]\n");
-   fprintf(stderr, "   arp -W ether_addr [iface]\n");
+   fprintf(stderr, "usage: arp [-adn] [-V rdomain] [hostname]\n"
+   "   arp [-F] [-f file] [-V rdomain] -s hostname ether_addr\n"
+   "   [temp | permanent] [pub]\n"
+   "   arp -W ether_addr [iface]\n");
exit(1);
}

Re: vi(1): documenting :s

[Anthony J. Bentley]

Hi,

Jason McIntyre writes:
> ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
> so i'm not sure whether we should support this or not.

Hm, so it does. I think I would prefer to follow POSIX in this case.
Here's a diff to allow "substitute" to work.

Annoyingly, there's an ambiguity in the POSIX synopsis (and ours).

 [2addr] s[ubstitute][/pattern/repl/[options][count][flags]]

This implies that "su" could expand to "substitute". But it expands (and
should expand) to "suspend" instead. Since in our vi the first match
always wins, the simplest way to implement this is to use separate ex
commands:

 [2addr] s[/pattern/repl/[options][count][flags]]
 [2addr] sub[stitute][/pattern/repl/[options][count][flags]]

There are already plenty of duplicate commands (e.g., "#"/"nu",
"cd"/"chd", "co"/"t"...) so I think it's an acceptable approach.

Index: docs/USD.doc/vi.man/vi.1
===
RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
retrieving revision 1.67
diff -u -p -r1.67 vi.1
--- docs/USD.doc/vi.man/vi.115 Jun 2017 06:44:47 -  1.67
+++ docs/USD.doc/vi.man/vi.119 Jun 2017 09:19:23 -
@@ -356,7 +356,7 @@ matches the end of the word.
 .It
 .Sq ~
 matches the replacement part of the last
-.Cm substitute
+.Cm s
 command.
 .El
 .Sh BUFFERS
@@ -1996,52 +1996,65 @@ Grow or shrink the current screen.
 Rewind the argument list.
 .Pp
 .It Xo
-.Cm se Ns Op Cm t
+.Op Ar range
 .Sm off
-.Op option Oo = Oo value Oc Oc \ \&...
+.Cm s
+.Oo Cm / Ar pattern Cm / Ar replace Cm /
+.Op Ar options
+.Op Ar count
+.Op Ar flags
+.Oc
 .Sm on
-.Pf \ \& Op nooption ...
-.Op option? ...
-.Op Ar all
-.Xc
-Display or set editor options.
-.Pp
-.It Cm sh Ns Op Cm ell
-Run a shell program.
-.Pp
-.It Xo
-.Cm so Ns Op Cm urce
-.Ar file
 .Xc
-Read and execute
-.Nm ex
-commands from a file.
-.Pp
 .It Xo
 .Op Ar range
-.Cm s Ns Op Cm ubstitute
 .Sm off
-.Op / Ar pattern No / Ar replace  No /
-.Sm on
-.Pf \ \& Op Ar options
+.Cm sub Op Cm stitute
+.Oo Cm / Ar pattern Cm / Ar replace Cm /
+.Op Ar options
 .Op Ar count
 .Op Ar flags
+.Sm on
 .Xc
 .It Xo
 .Op Ar range
+.Sm off
 .Cm &
 .Op Ar options
 .Op Ar count
 .Op Ar flags
+.Sm on
 .Xc
 .It Xo
 .Op Ar range
+.Sm off
 .Cm ~
 .Op Ar options
 .Op Ar count
 .Op Ar flags
+.Sm on
 .Xc
-Make substitutions.
+Substitute the regular expression
+.Ar pattern
+with
+.Ar replace .
+When invoked as
+.Cm & ,
+or if
+.Cm / Ns Ar pattern Ns Cm / Ns Ar replace Ns Cm /
+is omitted,
+.Ar pattern
+and
+.Ar replace
+from the most recent
+.Cm s
+command are used.
+.Cm ~
+behaves like
+.Cm & ,
+except the pattern used is the most recent regular expression used by any
+command.
+.Pp
 The
 .Ar replace
 field may contain any of the following sequences:
@@ -2051,13 +2064,13 @@ The text matched by
 .Ar pattern .
 .It Sq \(a~
 The replacement part of the previous
-.Cm substitute
+.Cm s
 command.
 .It Sq %
 If this is the entire
 .Ar replace
 pattern, the replacement part of the previous
-.Cm substitute
+.Cm s
 command.
 .It Sq \e#
 Where
@@ -2082,6 +2095,40 @@ to be converted to uppercase.
 Causes the next character to be converted to uppercase.
 .El
 .Pp
+The
+.Ar options
+field may contain any of the following characters:
+.Bl -tag -width Ds
+.It Sq c
+Prompt for confirmation before each replacement is done.
+.It Sq g
+Replace all instances of
+.Ar pattern
+in a line, not just the first.
+.El
+.Pp
+.It Xo
+.Cm se Ns Op Cm t
+.Sm off
+.Op option Oo = Oo value Oc Oc \ \&...
+.Sm on
+.Pf \ \& Op nooption ...
+.Op option? ...
+.Op Ar all
+.Xc
+Display or set editor options.
+.Pp
+.It Cm sh Ns Op Cm ell
+Run a shell program.
+.Pp
+.It Xo
+.Cm so Ns Op Cm urce
+.Ar file
+.Xc
+Read and execute
+.Nm ex
+commands from a file.
+.Pp
 .It Xo
 .Cm su Ns Op Cm spend Ns
 .Op Cm !\&
@@ -2291,7 +2338,9 @@ Remember the values of the
 and
 .Sq g
 suffixes to the
-.Cm substitute
+.Cm s , &
+and
+.Cm ~
 commands, instead of initializing them as unset for each new command.
 .It Cm escapetime Bq 1
 The tenths of a second
Index: ex/ex_cmd.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_cmd.c,v
retrieving revision 1.10
diff -u -p -r1.10 ex_cmd.c
--- ex/ex_cmd.c 19 Nov 2015 07:53:31 -  1.10
+++ ex/ex_cmd.c 19 Jun 2017 09:19:23 -
@@ -288,8 +288,8 @@ EXCMDLIST const cmds[] = {
  * Adding new commands starting with 's' may break the substitute command code
  * in ex_cmd() (the ex parser).  Read through the comments there, first.
  */
-/* C_SUBSTITUTE */
-   {"s",   ex_s,   E_ADDR2,
+/* C_S */
+   {"s",   ex_s,   E_ADDR2,
"s",
"[line [,line]] s [[/;]RE[/;]repl[/;] [cgr] [count] [#lp]]",
"substitute on lines matching an RE"},
@@ -323,6 +323,11 @@ EXCMDLIST const cmds[] = {
"!",
"su[spend][!]",
"suspend the edit session"},
+/* C_SUBSTITUTE */
+   

Re: remove redundant flag from iwm(4)

[Mark Kettenis]

> Date: Fri, 16 Jun 2017 15:39:30 +0200
> From: Stefan Sperling 
> 
> The HW_INITED flag has a grammatically dubious name and is unnecessary
> because it duplicates the purpose of the IFF_RUNNING flag.
> 
> ok?

I don't think so...

> Index: sys/dev/pci/if_iwm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.197
> diff -u -p -r1.197 if_iwm.c
> --- sys/dev/pci/if_iwm.c  16 Jun 2017 08:45:34 -  1.197
> +++ sys/dev/pci/if_iwm.c  16 Jun 2017 11:47:09 -
> @@ -6106,9 +6106,6 @@ iwm_init(struct ifnet *ifp)
>   struct ieee80211com *ic = >sc_ic;
>   int err, generation;
>  
> - if (sc->sc_flags & IWM_FLAG_HW_INITED) {
> - return 0;
> - }
>   sc->sc_generation++;
>  
>   err = iwm_init_hw(sc);
> @@ -6135,8 +6132,6 @@ iwm_init(struct ifnet *ifp)
>   return err;
>   } while (ic->ic_state != IEEE80211_S_SCAN);
>  
> - sc->sc_flags |= IWM_FLAG_HW_INITED;
> -
>   return 0;
>  }
>  
> @@ -6214,7 +6209,6 @@ iwm_stop(struct ifnet *ifp, int disable)
>   struct ieee80211com *ic = >sc_ic;
>   struct iwm_node *in = (void *)ic->ic_bss;
>  
> - sc->sc_flags &= ~IWM_FLAG_HW_INITED;
>   sc->sc_generation++;
>   ic->ic_scan_lock = IEEE80211_SCAN_UNLOCKED;
>   ifp->if_flags &= ~IFF_RUNNING;
> @@ -7450,7 +7444,7 @@ iwm_init_task(void *arg1)
>   }
>   s = splnet();
>  
> - if (sc->sc_flags & IWM_FLAG_HW_INITED)
> + if (sc->sc_flags & IFF_RUNNING)
>   iwm_stop(ifp, 0);

This looks wrong to me.

>   if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP)
>   iwm_init(ifp);
> Index: sys/dev/pci/if_iwmvar.h
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwmvar.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 if_iwmvar.h
> --- sys/dev/pci/if_iwmvar.h   14 Jun 2017 16:56:04 -  1.28
> +++ sys/dev/pci/if_iwmvar.h   14 Jun 2017 19:17:23 -
> @@ -280,9 +280,8 @@ struct iwm_rx_ring {
>  };
>  
>  #define IWM_FLAG_USE_ICT 0x01
> -#define IWM_FLAG_HW_INITED   0x02
> -#define IWM_FLAG_RFKILL  0x04
> -#define IWM_FLAG_SCANNING0x08
> +#define IWM_FLAG_RFKILL  0x02
> +#define IWM_FLAG_SCANNING0x04
>  
>  struct iwm_ucode_status {
>   uint32_t uc_error_event_table;
> 
> 
> 
> 

Re: smtpd session hang

[Gilles Chehade]

On Fri, Jun 16, 2017 at 07:12:43PM +0300, Henri Kemppainen wrote:
> > > Nice catch, the diff reads fine to me, I'll commit later today when I
> > > have another ok from eric@
> 
> > Yes, this looks correct. But, I would rather move the resume test before
> > the EOM test, to avoid touching the session after the transfer has been
> > finalized by smtp_data_io_done().
> 
> It oughtn't make a difference as long as the duplex control is correct,
> but I'm fine with that change.
> 
> > > side note, smtpd should not have been able to leak more than 5 fd, it
> > > should not have been able to exhaust, is this what you observed ?
> 
> For the record, we discussed this with Gilles on irc and while we saw
> more than a dozen leaked fds, it's okay as smtpd will allow as many
> incoming sessions as the dtable can accommodate (with an fd reserve).
> The lower limits are on outgoing connections.
> 
> New diff with reordered code.  I'll see if I can get Adam to run one more
> round of testing..
> 

Committed thanks :)
-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg