Re: pf_pull_hdr af switch

[Claudio Jeker]

On Tue, May 23, 2017 at 03:00:57AM +0200, Alexander Bluhm wrote:
> Hi,
> 
> I would like to move the common length check in pf_pull_hdr() after
> the address family switch.  This makes the specific calculation
> more obvious.
> 
> ok?

OK
 
> bluhm
> 
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1026
> diff -u -p -r1.1026 pf.c
> --- net/pf.c  20 May 2017 22:56:43 -  1.1026
> +++ net/pf.c  23 May 2017 00:48:35 -
> @@ -5611,6 +5611,8 @@ void *
>  pf_pull_hdr(struct mbuf *m, int off, void *p, int len,
>  u_short *actionp, u_short *reasonp, sa_family_t af)
>  {
> + int iplen = 0;
> +
>   switch (af) {
>   case AF_INET: {
>   struct ip   *h = mtod(m, struct ip *);
> @@ -5625,28 +5627,22 @@ pf_pull_hdr(struct mbuf *m, int off, voi
>   }
>   return (NULL);
>   }
> - if (m->m_pkthdr.len < off + len ||
> - ntohs(h->ip_len) < off + len) {
> - ACTION_SET(actionp, PF_DROP);
> - REASON_SET(reasonp, PFRES_SHORT);
> - return (NULL);
> - }
> + iplen = ntohs(h->ip_len);
>   break;
>   }
>  #ifdef INET6
>   case AF_INET6: {
>   struct ip6_hdr  *h = mtod(m, struct ip6_hdr *);
>  
> - if (m->m_pkthdr.len < off + len ||
> - (ntohs(h->ip6_plen) + sizeof(struct ip6_hdr)) <
> - (unsigned)(off + len)) {
> - ACTION_SET(actionp, PF_DROP);
> - REASON_SET(reasonp, PFRES_SHORT);
> - return (NULL);
> - }
> + iplen = ntohs(h->ip6_plen) + sizeof(struct ip6_hdr);
>   break;
>   }
>  #endif /* INET6 */
> + }
> + if (m->m_pkthdr.len < off + len || iplen < off + len) {
> + ACTION_SET(actionp, PF_DROP);
> + REASON_SET(reasonp, PFRES_SHORT);
> + return (NULL);
>   }
>   m_copydata(m, off, len, p);
>   return (p);
> 

-- 
:wq Claudio

pf_pull_hdr af switch

[Alexander Bluhm]

Hi,

I would like to move the common length check in pf_pull_hdr() after
the address family switch.  This makes the specific calculation
more obvious.

ok?

bluhm

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1026
diff -u -p -r1.1026 pf.c
--- net/pf.c20 May 2017 22:56:43 -  1.1026
+++ net/pf.c23 May 2017 00:48:35 -
@@ -5611,6 +5611,8 @@ void *
 pf_pull_hdr(struct mbuf *m, int off, void *p, int len,
 u_short *actionp, u_short *reasonp, sa_family_t af)
 {
+   int iplen = 0;
+
switch (af) {
case AF_INET: {
struct ip   *h = mtod(m, struct ip *);
@@ -5625,28 +5627,22 @@ pf_pull_hdr(struct mbuf *m, int off, voi
}
return (NULL);
}
-   if (m->m_pkthdr.len < off + len ||
-   ntohs(h->ip_len) < off + len) {
-   ACTION_SET(actionp, PF_DROP);
-   REASON_SET(reasonp, PFRES_SHORT);
-   return (NULL);
-   }
+   iplen = ntohs(h->ip_len);
break;
}
 #ifdef INET6
case AF_INET6: {
struct ip6_hdr  *h = mtod(m, struct ip6_hdr *);
 
-   if (m->m_pkthdr.len < off + len ||
-   (ntohs(h->ip6_plen) + sizeof(struct ip6_hdr)) <
-   (unsigned)(off + len)) {
-   ACTION_SET(actionp, PF_DROP);
-   REASON_SET(reasonp, PFRES_SHORT);
-   return (NULL);
-   }
+   iplen = ntohs(h->ip6_plen) + sizeof(struct ip6_hdr);
break;
}
 #endif /* INET6 */
+   }
+   if (m->m_pkthdr.len < off + len || iplen < off + len) {
+   ACTION_SET(actionp, PF_DROP);
+   REASON_SET(reasonp, PFRES_SHORT);
+   return (NULL);
}
m_copydata(m, off, len, p);
return (p);

Fix yacc(1) warnings

[Brian Callahan]

Hi tech --

I attemped to build a DEBUG version of yacc. gcc gave warnings and
clang errored out, so I turned on WARNINGS=yes and fixed all the
warnings.

OpenBSD seems to be upstream for this version of byacc, as the
other BSDs follow Thomas Dickey's byacc.

~Brian

Index: closure.c
===
RCS file: /cvs/src/usr.bin/yacc/closure.c,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 closure.c
--- closure.c   2 Dec 2014 15:56:22 -   1.14
+++ closure.c   15 May 2017 16:56:56 -
@@ -43,7 +43,76 @@ static unsigned *first_derives;
 static unsigned *EFF;
 
 
-void
+#ifdef DEBUG
+
+static void
+print_closure(int n)
+{
+   short *isp;
+
+   printf("\n\nn = %d\n\n", n);
+   for (isp = itemset; isp < itemsetend; isp++)
+   printf("   %d\n", *isp);
+}
+
+static void
+print_EFF(void)
+{
+   int i, j;
+   unsigned int *rowp;
+   unsigned int k, word;
+
+   printf("\n\nEpsilon Free Firsts\n");
+
+   for (i = start_symbol; i < nsyms; i++) {
+   printf("\n%s", symbol_name[i]);
+   rowp = EFF + ((i - start_symbol) * WORDSIZE(nvars));
+   word = *rowp++;
+
+   k = BITS_PER_WORD;
+   for (j = 0; j < nvars; k++, j++) {
+   if (k >= BITS_PER_WORD) {
+   word = *rowp++;
+   k = 0;
+   }
+
+   if (word & (1 << k))
+   printf("  %s", symbol_name[start_symbol + j]);
+   }
+   }
+}
+
+static void
+print_first_derives(void)
+{
+   int i, j;
+   unsigned int *rp;
+   unsigned int k, cword = 0;
+
+   printf("\n\n\nFirst Derives\n");
+
+   for (i = start_symbol; i < nsyms; i++) {
+   printf("\n%s derives\n", symbol_name[i]);
+   rp = first_derives + i * WORDSIZE(nrules);
+   k = BITS_PER_WORD;
+   for (j = 0; j <= nrules; k++, j++) {
+   if (k >= BITS_PER_WORD) {
+   cword = *rp++;
+   k = 0;
+   }
+
+   if (cword & (1 << k))
+   printf("   %d\n", j);
+   }
+   }
+
+   fflush(stdout);
+}
+
+#endif
+
+
+static void
 set_EFF(void)
 {
unsigned int *row;
@@ -177,72 +246,3 @@ finalize_closure(void)
free(ruleset);
free(first_derives + ntokens * WORDSIZE(nrules));
 }
-
-
-#ifdef DEBUG
-
-void
-print_closure(int n)
-{
-   short *isp;
-
-   printf("\n\nn = %d\n\n", n);
-   for (isp = itemset; isp < itemsetend; isp++)
-   printf("   %d\n", *isp);
-}
-
-void
-print_EFF(void)
-{
-   int i, j;
-   unsigned int *rowp;
-   unsigned int k, word;
-
-   printf("\n\nEpsilon Free Firsts\n");
-
-   for (i = start_symbol; i < nsyms; i++) {
-   printf("\n%s", symbol_name[i]);
-   rowp = EFF + ((i - start_symbol) * WORDSIZE(nvars));
-   word = *rowp++;
-
-   k = BITS_PER_WORD;
-   for (j = 0; j < nvars; k++, j++) {
-   if (k >= BITS_PER_WORD) {
-   word = *rowp++;
-   k = 0;
-   }
-
-   if (word & (1 << k))
-   printf("  %s", symbol_name[start_symbol + j]);
-   }
-   }
-}
-
-void
-print_first_derives(void)
-{
-   int i, j;
-   unsigned int *rp;
-   unsigned int k, cword = 0;
-
-   printf("\n\n\nFirst Derives\n");
-
-   for (i = start_symbol; i < nsyms; i++) {
-   printf("\n%s derives\n", symbol_name[i]);
-   rp = first_derives + i * WORDSIZE(nrules);
-   k = BITS_PER_WORD;
-   for (j = 0; j <= nrules; k++, j++) {
-   if (k >= BITS_PER_WORD) {
-   cword = *rp++;
-   k = 0;
-   }
-
-   if (cword & (1 << k))
-   printf("   %d\n", j);
-   }
-   }
-
-   fflush(stdout);
-}
-
-#endif
Index: lalr.c
===
RCS file: /cvs/src/usr.bin/yacc/lalr.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 lalr.c
--- lalr.c  11 Dec 2015 20:25:47 -  1.18
+++ lalr.c  15 May 2017 16:56:56 -
@@ -52,7 +52,7 @@ short *goto_map;
 short *from_state;
 short *to_state;
 
-short **transpose();
+short **transpose(short **, int);
 void set_state_table(void);
 void set_accessing_symbol(void);
 void set_shift_table(void);
@@ -448,7 +448,7 @@ add_lookback_edge(int stateno, int rulen
 
 
 short **
-transpose(short **R, int n)
+transpose(short **old_R, int n)
 {
short **new_R, **temp_R, *nedges, *sp;
int i, k;
@@ -456,7 +456,7 @@ 

Re: IPsec ours policy check in IPv6 input

[Mike Belopuhov]

On 22 May 2017 at 21:02, Alexander Bluhm  wrote:
>
> Hi,
>
> In the IPv4 input path the IPsec policy is checked by
> ip_input_ipsec_ours_check().  This is missing in the IPv6 case.  So
> call this function also from ip6_local().
>
> ok?
>
> bluhm
>

This looks good, but please consider moving these functions out of
ip_input.c :-)

Re: qsort: store the partition value out of line

[Todd C. Miller]

On Sat, 20 May 2017 15:27:06 -0600, "Todd C. Miller" wrote:

> One optimization implemented in the sample code from "Engineering
> a Sort Function" that our qsort lacks is storing the partition value
> out of line when convenient.  Currently, we swap the partition value
> into a[0], but this can significantly degrade performance when the
> array is sorted in reverse or near-reverse order.
> 
> Since we don't want to allocate memory to store the value, only do
> this when the elements of the array are int or long sized (which
> is often the case).  This speeds up the qsort  regress test a bit,
> which is probably due to the tests on reverse sorted input.
> 
> This diff requires my "support swapping int-sized elements" diff
> be applied first.

Fixed diff.

 - todd

Index: lib/libc/stdlib/qsort.c
===
--- /usr/src/lib/libc/stdlib/qsort.cSat May 20 08:08:08 2017
+++ /usr/src/lib/libc/stdlib/qsort.cMon May 22 13:17:35 2017
@@ -40,15 +40,12 @@
  * Qsort routine from Bentley & McIlroy's "Engineering a Sort Function".
  *
  * This version differs from Bentley & McIlroy in the following ways:
- *   1. The partition value is swapped into a[0] instead of being
- * stored out of line.
- *
- *   2. It uses David Musser's introsort algorithm to fall back to
+ *   1. It uses David Musser's introsort algorithm to fall back to
  * heapsort(3) when the recursion depth reaches 2*lg(n + 1).
  * This avoids quicksort's quadratic behavior for pathological
  * input without appreciably changing the average run time.
  *
- *   3. Tail recursion is eliminated when sorting the larger of two
+ *   2. Tail recursion is eliminated when sorting the larger of two
  * subpartitions to save stack space.
  */
 #define SWAPTYPE_BYTEV 1
@@ -57,6 +54,23 @@
 #define SWAPTYPE_INT   4
 #define SWAPTYPE_LONG  5
 
+#define PVINIT(pv, pm) do {\
+   switch (swaptype) { \
+   case SWAPTYPE_INT:  \
+   pv = (char *)\
+   v.i = *(int *)pm;   \
+   break;  \
+   case SWAPTYPE_LONG: \
+   pv = (char *)\
+   v.l = *(long *)pm;  \
+   break;  \
+   default:\
+   pv = a; \
+   swap(pv, pm);   \
+   break;  \
+   }   \
+} while(0)
+
 #define TYPE_ALIGNED(TYPE, a, es)  \
(((char *)a - (char *)0) % sizeof(TYPE) == 0 && es % sizeof(TYPE) == 0)
 
@@ -122,9 +136,13 @@
 introsort(char *a, size_t n, size_t es, size_t maxdepth, int swaptype,
 int (*cmp)(const void *, const void *))
 {
-   char *pa, *pb, *pc, *pd, *pl, *pm, *pn;
+   char *pa, *pb, *pc, *pd, *pl, *pm, *pn, *pv;
int cmp_result;
size_t r, s;
+   union {
+   int i;
+   long l;
+   } v;
 
 loop:  if (maxdepth == 0) {
if (heapsort(a, n, es, cmp) == 0)
@@ -150,18 +168,18 @@
}
pm = med3(pl, pm, pn, cmp);
}
-   swap(a, pm);
-   pa = pb = a + es;
+   PVINIT(pv, pm); /* pv points to partition value */
+   pa = pb = a;
pc = pd = a + (n - 1) * es;
for (;;) {
-   while (pb <= pc && (cmp_result = cmp(pb, a)) <= 0) {
+   while (pb <= pc && (cmp_result = cmp(pb, pv)) <= 0) {
if (cmp_result == 0) {
swap(pa, pb);
pa += es;
}
pb += es;
}
-   while (pb <= pc && (cmp_result = cmp(pc, a)) >= 0) {
+   while (pb <= pc && (cmp_result = cmp(pc, pv)) >= 0) {
if (cmp_result == 0) {
swap(pc, pd);
pd -= es;

IPsec ours policy check in IPv6 input

[Alexander Bluhm]

Hi,

In the IPv4 input path the IPsec policy is checked by
ip_input_ipsec_ours_check().  This is missing in the IPv6 case.  So
call this function also from ip6_local().

ok?

bluhm

Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.302
diff -u -p -r1.302 ip_input.c
--- netinet/ip_input.c  16 May 2017 12:24:01 -  1.302
+++ netinet/ip_input.c  22 May 2017 18:42:00 -
@@ -129,9 +129,6 @@ static struct mbuf_queueipsend_mq;
 void   ip_ours(struct mbuf *);
 intip_dooptions(struct mbuf *, struct ifnet *);
 intin_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
-#ifdef IPSEC
-intip_input_ipsec_ours_check(struct mbuf *, int);
-#endif /* IPSEC */
 
 static void ip_send_dispatch(void *);
 static struct task ipsend_task = TASK_INITIALIZER(ip_send_dispatch, 
_mq);
@@ -583,7 +580,7 @@ ip_local(struct mbuf *m, int off, int nx
 
 #ifdef IPSEC
if (ipsec_in_use) {
-   if (ip_input_ipsec_ours_check(m, off) != 0) {
+   if (ip_input_ipsec_ours_check(m, off, nxt, AF_INET) != 0) {
ipstat_inc(ips_cantforward);
m_freem(m);
return;
@@ -707,9 +704,8 @@ ip_input_ipsec_fwd_check(struct mbuf *m,
 }
 
 int
-ip_input_ipsec_ours_check(struct mbuf *m, int hlen)
+ip_input_ipsec_ours_check(struct mbuf *m, int hlen, int proto, int af)
 {
-   struct ip *ip = mtod(m, struct ip *);
struct tdb *tdb;
struct tdb_ident *tdbi;
struct m_tag *mtag;
@@ -723,8 +719,8 @@ ip_input_ipsec_ours_check(struct mbuf *m
 * some flexibility in handling nested tunnels (in setting up
 * the policies).
 */
-   if ((ip->ip_p == IPPROTO_ESP) || (ip->ip_p == IPPROTO_AH) ||
-   (ip->ip_p == IPPROTO_IPCOMP))
+   if ((proto == IPPROTO_ESP) || (proto == IPPROTO_AH) ||
+   (proto == IPPROTO_IPCOMP))
return 0;
 
/*
@@ -735,7 +731,16 @@ ip_input_ipsec_ours_check(struct mbuf *m
 * the packet header (the encapsulation routines know how
 * to deal with that).
 */
-   if ((ip->ip_p == IPPROTO_IPIP) || (ip->ip_p == IPPROTO_IPV6))
+   if ((proto == IPPROTO_IPV4) || (proto == IPPROTO_IPV6))
+   return 0;
+
+   /*
+* When processing IPv6 header chains, do not look at the
+* outer header.  The inner protocol is relevant and will
+* be checked by the local delivery loop later.
+*/
+   if ((af == AF_INET6) && ((proto == IPPROTO_DSTOPTS) ||
+   (proto == IPPROTO_ROUTING) || (proto == IPPROTO_FRAGMENT)))
return 0;
 
/*
@@ -743,7 +748,7 @@ ip_input_ipsec_ours_check(struct mbuf *m
 * policy check in the respective input routine, so we can
 * check for bypass sockets.
 */
-   if ((ip->ip_p == IPPROTO_TCP) || (ip->ip_p == IPPROTO_UDP))
+   if ((proto == IPPROTO_TCP) || (proto == IPPROTO_UDP))
return 0;
 
/*
@@ -764,7 +769,7 @@ ip_input_ipsec_ours_check(struct mbuf *m
tdbi->proto);
} else
tdb = NULL;
-   ipsp_spd_lookup(m, AF_INET, hlen, , IPSP_DIRECTION_IN,
+   ipsp_spd_lookup(m, af, hlen, , IPSP_DIRECTION_IN,
tdb, NULL, 0);
 
return error;
Index: netinet/ip_var.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.73
diff -u -p -r1.73 ip_var.h
--- netinet/ip_var.h12 May 2017 23:05:58 -  1.73
+++ netinet/ip_var.h22 May 2017 18:02:37 -
@@ -252,6 +252,7 @@ void ipv4_input(struct mbuf *);
 voidip_local(struct mbuf *, int, int);
 voidip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
 int ip_input_ipsec_fwd_check(struct mbuf *, int, int);
+int ip_input_ipsec_ours_check(struct mbuf *, int, int, int);
 int rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
 voidrip_init(void);
 int rip_input(struct mbuf **, int *, int, int);
Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.186
diff -u -p -r1.186 ip6_input.c
--- netinet6/ip6_input.c12 May 2017 14:04:09 -  1.186
+++ netinet6/ip6_input.c22 May 2017 17:47:06 -
@@ -550,6 +550,18 @@ ip6_local(struct mbuf *m, int off, int n
goto bad;
}
 
+#ifdef IPSEC
+   if (ipsec_in_use) {
+   if (ip_input_ipsec_ours_check(m, off, nxt, AF_INET6)
+   != 0) {
+   ipstat_inc(ip6s_cantforward);
+   m_freem(m);
+   return;
+   }
+ 

Re: qsort: store the partition value out of line

[Todd C. Miller]

On Sat, 20 May 2017 15:27:06 -0600, "Todd C. Miller" wrote:

> One optimization implemented in the sample code from "Engineering
> a Sort Function" that our qsort lacks is storing the partition value
> out of line when convenient.  Currently, we swap the partition value
> into a[0], but this can significantly degrade performance when the
> array is sorted in reverse or near-reverse order.
> 
> Since we don't want to allocate memory to store the value, only do
> this when the elements of the array are int or long sized (which
> is often the case).  This speeds up the qsort  regress test a bit,
> which is probably due to the tests on reverse sorted input.

Ignore this diff for now, it can cause a crash in clang when the
optimizer is used.

 - todd

[patch/openbgpd] remove unused argument from community_ext_*

[Denis Fondras]

Hello,

Here is a patch to remove "u_int16_t neighas", an unused arguments from
community_ext_* functions.

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.298
diff -u -p -r1.298 parse.y
--- parse.y 22 Feb 2017 13:55:14 -  1.298
+++ parse.y 22 May 2017 18:52:27 -
@@ -848,7 +848,7 @@ rdomainopts : RD STRING {
 * RD is almost encode like an ext-community,
 * but only almost so convert here.
 */
-   if (community_ext_conv(, 0, )) {
+   if (community_ext_conv(, )) {
yyerror("bad encoding of rd");
YYERROR;
}
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.361
diff -u -p -r1.361 rde.c
--- rde.c   25 Jan 2017 03:21:55 -  1.361
+++ rde.c   22 May 2017 18:52:27 -
@@ -2455,7 +2455,7 @@ rde_rdomain_import(struct rde_aspath *as
struct filter_set   *s;
 
TAILQ_FOREACH(s, >import, entry) {
-   if (community_ext_match(asp, >action.ext_community, 0))
+   if (community_ext_match(asp, >action.ext_community))
return (1);
}
return (0);
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.160
diff -u -p -r1.160 rde.h
--- rde.h   25 Jan 2017 03:21:55 -  1.160
+++ rde.h   22 May 2017 18:52:27 -
@@ -379,12 +379,12 @@ intcommunity_large_set(struct rde_asp
 voidcommunity_large_delete(struct rde_aspath *, int64_t,
int64_t, int64_t);
 int community_ext_match(struct rde_aspath *,
-   struct filter_extcommunity *, u_int16_t);
+   struct filter_extcommunity *);
 int community_ext_set(struct rde_aspath *,
-   struct filter_extcommunity *, u_int16_t);
+   struct filter_extcommunity *);
 voidcommunity_ext_delete(struct rde_aspath *,
-   struct filter_extcommunity *, u_int16_t);
-int community_ext_conv(struct filter_extcommunity *, u_int16_t,
+   struct filter_extcommunity *);
+int community_ext_conv(struct filter_extcommunity *,
u_int64_t *);
 
 /* rde_decide.c */
Index: rde_attr.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.97
diff -u -p -r1.97 rde_attr.c
--- rde_attr.c  24 Jan 2017 04:22:42 -  1.97
+++ rde_attr.c  22 May 2017 18:52:27 -
@@ -974,7 +974,7 @@ aspath_lenmatch(struct aspath *a, enum a
  * Functions handling communities and extended communities.
  */
 
-int community_ext_matchone(struct filter_extcommunity *, u_int16_t, u_int64_t);
+int community_ext_matchone(struct filter_extcommunity *, u_int64_t);
 
 int
 community_match(struct rde_aspath *asp, int as, int type)
@@ -1115,8 +1115,7 @@ community_delete(struct rde_aspath *asp,
 }
 
 int
-community_ext_match(struct rde_aspath *asp, struct filter_extcommunity *c,
-u_int16_t neighas)
+community_ext_match(struct rde_aspath *asp, struct filter_extcommunity *c)
 {
struct attr *attr;
u_int8_t*p;
@@ -1131,7 +1130,7 @@ community_ext_match(struct rde_aspath *a
p = attr->data;
for (len = attr->len / sizeof(ec); len > 0; len--) {
memcpy(, p, sizeof(ec));
-   if (community_ext_matchone(c, neighas, ec))
+   if (community_ext_matchone(c, ec))
return (1);
p += sizeof(ec);
}
@@ -1140,8 +1139,7 @@ community_ext_match(struct rde_aspath *a
 }
 
 int
-community_ext_set(struct rde_aspath *asp, struct filter_extcommunity *c,
-u_int16_t neighas)
+community_ext_set(struct rde_aspath *asp, struct filter_extcommunity *c)
 {
struct attr *attr;
u_int8_t*p = NULL;
@@ -1149,7 +1147,7 @@ community_ext_set(struct rde_aspath *asp
unsigned int i, ncommunities = 0;
u_int8_t f = ATTR_OPTIONAL|ATTR_TRANSITIVE;
 
-   if (community_ext_conv(c, neighas, ))
+   if (community_ext_conv(c, ))
return (0);
 
attr = attr_optget(asp, ATTR_EXT_COMMUNITIES);
@@ -1188,8 +1186,7 @@ community_ext_set(struct rde_aspath *asp
 }
 
 void
-community_ext_delete(struct rde_aspath *asp, struct filter_extcommunity *c,
-u_int16_t neighas)
+community_ext_delete(struct rde_aspath *asp, struct filter_extcommunity *c)
 {
struct attr *attr;
u_int8_t*p, *n;
@@ -1197,7 +1194,7 @@ community_ext_delete(struct rde_aspath *
u_int16_tl, 

Re: kern.allowkmem in examples?

[Theo de Raadt]

Yes, that is a lot better.  The command could even suggest so
upon runttime failure ...

> > On Mon, May 22, 2017 at 02:00:51PM +0200, Jan Stary wrote:
> > > acpidump(8) wants kern.allowkmem.
> > > Does it make sense to put that into examples sysctl.conf?
> 
> On May 22 09:06:51, dera...@openbsd.org wrote:
> > No, in fact I want to kill that sysctl eventually, perhaps couple this
> > with securelevel=0, and never allow it otherwise.
> 
> On May 22 14:09:57, sema...@online.fr wrote:
> > It should be rare to need to run acpidump(8) post-boot. The system
> > already runs it at startup and stock files under /var/db/acpi.
> 
> On May 22 14:42:57, mark.kette...@xs4all.nl wrote:
> > The bootscript already runs acpidump ad stores the results in /var/db/acpi.
> > So there is no reason to run acpidump.
> 
> Should the manpage say so?
> 
> 
> Index: acpidump.8
> ===
> RCS file: /cvs/src/usr.sbin/acpidump/acpidump.8,v
> retrieving revision 1.17
> diff -u -p -r1.17 acpidump.8
> --- acpidump.826 Sep 2016 19:58:26 -  1.17
> +++ acpidump.822 May 2017 17:41:03 -
> @@ -71,9 +71,14 @@ requires the ability to open
>  which may be restricted based upon the value of the
>  .Ar kern.allowkmem
>  .Xr sysctl 8 .
> +.Pp
> +.Nm
> +is run at startup and stores the results in
> +.Pa /var/db/acpi .
>  .Sh FILES
>  .Bl -tag -width /dev/mem
>  .It Pa /dev/mem
> +.It Pa /var/db/acpi
>  .El
>  .Sh SEE ALSO
>  .Xr pkg_add 1 ,
> 

Re: kern.allowkmem in examples?

[Jan Stary]

> On Mon, May 22, 2017 at 02:00:51PM +0200, Jan Stary wrote:
> > acpidump(8) wants kern.allowkmem.
> > Does it make sense to put that into examples sysctl.conf?

On May 22 09:06:51, dera...@openbsd.org wrote:
> No, in fact I want to kill that sysctl eventually, perhaps couple this
> with securelevel=0, and never allow it otherwise.

On May 22 14:09:57, sema...@online.fr wrote:
> It should be rare to need to run acpidump(8) post-boot. The system
> already runs it at startup and stock files under /var/db/acpi.

On May 22 14:42:57, mark.kette...@xs4all.nl wrote:
> The bootscript already runs acpidump ad stores the results in /var/db/acpi.
> So there is no reason to run acpidump.

Should the manpage say so?


Index: acpidump.8
===
RCS file: /cvs/src/usr.sbin/acpidump/acpidump.8,v
retrieving revision 1.17
diff -u -p -r1.17 acpidump.8
--- acpidump.8  26 Sep 2016 19:58:26 -  1.17
+++ acpidump.8  22 May 2017 17:41:03 -
@@ -71,9 +71,14 @@ requires the ability to open
 which may be restricted based upon the value of the
 .Ar kern.allowkmem
 .Xr sysctl 8 .
+.Pp
+.Nm
+is run at startup and stores the results in
+.Pa /var/db/acpi .
 .Sh FILES
 .Bl -tag -width /dev/mem
 .It Pa /dev/mem
+.It Pa /var/db/acpi
 .El
 .Sh SEE ALSO
 .Xr pkg_add 1 ,

Re: kern.allowkmem in examples?

[Theo de Raadt]

No, in fact I want to kill that sysctl eventually, perhaps couple this
with securelevel=0, and never allow it otherwise.

The days of kernel memory snooping are OVER.

> acpidump(8) wants kern.allowkmem.
> Does it make sense to put that into examples sysctl.conf?
> 
>   Jan
> 
> Index: sysctl.conf
> ===
> RCS file: /cvs/src/etc/examples/sysctl.conf,v
> retrieving revision 1.4
> diff -u -p -r1.4 sysctl.conf
> --- sysctl.conf   3 Apr 2015 15:50:28 -   1.4
> +++ sysctl.conf   22 May 2017 11:58:27 -
> @@ -33,4 +33,5 @@
>  #kern.nosuidcoredump=3   # 3=Put suid coredumps in 
> /var/crash/progname
>  #kern.watchdog.period=32 # >0=Enable hardware watchdog(4) timer if 
> available
>  #kern.watchdog.auto=0# 0=Disable automatic watchdog(4) 
> retriggering
> +#kern.allowkmem=1# 1=Allow e.g. acpidump to open /dev/kmem
>  #hw.allowpowerdown=0 # 0=Disable power button shutdown
> 

Re: ksh(1): don't output invalid UTF-8 characters

[Boudewijn Dijkstra]

Op Fri, 19 May 2017 15:17:55 +0200 schreef Anton Lindqvist  
:

On Fri, May 19, 2017 at 09:33:33AM -0300, Lucas Gabriel Vuotto wrote:

On 19/05/17 03:42, Anton Lindqvist wrote:
>
> +static int
> +u8len(unsigned char c)
> +{
> +  switch (c & 0xF0) {
> +  case 0xF0:
> +  return 4;
> +  case 0xE0:
> +  return 3;
> +  case 0xC0:
> +  return 2;
> +  default:
> +  return 1;
> +  }
> +}
> +

This is wrong: most codepoints in the range U+0080-U+07ff (the ones  
greater than U+0400) would be interpreted as being 1 character long  
instead of 2.


Thanks for the heads-up. Maybe a more reliable solution would be to call
mbtowc(3) repeatedly as new input arrives until it returns successfully.
Assuming the first read byte is a UTF-8 start byte.


Not needed. Only case 0xD0 is missing.

case 0xC0: case 0xD0:
 return 2;



--
Gemaakt met Opera's e-mailprogramma: http://www.opera.com/mail/

Re: kern.allowkmem in examples?

[Mark Kettenis]

> Date: Mon, 22 May 2017 14:00:51 +0200
> From: Jan Stary 
> 
> acpidump(8) wants kern.allowkmem.
> Does it make sense to put that into examples sysctl.conf?

I'm not necessaily against adding and kern.allowkmem entry, but
acpidump is a bit of a bad example.  The bootscript already runs
acpidump ad stores the results in /var/db/acpi.  So there is no reason
to run acpidump.

> Index: sysctl.conf
> ===
> RCS file: /cvs/src/etc/examples/sysctl.conf,v
> retrieving revision 1.4
> diff -u -p -r1.4 sysctl.conf
> --- sysctl.conf   3 Apr 2015 15:50:28 -   1.4
> +++ sysctl.conf   22 May 2017 11:58:27 -
> @@ -33,4 +33,5 @@
>  #kern.nosuidcoredump=3   # 3=Put suid coredumps in 
> /var/crash/progname
>  #kern.watchdog.period=32 # >0=Enable hardware watchdog(4) timer if 
> available
>  #kern.watchdog.auto=0# 0=Disable automatic watchdog(4) 
> retriggering
> +#kern.allowkmem=1# 1=Allow e.g. acpidump to open /dev/kmem
>  #hw.allowpowerdown=0 # 0=Disable power button shutdown
> 
> 

Re: kern.allowkmem in examples?

[Sebastien Marie]

On Mon, May 22, 2017 at 02:00:51PM +0200, Jan Stary wrote:
> acpidump(8) wants kern.allowkmem.
> Does it make sense to put that into examples sysctl.conf?
> 

It should be rare to need to run acpidump(8) post-boot. The system
already runs it at startup and stock files under /var/db/acpi.

-- 
Sebastien Marie

kern.allowkmem in examples?

[Jan Stary]

acpidump(8) wants kern.allowkmem.
Does it make sense to put that into examples sysctl.conf?

Jan

Index: sysctl.conf
===
RCS file: /cvs/src/etc/examples/sysctl.conf,v
retrieving revision 1.4
diff -u -p -r1.4 sysctl.conf
--- sysctl.conf 3 Apr 2015 15:50:28 -   1.4
+++ sysctl.conf 22 May 2017 11:58:27 -
@@ -33,4 +33,5 @@
 #kern.nosuidcoredump=3 # 3=Put suid coredumps in /var/crash/progname
 #kern.watchdog.period=32   # >0=Enable hardware watchdog(4) timer if 
available
 #kern.watchdog.auto=0  # 0=Disable automatic watchdog(4) retriggering
+#kern.allowkmem=1  # 1=Allow e.g. acpidump to open /dev/kmem
 #hw.allowpowerdown=0   # 0=Disable power button shutdown

Re: [patch] Avoid system(3) in ikectl

[Jonathan Gray]

On Fri, May 19, 2017 at 09:56:14AM +0100, Stuart Henderson wrote:
> On 2017/05/19 00:32, Matthew Martin wrote:
> > ikectl errors in a number of situations where shell special characters
> > are used. For example:
> > 
> > % doas ikectl ca test create password \'
> > [...]
> > subject=/C=DE/ST=Lower Saxony/L=Hanover/O=OpenBSD/OU=iked/CN=VPN 
> > CA/emailAddress=r...@openbsd.org
> > Getting Private key
> > sh: no closing quote
> > 
> > This is because it uses system(3) in various places to run openssl, tar,
> > and zip. Take the hint from the system(3) man page, and write a small
> > function that does the fork and exec bypassing sh.
> 
> This seems like a good idea anyway, but this diff from Andrei-Marius Radu
> to stop passing the password on the command line is still pending :
> 
> https://marc.info/?l=openbsd-bugs=149064755410645=2

Trying to test this gets me

ikectl -q ca test2 certificate 10.0.0.6 revoke 
Using configuration from /etc/ssl/test2/ca-revoke-ssl.cnf
error on line 27 of config file '/etc/ssl/test2/ca-revoke-ssl.cnf'
30728805483616:error:0EFFF068:configuration file 
routines:CRYPTO_internal:variable has no 
value:/usr/src/lib/libcrypto/conf/conf_def.c:563:line 27
Using configuration from /etc/ssl/test2/ca-revoke-ssl.cnf
error on line 27 of config file '/etc/ssl/test2/ca-revoke-ssl.cnf'
19976781949600:error:0EFFF068:configuration file 
routines:CRYPTO_internal:variable has no 
value:/usr/src/lib/libcrypto/conf/conf_def.c:563:line 27

21  [ req ]
22  #default_bits   = 2048
23  #default_md = sha256
24  #default_keyfile= privkey.pem
25  distinguished_name  = req_distinguished_name
26  #attributes = req_attributes
27  req_extensions  = $ENV::REQ_EXT

I'm ok with the Andrei-Marius Radu diff going in.  Though below diff
should go in as well.  Then we can start looking at the other two diffs.

Index: ikeca.c
===
RCS file: /cvs/src/usr.sbin/ikectl/ikeca.c,v
retrieving revision 1.43
diff -u -p -r1.43 ikeca.c
--- ikeca.c 21 May 2017 02:37:52 -  1.43
+++ ikeca.c 22 May 2017 10:17:08 -
@@ -900,6 +900,7 @@ ca_revoke(struct ca *ca, char *keyname)
 
ca_setenv("$ENV::CADB", ca->index);
ca_setenv("$ENV::CASERIAL", ca->serial);
+   ca_setenv("$ENV::REQ_EXT", "req");
ca_setcnf(ca, "ca-revoke");
 
if (keyname) {

Re: [PATCH] add D-Link DWA-525 rev A2 to ral(4)

[Kevin Lo]

On Mon, May 22, 2017 at 03:50:27PM +1000, Jonathan Gray wrote:
> 
> On Mon, May 22, 2017 at 01:13:34PM +0800, Kevin Lo wrote:
> > Hi,
> > 
> > The diff below adds D-Link DWA-525 rev A2 to ral(4) which works fine on 
> > amd64
> > and update ral(4) manpage.
> 
> This could also add other missing ids.
> 
> RALINK
> 0x3390 (RT33XX)
> 0x359f (RT35XX)
> 0x5362 (RT53XX)
> 
> EDIMAX
> 0x7711 (RT35XX)
> 0x7722 (RT35XX)

If we want to add those missing ids, rt2860_get_rf() function needs to be
updated as well.  Since I don't have those chips, I'm not sure if they work.

> 
> You don't need to change $Mdocdate$, that gets expanded automatically.

Ok, here's the revised patch, thanks.

Index: share/man/man4/ral.4
===
RCS file: /cvs/src/share/man/man4/ral.4,v
retrieving revision 1.110
diff -u -p -u -p -r1.110 ral.4
--- share/man/man4/ral.417 Aug 2016 11:52:29 -  1.110
+++ share/man/man4/ral.422 May 2017 06:04:03 -
@@ -160,6 +160,7 @@ CNet CWP-854.
 Compex WLP54G.
 Conceptronic C54Ri.
 Corega CG-WLPCI54GL.
+D-Link DWA-525 rev A2.
 Digitus DN-7006G-RA.
 Dynalink WLG25PCI.
 E-Tech WGPI02.
Index: sys/dev/ic/rt2860.c
===
RCS file: /cvs/src/sys/dev/ic/rt2860.c,v
retrieving revision 1.92
diff -u -p -u -p -r1.92 rt2860.c
--- sys/dev/ic/rt2860.c 22 Jan 2017 10:17:38 -  1.92
+++ sys/dev/ic/rt2860.c 22 May 2017 06:04:06 -
@@ -3106,6 +3106,7 @@ rt2860_get_rf(uint16_t rev)
case RT3070_RF_3052:return "RT3052";
case RT3070_RF_3320:return "RT3320";
case RT3070_RF_3053:return "RT3053";
+   case RT5390_RF_5360:return "RT5360";
case RT5390_RF_5390:return "RT5390";
case RT5390_RF_5392:return "RT5392";
default:return "unknown";
Index: sys/dev/ic/rt2860reg.h
===
RCS file: /cvs/src/sys/dev/ic/rt2860reg.h,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 rt2860reg.h
--- sys/dev/ic/rt2860reg.h  17 Aug 2016 11:50:52 -  1.33
+++ sys/dev/ic/rt2860reg.h  22 May 2017 06:04:06 -
@@ -925,6 +925,7 @@ struct rt2860_rxwi {
 #define RT3070_RF_3320 0x000b  /* 1T1R */
 #define RT3070_RF_3053 0x000d  /* dual-band 3T3R */
 #define RT5592_RF_5592 0x000f  /* dual-band 2T2R */
+#define RT5390_RF_5360 0x5360  /* 1T1R */
 #define RT5390_RF_5370 0x5370  /* 1T1R */
 #define RT5390_RF_5372 0x5372  /* 2T2R */
 #define RT5390_RF_5390 0x5390  /* 1T1R */
Index: sys/dev/pci/if_ral_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_ral_pci.c,v
retrieving revision 1.25
diff -u -p -u -p -r1.25 if_ral_pci.c
--- sys/dev/pci/if_ral_pci.c17 Aug 2016 11:50:52 -  1.25
+++ sys/dev/pci/if_ral_pci.c22 May 2017 06:04:07 -
@@ -136,6 +136,7 @@ const struct pci_matchid ral_pci_devices
{ PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT3562 },
{ PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT3592 },
{ PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT3593 },
+   { PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT5360 },
{ PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT5390 },
{ PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT5392 },
{ PCI_VENDOR_RALINK, PCI_PRODUCT_RALINK_RT5390_1 },
Index: sys/dev/pci/pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1818
diff -u -p -u -p -r1.1818 pcidevs
--- sys/dev/pci/pcidevs 17 May 2017 05:00:17 -  1.1818
+++ sys/dev/pci/pcidevs 22 May 2017 06:04:08 -
@@ -6560,6 +6560,7 @@ product RALINK RT3298 0x3298  Bluetooth
 product RALINK RT3562  0x3562  RT3562
 product RALINK RT3592  0x3592  RT3592
 product RALINK RT3593  0x3593  RT3593
+product RALINK RT5360  0x5360  RT5360
 product RALINK RT5390  0x5390  RT5390
 product RALINK RT5392  0x5392  RT5392
 product RALINK RT5390_10x539a  RT5390