Re: preparing pfi_kif to MP world

[Martin Pieuchot]

On 29/10/15(Thu) 13:32, Mike Belopuhov wrote:
> On Thu, Oct 29, 2015 at 11:58 +0100, Martin Pieuchot wrote:
> > On 29/10/15(Thu) 02:49, Mike Belopuhov wrote:
> > > On 28 October 2015 at 18:41, Alexandr Nedvedicky
> > >  wrote:
> > > > Hello Mike,
> > > >
> > > > just a quick question:
> > > >
> > > > are you going to commit your pfi_kif_find() et. al.?
> > > > or more work is needed there?
> > > >
> > > 
> > > I need OKs
> > > [...]
> > > >  pfsync_in_clr(caddr_t buf, int len, int count, int flags)
> > > >  {
> > > > struct pfsync_clr *clr;
> > > > -   int i;
> > > > -
> > > > struct pf_state *st, *nexts;
> > > > -   struct pf_state_key *sk, *nextsk;
> > > > -   struct pf_state_item *si;
> > > > +   struct pfi_kif *kif = NULL;
> > > > u_int32_t creatorid;
> > > > +   int i;
> > > >
> > > > for (i = 0; i < count; i++) {
> > > > clr = (struct pfsync_clr *)buf + len * i;
> > > > creatorid = clr->creatorid;
> > > > +   if (strlen(clr->ifname) &&
> > > > +   (kif = pfi_kif_find(clr->ifname)) == NULL)
> > > > +   continue;
> > > > -[...]
> > > > +   for (st = RB_MIN(pf_state_tree_id, _id); st; st = 
> > > > nexts) {
> > > > +   nexts = RB_NEXT(pf_state_tree_id, _id, st);
> > > > +   if (st->creatorid == creatorid &&
> > > > +   ((kif && st->kif == kif) || !kif)) {
> > 
> > I find this check confusing.
> > 
> > Since you're continuing when "kif == NULL" can't you change this check
> > into "st->kif == kif"?
> >
> 
> No.
> 
> > Or is it possible for strlen(clr->ifname) to return 0
> 
> Yes.
> 
> > in which case you
> > might end up comparing a different ``kif''?
> >
> 
> To garbage even...
> 
> ...and I need to move the "kif = NULL" after creatorid actually.
> It's rather unlikely to get a message with several entries having
> clr->ifname set, but the code must be correct nevertheless.

ok mpi@

> 
> diff --git sys/net/if_pfsync.c sys/net/if_pfsync.c
> index 7d633db..5296642 100644
> --- sys/net/if_pfsync.c
> +++ sys/net/if_pfsync.c
> @@ -752,46 +752,29 @@ done:
>  
>  int
>  pfsync_in_clr(caddr_t buf, int len, int count, int flags)
>  {
>   struct pfsync_clr *clr;
> - int i;
> -
>   struct pf_state *st, *nexts;
> - struct pf_state_key *sk, *nextsk;
> - struct pf_state_item *si;
> + struct pfi_kif *kif;
>   u_int32_t creatorid;
> + int i;
>  
>   for (i = 0; i < count; i++) {
>   clr = (struct pfsync_clr *)buf + len * i;
> + kif = NULL;
>   creatorid = clr->creatorid;
> + if (strlen(clr->ifname) &&
> + (kif = pfi_kif_find(clr->ifname)) == NULL)
> + continue;
>  
> - if (clr->ifname[0] == '\0') {
> - for (st = RB_MIN(pf_state_tree_id, _id);
> - st; st = nexts) {
> - nexts = RB_NEXT(pf_state_tree_id, _id, st);
> - if (st->creatorid == creatorid) {
> - SET(st->state_flags, PFSTATE_NOSYNC);
> - pf_unlink_state(st);
> - }
> - }
> - } else {
> - if (pfi_kif_get(clr->ifname) == NULL)
> - continue;
> -
> - /* XXX correct? */
> - for (sk = RB_MIN(pf_state_tree, _statetbl);
> - sk; sk = nextsk) {
> - nextsk = RB_NEXT(pf_state_tree,
> - _statetbl, sk);
> - TAILQ_FOREACH(si, >states, entry) {
> - if (si->s->creatorid == creatorid) {
> - SET(si->s->state_flags,
> - PFSTATE_NOSYNC);
> - pf_unlink_state(si->s);
> - }
> - }
> + for (st = RB_MIN(pf_state_tree_id, _id); st; st = nexts) {
> + nexts = RB_NEXT(pf_state_tree_id, _id, st);
> + if (st->creatorid == creatorid &&
> + ((kif && st->kif == kif) || !kif)) {
> + SET(st->state_flags, PFSTATE_NOSYNC);
> + pf_unlink_state(st);
>   }
>   }
>   }
>  
>   return (0);
> diff --git sys/net/pf_if.c sys/net/pf_if.c
> index caaf9f9..bf77184 100644
> --- sys/net/pf_if.c
> +++ sys/net/pf_if.c
> @@ -97,18 +97,25 @@ pfi_initialize(void)
>   if ((pfi_all = pfi_kif_get(IFG_ALL)) == NULL)
>   panic("pfi_kif_get for pfi_all failed");
>  }
>  
>  struct pfi_kif *
> 

bank-donation.html SEPA BIC format error

[Marcus MERIGHI]

so diffs go to tech@? even that one?

Reason: the electronic banking form of my bank does not like the
formatting of the BIC number. 
Possibly others have different experiences...

(In the SWIFT column the proposed formatting is already used.)

Bye, Marcus

Index: bank-donation.html
===
RCS file: /cvs/www/bank-donation.html,v
retrieving revision 1.23
diff -u -p -u -r1.23 bank-donation.html
--- bank-donation.html  11 May 2015 11:18:29 -  1.23
+++ bank-donation.html  29 Oct 2015 12:38:55 -
@@ -21,7 +21,7 @@ the following information:
 From Inside Europe (SEPA):
 
 IBAN: DE91 7007 0024 0338 1779 00
-BIC: DEUT DE DBMUC
+BIC: DEUTDEDBMUC
 Name: Theo de Raadt
 Address: Deutsche Bank, Marienplatz 21
 80331 Mnchen, Germany

Re: preparing pfi_kif to MP world

[Mike Belopuhov]

On Thu, Oct 29, 2015 at 11:58 +0100, Martin Pieuchot wrote:
> On 29/10/15(Thu) 02:49, Mike Belopuhov wrote:
> > On 28 October 2015 at 18:41, Alexandr Nedvedicky
> >  wrote:
> > > Hello Mike,
> > >
> > > just a quick question:
> > >
> > > are you going to commit your pfi_kif_find() et. al.?
> > > or more work is needed there?
> > >
> > 
> > I need OKs
> > [...]
> > >  pfsync_in_clr(caddr_t buf, int len, int count, int flags)
> > >  {
> > > struct pfsync_clr *clr;
> > > -   int i;
> > > -
> > > struct pf_state *st, *nexts;
> > > -   struct pf_state_key *sk, *nextsk;
> > > -   struct pf_state_item *si;
> > > +   struct pfi_kif *kif = NULL;
> > > u_int32_t creatorid;
> > > +   int i;
> > >
> > > for (i = 0; i < count; i++) {
> > > clr = (struct pfsync_clr *)buf + len * i;
> > > creatorid = clr->creatorid;
> > > +   if (strlen(clr->ifname) &&
> > > +   (kif = pfi_kif_find(clr->ifname)) == NULL)
> > > +   continue;
> > > -[...]
> > > +   for (st = RB_MIN(pf_state_tree_id, _id); st; st = 
> > > nexts) {
> > > +   nexts = RB_NEXT(pf_state_tree_id, _id, st);
> > > +   if (st->creatorid == creatorid &&
> > > +   ((kif && st->kif == kif) || !kif)) {
> 
> I find this check confusing.
> 
> Since you're continuing when "kif == NULL" can't you change this check
> into "st->kif == kif"?
>

No.

> Or is it possible for strlen(clr->ifname) to return 0

Yes.

> in which case you
> might end up comparing a different ``kif''?
>

To garbage even...

...and I need to move the "kif = NULL" after creatorid actually.
It's rather unlikely to get a message with several entries having
clr->ifname set, but the code must be correct nevertheless.

diff --git sys/net/if_pfsync.c sys/net/if_pfsync.c
index 7d633db..5296642 100644
--- sys/net/if_pfsync.c
+++ sys/net/if_pfsync.c
@@ -752,46 +752,29 @@ done:
 
 int
 pfsync_in_clr(caddr_t buf, int len, int count, int flags)
 {
struct pfsync_clr *clr;
-   int i;
-
struct pf_state *st, *nexts;
-   struct pf_state_key *sk, *nextsk;
-   struct pf_state_item *si;
+   struct pfi_kif *kif;
u_int32_t creatorid;
+   int i;
 
for (i = 0; i < count; i++) {
clr = (struct pfsync_clr *)buf + len * i;
+   kif = NULL;
creatorid = clr->creatorid;
+   if (strlen(clr->ifname) &&
+   (kif = pfi_kif_find(clr->ifname)) == NULL)
+   continue;
 
-   if (clr->ifname[0] == '\0') {
-   for (st = RB_MIN(pf_state_tree_id, _id);
-   st; st = nexts) {
-   nexts = RB_NEXT(pf_state_tree_id, _id, st);
-   if (st->creatorid == creatorid) {
-   SET(st->state_flags, PFSTATE_NOSYNC);
-   pf_unlink_state(st);
-   }
-   }
-   } else {
-   if (pfi_kif_get(clr->ifname) == NULL)
-   continue;
-
-   /* XXX correct? */
-   for (sk = RB_MIN(pf_state_tree, _statetbl);
-   sk; sk = nextsk) {
-   nextsk = RB_NEXT(pf_state_tree,
-   _statetbl, sk);
-   TAILQ_FOREACH(si, >states, entry) {
-   if (si->s->creatorid == creatorid) {
-   SET(si->s->state_flags,
-   PFSTATE_NOSYNC);
-   pf_unlink_state(si->s);
-   }
-   }
+   for (st = RB_MIN(pf_state_tree_id, _id); st; st = nexts) {
+   nexts = RB_NEXT(pf_state_tree_id, _id, st);
+   if (st->creatorid == creatorid &&
+   ((kif && st->kif == kif) || !kif)) {
+   SET(st->state_flags, PFSTATE_NOSYNC);
+   pf_unlink_state(st);
}
}
}
 
return (0);
diff --git sys/net/pf_if.c sys/net/pf_if.c
index caaf9f9..bf77184 100644
--- sys/net/pf_if.c
+++ sys/net/pf_if.c
@@ -97,18 +97,25 @@ pfi_initialize(void)
if ((pfi_all = pfi_kif_get(IFG_ALL)) == NULL)
panic("pfi_kif_get for pfi_all failed");
 }
 
 struct pfi_kif *
-pfi_kif_get(const char *kif_name)
+pfi_kif_find(const char *kif_name)
 {
-   struct pfi_kif  *kif;
struct pfi_kif_cmp   s;
 
bzero(, sizeof(s));
strlcpy(s.pfik_name, kif_name, 

Re: ntpd doesn't work with -sv - syscall 5

[Sebastien Marie]

On Thu, Oct 29, 2015 at 12:17:15PM +0200, Gregory Edigarov wrote:
> Hello
> 
> ntpd_flags=-sv
> 
> Oct 29 11:19:32 lbld12 /bsd: ntpd(15132): syscall 5
> Oct 29 11:19:32 lbld12 ntpd[697]: ntp_dispatch_imsg in ntp engine: pipe
> closed
> Oct 29 11:19:32 lbld12 ntpd[10730]: dispatch_imsg in main: pipe closed
> 
> Without flags everithing works.
> 

Without a dmesg or more information it is a bit complex to be sure... I
suppose your running kernel isn't enough current. A problem like that
was corrected some days ago (by revision 1.72 of
src/sys/kern/kern_pledge.c).

Please check your running kernel (the rev 1.72 is from Sun Oct 25
11:09:28 2015 UTC).

If the problem is on latest -current, please include in your report at
least:
  - dmesg
  - a kdump extract of the ntpd running
ktrace -di /etc/rc.d/ntpd start ; kdump | tail -40

For a more complete report for pledge(2) issue, please refer to previous
mails in tech: http://marc.info/?l=openbsd-tech=144412493925465=2
(note: pledge(2) was tame(2) before).

Thanks.
-- 
Sebastien Marie

Re: calloc -> malloc in get_data() and get_string()

[Sebastian Benoit]

Michael McConville(mm...@mykolab.com) on 2015.10.28 12:05:24 -0400:
> Relayd, httpd, and ntpd define the functions get_data() and
> get_string(). Both call calloc and then immediately memcpy. Calloc's
> zeroing isn't optimized out. These functions are called in network data
> paths in at least a couple places, so all this extra writing could have
> a meaningful performance impact.

in relayd these functions are *not* called in the network path, they are used
when loading the (TLS) configuration data.

they dont have a performance impact.

> 
> 
> Index: relayd.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 relayd.c
> --- relayd.c  14 Oct 2015 07:58:14 -  1.144
> +++ relayd.c  28 Oct 2015 15:57:16 -
> @@ -1501,9 +1501,10 @@ get_string(u_int8_t *ptr, size_t len)
>   isspace((unsigned char)ptr[i])))
>   break;
>  
> - if ((str = calloc(1, i + 1)) == NULL)
> + if ((str = malloc(i + 1)) == NULL)
>   return (NULL);
>   memcpy(str, ptr, i);
> + str[i] = '\0';
>  
>   return (str);
>  }
> @@ -1513,7 +1514,7 @@ get_data(u_int8_t *ptr, size_t len)
>  {
>   u_int8_t*data;
>  
> - if ((data = calloc(1, len)) == NULL)
> + if ((data = malloc(len)) == NULL)
>   return (NULL);
>   memcpy(data, ptr, len);
>  
> 

-- 

Stop using rt_ifp in nd6*

[Martin Pieuchot]

When we already had a valid ``ifp'' I used it.  Since defrouter_lookup()
is only doing a comparison, let's use interface indexes.

ok?

Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.166
diff -u -p -r1.166 nd6.c
--- netinet6/nd6.c  29 Oct 2015 14:28:34 -  1.166
+++ netinet6/nd6.c  29 Oct 2015 14:49:03 -
@@ -658,7 +658,7 @@ nd6_lookup(struct in6_addr *addr6, int c
 */
if ((rt->rt_flags & RTF_GATEWAY) || (rt->rt_flags & RTF_LLINFO) == 0 ||
rt->rt_gateway->sa_family != AF_LINK || rt->rt_llinfo == NULL ||
-   (ifp != NULL && rt->rt_ifp != ifp)) {
+   (ifp != NULL && rt->rt_ifidx != ifp->if_index)) {
if (create) {
char addr[INET6_ADDRSTRLEN];
nd6log((LOG_DEBUG, "%s: failed to lookup %s (if=%s)\n",
@@ -751,17 +751,19 @@ nd6_free(struct rtentry *rt, int gc)
struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo, *next;
struct in6_addr in6 = satosin6(rt_key(rt))->sin6_addr;
struct nd_defrouter *dr;
+   struct ifnet *ifp;
int s;
 
/*
 * we used to have pfctlinput(PRC_HOSTDEAD) here.
 * even though it is not harmful, it was not really necessary.
 */
+   ifp = if_get(rt->rt_ifidx);
 
s = splsoftnet();
if (!ip6_forwarding) {
dr = defrouter_lookup((rt_key(rt))->sin6_addr,
-   rt->rt_ifp);
+   rt->rt_ifidx);
 
if (dr != NULL && dr->expire &&
ln->ln_state == ND6_LLINFO_STALE && gc) {
@@ -783,6 +785,7 @@ nd6_free(struct rtentry *rt, int gc)
} else
nd6_llinfo_settimer(ln, (long)nd6_gctimer * hz);
splx(s);
+   if_put(ifp);
return (ln->ln_next);
}
 
@@ -792,7 +795,7 @@ nd6_free(struct rtentry *rt, int gc)
 * is in the Default Router List.
 * See a corresponding comment in nd6_na_input().
 */
-   rt6_flush(, rt->rt_ifp);
+   rt6_flush(, ifp);
}
 
if (dr) {
@@ -839,9 +842,11 @@ nd6_free(struct rtentry *rt, int gc)
 * caches, and disable the route entry not to be used in already
 * cached routes.
 */
-   rtdeletemsg(rt, rt->rt_ifp->if_rdomain);
+   rtdeletemsg(rt, ifp->if_rdomain);
splx(s);
 
+   if_put(ifp);
+
return (next);
 }
 
@@ -899,7 +904,8 @@ nd6_rtrequest(struct ifnet *ifp, int req
_any) && rt_mask(rt) && (rt_mask(rt)->sa_len == 0 ||
IN6_ARE_ADDR_EQUAL(&(satosin6(rt_mask(rt)))->sin6_addr,
_any {
-   dr = defrouter_lookup((gate)->sin6_addr, ifp);
+   dr = defrouter_lookup((gate)->sin6_addr,
+   ifp->if_index);
if (dr)
dr->installed = 0;
}
Index: netinet6/nd6.h
===
RCS file: /cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.52
diff -u -p -r1.52 nd6.h
--- netinet6/nd6.h  28 Oct 2015 12:14:25 -  1.52
+++ netinet6/nd6.h  29 Oct 2015 14:42:35 -
@@ -299,7 +299,7 @@ int prelist_update(struct nd_prefix *, s
 int nd6_prelist_add(struct nd_prefix *, struct nd_defrouter *,
struct nd_prefix **);
 void pfxlist_onlink_check(void);
-struct nd_defrouter *defrouter_lookup(struct in6_addr *, struct ifnet *);
+struct nd_defrouter *defrouter_lookup(struct in6_addr *, unsigned int);
 
 struct nd_prefix *nd6_prefix_lookup(struct nd_prefix *);
 int in6_ifdel(struct ifnet *, struct in6_addr *);
Index: netinet6/nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.97
diff -u -p -r1.97 nd6_nbr.c
--- netinet6/nd6_nbr.c  22 Oct 2015 15:37:47 -  1.97
+++ netinet6/nd6_nbr.c  29 Oct 2015 14:47:42 -
@@ -730,7 +730,7 @@ nd6_na_input(struct mbuf *m, int off, in
ln->ln_byhint = 0;
if (!ND6_LLINFO_PERMANENT(ln)) {
nd6_llinfo_settimer(ln,
-   (long)ND_IFINFO(rt->rt_ifp)->reachable * 
hz);
+   (long)ND_IFINFO(ifp)->reachable * hz);
}
} else {
ln->ln_state = ND6_LLINFO_STALE;
@@ -851,7 +851,7 @@ nd6_na_input(struct mbuf *m, int off, in
 * context.  However, we keep it just for safety.
 */
s = splsoftnet();
-   dr = defrouter_lookup(in6, rt->rt_ifp);
+   dr = defrouter_lookup(in6, rt->rt_ifidx);
 

rt_ifix for ip6_forward()

[Martin Pieuchot]

Stop using rt_ifp in this function.

ok?

Index: netinet6/ip6_forward.c
===
RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.85
diff -u -p -r1.85 ip6_forward.c
--- netinet6/ip6_forward.c  28 Oct 2015 12:14:25 -  1.85
+++ netinet6/ip6_forward.c  29 Oct 2015 14:36:35 -
@@ -89,6 +89,7 @@ ip6_forward(struct mbuf *m, int srcrt)
struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
struct sockaddr_in6 *dst;
struct rtentry *rt;
+   struct ifnet *ifp = NULL;
int error = 0, type = 0, code = 0;
struct mbuf *mcopy = NULL;
 #ifdef IPSEC
@@ -362,11 +363,12 @@ reroute:
 * Also, don't send redirect if forwarding using a route
 * modified by a redirect.
 */
-   if (rt->rt_ifp->if_index == m->m_pkthdr.ph_ifidx && !srcrt &&
+   ifp = if_get(rt->rt_ifidx);
+   if (rt->rt_ifidx == m->m_pkthdr.ph_ifidx && !srcrt &&
ip6_sendredirects &&
(rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0) {
-   if ((rt->rt_ifp->if_flags & IFF_POINTOPOINT) &&
-   nd6_is_addr_neighbor(_forward_rt.ro_dst, rt->rt_ifp)) {
+   if ((ifp->if_flags & IFF_POINTOPOINT) &&
+   nd6_is_addr_neighbor(_forward_rt.ro_dst, ifp)) {
/*
 * If the incoming interface is equal to the outgoing
 * one, the link attached to the interface is
@@ -405,7 +407,7 @@ reroute:
ip6->ip6_dst.s6_addr16[1] = 0;
 
 #if NPF > 0
-   if (pf_test(AF_INET6, PF_FWD, rt->rt_ifp, ) != PF_PASS) {
+   if (pf_test(AF_INET6, PF_FWD, ifp, ) != PF_PASS) {
m_freem(m);
goto senderr;
}
@@ -420,21 +422,23 @@ reroute:
/* tag as generated to skip over pf_test on rerun */
m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
srcrt = 1;
+   if_put(ifp);
+   ifp = NULL;
goto reroute;
}
 #endif
-   in6_proto_cksum_out(m, rt->rt_ifp);
+   in6_proto_cksum_out(m, ifp);
 
/* Check the size after pf_test to give pf a chance to refragment. */
-   if (m->m_pkthdr.len > rt->rt_ifp->if_mtu) {
+   if (m->m_pkthdr.len > ifp->if_mtu) {
if (mcopy)
icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0,
-   rt->rt_ifp->if_mtu);
+   ifp->if_mtu);
m_freem(m);
goto freert;
}
 
-   error = nd6_output(rt->rt_ifp, m, dst, rt);
+   error = nd6_output(ifp, m, dst, rt);
if (error) {
ip6stat.ip6s_cantforward++;
} else {
@@ -490,5 +494,5 @@ senderr:
ip6_forward_rt.ro_rt = NULL;
}
 #endif
-   return;
+   if_put(ifp);
 }

upgrade58.html - remove tip(1)

[LÉVAI Dániel]

Hi!

I guess, if tip(1) has been removed, this is correct:


--- upgrade58.html.orig 2015-10-29 16:56:17.577783347 +0100
+++ upgrade58.html  2015-10-29 16:56:40.686678568 +0100
@@ -436,6 +436,7 @@ rm -f /usr/bin/sudo /usr/bin/sudoedit /u
 rm -f /usr/share/man/man8/sudo.8 /usr/share/man/man8/sudoedit.8
 rm -f /usr/share/man/man8/visudo.8 /usr/share/man/man5/sudoers.5
 rm -f /usr/libexec/sudo_noexec.so
+rm -f /usr/bin/tip /usr/share/man/man1/tip.1
 
 
 



Daniel

-- 
LÉVAI Dániel
PGP key ID = 0x83B63A8F
Key fingerprint = DBEC C66B A47A DFA2 792D  650C C69B BE4C 83B6 3A8F

Re: ntpd doesn't work with -sv - syscall 5

[Michael McConville]

Gregory Edigarov wrote:
> ntpd_flags=-sv
> 
> Oct 29 11:19:32 lbld12 /bsd: ntpd(15132): syscall 5
> Oct 29 11:19:32 lbld12 ntpd[697]: ntp_dispatch_imsg in ntp engine: pipe
> closed
> Oct 29 11:19:32 lbld12 ntpd[10730]: dispatch_imsg in main: pipe closed
> 
> Without flags everithing works.

We at least partially fixed this a few days ago. How recent is your
install?

Re: ntpd doesn't work with -sv - syscall 5

[Gregory Edigarov]

On 10/29/2015 06:29 PM, Michael McConville wrote:

Gregory Edigarov wrote:

ntpd_flags=-sv

Oct 29 11:19:32 lbld12 /bsd: ntpd(15132): syscall 5
Oct 29 11:19:32 lbld12 ntpd[697]: ntp_dispatch_imsg in ntp engine: pipe
closed
Oct 29 11:19:32 lbld12 ntpd[10730]: dispatch_imsg in main: pipe closed

Without flags everithing works.

We at least partially fixed this a few days ago. How recent is your
install?


Sorry for the buzz with today's -current it's ok now

Re: pledge idea

[Reyk Floeter]

On Thu, Oct 29, 2015 at 04:32:25PM +, Peter J. Philipp wrote:
> Hi deraadt,
> 
> I know you know I don't code well, but in order to show you what's on my 
> mind I had to write code, I took the bsearch() from the ieee80211 code, so
> perhaps there is a better way (like always) perhaps to unify the function 
> between these two areas.
> 
> The reason I did this is to save on cpu cycles when you look at X amount
> of processes all pledging...one time'd process won't show much difference.
> 

I'm not deraadt, but -

smart but have you considered that this is not worth the effort?
pledge() is only called once or twice during a process' lifetime -
start, pledge, run - the linear search on such a short list is still
relatively fast, and the entries are even sorted in the order of
relevance.  By convention "stdio" always comes first.  So what is the
impact without your diff of pledge in src/bin/sleep/sleep.c:

if (pledge("stdio", NULL) == -1)
err(1, "pledge");

$ time obj/sleep 0.01 
0m00.01s real 0m00.00s user 0m00.01s system

Are you trying to solve an issue?

Reyk

> below's diff:
> 
> -peter
> 
> 
> ? pledge.diff
> Index: kern/kern_pledge.c
> ===
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.90
> diff -u -p -u -r1.90 kern_pledge.c
> --- kern/kern_pledge.c28 Oct 2015 17:38:52 -  1.90
> +++ kern/kern_pledge.c29 Oct 2015 16:23:04 -
> @@ -58,6 +58,32 @@
>  
>  int canonpath(const char *input, char *buf, size_t bufsize);
>  int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
> +int pledge_cmp(const void *pi, const void *ph);
> +
> +/* taken from net80211/ieee80211_regdomain.c */
> +const void *pledge_bsearch(const void *, const void *, size_t, size_t,
> +int (*)(const void *, const void *));
> +
> +const void *
> +pledge_bsearch(const void *key, const void *base0, size_t nmemb, size_t size,
> +int (*compar)(const void *, const void *))
> +{
> +const char *base = base0;
> +int lim, cmp;
> +const void *p;
> +
> +for (lim = nmemb; lim != 0; lim >>= 1) {
> +p = base + (lim >> 1) * size;
> +cmp = (*compar)(key, p);
> +if (cmp == 0)
> +return ((const void *)p);
> +if (cmp > 0) {  /* key > p: move right */
> +base = (const char *)p + size;
> +lim--;
> +} /* else move left */
> +}
> +return (NULL);
> +}
>  
>  const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
>   [SYS_exit] = 0x,
> @@ -256,40 +282,42 @@ const u_int pledge_syscalls[SYS_MAXSYSCA
>   [SYS_swapctl] = PLEDGE_VMINFO,  /* XXX should limit to "get" operations 
> */
>  };
>  
> -static const struct {
> +/* MUST be sorted! */
> +static const struct PR {
>   char *name;
>   int flags;
>  } pledgereq[] = {
> - { "stdio",  PLEDGE_STDIO },
> - { "rpath",  PLEDGE_RPATH },
> - { "wpath",  PLEDGE_WPATH },
> - { "tmppath",PLEDGE_TMPPATH },
> - { "inet",   PLEDGE_INET },
> - { "unix",   PLEDGE_UNIX },
> + { "abort",  0 },/* XXX reserve for later */
> + { "cpath",  PLEDGE_CPATH },
>   { "dns",PLEDGE_DNS },
> + { "exec",   PLEDGE_EXEC },
> + { "fattr",  PLEDGE_FATTR },
> + { "flock",  PLEDGE_FLOCK },
>   { "getpw",  PLEDGE_GETPW },
> - { "sendfd", PLEDGE_SENDFD },
> - { "recvfd", PLEDGE_RECVFD },
> - { "ioctl",  PLEDGE_IOCTL },
>   { "id", PLEDGE_ID },
> - { "route",  PLEDGE_ROUTE },
> + { "inet",   PLEDGE_INET },
> + { "ioctl",  PLEDGE_IOCTL },
>   { "mcast",  PLEDGE_MCAST },
> - { "tty",PLEDGE_TTY },
>   { "proc",   PLEDGE_PROC },
> - { "exec",   PLEDGE_EXEC },
> - { "cpath",  PLEDGE_CPATH },
> - { "fattr",  PLEDGE_FATTR },
>   { "prot_exec",  PLEDGE_PROTEXEC },
> - { "flock",  PLEDGE_FLOCK },
>   { "ps", PLEDGE_PS },
> - { "vminfo", PLEDGE_VMINFO },
> + { "recvfd", PLEDGE_RECVFD },
> + { "route",  PLEDGE_ROUTE },
> + { "rpath",  PLEDGE_RPATH },
> + { "sendfd", PLEDGE_SENDFD },
>   { "settime",PLEDGE_SETTIME },
> - { "abort",  0 },/* XXX reserve for later */
> + { "stdio",  PLEDGE_STDIO },
> + { "tmppath",PLEDGE_TMPPATH },
> + { "tty",PLEDGE_TTY },
> + { "unix",   PLEDGE_UNIX },
> + { "vminfo", PLEDGE_VMINFO },

pledge idea

[Peter J. Philipp]

Hi deraadt,

I know you know I don't code well, but in order to show you what's on my 
mind I had to write code, I took the bsearch() from the ieee80211 code, so
perhaps there is a better way (like always) perhaps to unify the function 
between these two areas.

The reason I did this is to save on cpu cycles when you look at X amount
of processes all pledging...one time'd process won't show much difference.

below's diff:

-peter


? pledge.diff
Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.90
diff -u -p -u -r1.90 kern_pledge.c
--- kern/kern_pledge.c  28 Oct 2015 17:38:52 -  1.90
+++ kern/kern_pledge.c  29 Oct 2015 16:23:04 -
@@ -58,6 +58,32 @@
 
 int canonpath(const char *input, char *buf, size_t bufsize);
 int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
+int pledge_cmp(const void *pi, const void *ph);
+
+/* taken from net80211/ieee80211_regdomain.c */
+const void *pledge_bsearch(const void *, const void *, size_t, size_t,
+int (*)(const void *, const void *));
+
+const void *
+pledge_bsearch(const void *key, const void *base0, size_t nmemb, size_t size,
+int (*compar)(const void *, const void *))
+{
+const char *base = base0;
+int lim, cmp;
+const void *p;
+
+for (lim = nmemb; lim != 0; lim >>= 1) {
+p = base + (lim >> 1) * size;
+cmp = (*compar)(key, p);
+if (cmp == 0)
+return ((const void *)p);
+if (cmp > 0) {  /* key > p: move right */
+base = (const char *)p + size;
+lim--;
+} /* else move left */
+}
+return (NULL);
+}
 
 const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
[SYS_exit] = 0x,
@@ -256,40 +282,42 @@ const u_int pledge_syscalls[SYS_MAXSYSCA
[SYS_swapctl] = PLEDGE_VMINFO,  /* XXX should limit to "get" operations 
*/
 };
 
-static const struct {
+/* MUST be sorted! */
+static const struct PR {
char *name;
int flags;
 } pledgereq[] = {
-   { "stdio",  PLEDGE_STDIO },
-   { "rpath",  PLEDGE_RPATH },
-   { "wpath",  PLEDGE_WPATH },
-   { "tmppath",PLEDGE_TMPPATH },
-   { "inet",   PLEDGE_INET },
-   { "unix",   PLEDGE_UNIX },
+   { "abort",  0 },/* XXX reserve for later */
+   { "cpath",  PLEDGE_CPATH },
{ "dns",PLEDGE_DNS },
+   { "exec",   PLEDGE_EXEC },
+   { "fattr",  PLEDGE_FATTR },
+   { "flock",  PLEDGE_FLOCK },
{ "getpw",  PLEDGE_GETPW },
-   { "sendfd", PLEDGE_SENDFD },
-   { "recvfd", PLEDGE_RECVFD },
-   { "ioctl",  PLEDGE_IOCTL },
{ "id", PLEDGE_ID },
-   { "route",  PLEDGE_ROUTE },
+   { "inet",   PLEDGE_INET },
+   { "ioctl",  PLEDGE_IOCTL },
{ "mcast",  PLEDGE_MCAST },
-   { "tty",PLEDGE_TTY },
{ "proc",   PLEDGE_PROC },
-   { "exec",   PLEDGE_EXEC },
-   { "cpath",  PLEDGE_CPATH },
-   { "fattr",  PLEDGE_FATTR },
{ "prot_exec",  PLEDGE_PROTEXEC },
-   { "flock",  PLEDGE_FLOCK },
{ "ps", PLEDGE_PS },
-   { "vminfo", PLEDGE_VMINFO },
+   { "recvfd", PLEDGE_RECVFD },
+   { "route",  PLEDGE_ROUTE },
+   { "rpath",  PLEDGE_RPATH },
+   { "sendfd", PLEDGE_SENDFD },
{ "settime",PLEDGE_SETTIME },
-   { "abort",  0 },/* XXX reserve for later */
+   { "stdio",  PLEDGE_STDIO },
+   { "tmppath",PLEDGE_TMPPATH },
+   { "tty",PLEDGE_TTY },
+   { "unix",   PLEDGE_UNIX },
+   { "vminfo", PLEDGE_VMINFO },
+   { "wpath",  PLEDGE_WPATH },
 };
 
 int
 sys_pledge(struct proc *p, void *v, register_t *retval)
 {
+   struct PR *pr = NULL;
struct sys_pledge_args /* {
syscallarg(const char *)request;
syscallarg(const char **)paths;
@@ -300,7 +328,7 @@ sys_pledge(struct proc *p, void *v, regi
if (SCARG(uap, request)) {
size_t rbuflen;
char *rbuf, *rp, *pn;
-   int f, i;
+   int f;
 
rbuf = malloc(MAXPATHLEN, M_TEMP, M_WAITOK);
error = copyinstr(SCARG(uap, request), rbuf, MAXPATHLEN,
@@ -321,16 +349,15 @@ sys_pledge(struct proc *p, void *v, regi
*pn++ = '\0';
}
 
-   for (f = i = 0; i < 

Re: calloc -> malloc in get_data() and get_string()

[Michael McConville]

Sebastian Benoit wrote:
> Michael McConville(mm...@mykolab.com) on 2015.10.28 12:05:24 -0400:
> > Relayd, httpd, and ntpd define the functions get_data() and
> > get_string(). Both call calloc and then immediately memcpy. Calloc's
> > zeroing isn't optimized out. These functions are called in network
> > data paths in at least a couple places, so all this extra writing
> > could have a meaningful performance impact.
> 
> in relayd these functions are *not* called in the network path, they
> are used when loading the (TLS) configuration data.
> 
> they dont have a performance impact.

I was speaking of all the daemons together. I figured it'd be easiest to
just change all instances.

Re: pledge idea

[Peter J. Philipp]

On 10/29/15 18:51, Reyk Floeter wrote:
> On Thu, Oct 29, 2015 at 04:32:25PM +, Peter J. Philipp wrote:
>> Hi deraadt,
>>
>> I know you know I don't code well, but in order to show you what's on my 
>> mind I had to write code, I took the bsearch() from the ieee80211 code, so
>> perhaps there is a better way (like always) perhaps to unify the function 
>> between these two areas.
>>
>> The reason I did this is to save on cpu cycles when you look at X amount
>> of processes all pledging...one time'd process won't show much difference.
>>
> I'm not deraadt, but -
>
> smart but have you considered that this is not worth the effort?
> pledge() is only called once or twice during a process' lifetime -
> start, pledge, run - the linear search on such a short list is still
> relatively fast, and the entries are even sorted in the order of
> relevance.  By convention "stdio" always comes first.  So what is the
> impact without your diff of pledge in src/bin/sleep/sleep.c:
>
> if (pledge("stdio", NULL) == -1)
> err(1, "pledge");

Hi Reyk,

deraadt already told me there was a patch for this already.  Yes it
would be more cycles for stdio I see that.

Thanks for your effort in making me see this.

-peter

> $ time obj/sleep 0.01 
> 0m00.01s real 0m00.00s user 0m00.01s system
>
> Are you trying to solve an issue?
>
> Reyk
>
>> below's diff:
>>
>> -peter
>>
>>
>> ? pledge.diff
>> Index: kern/kern_pledge.c
>> ===
>> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
>> retrieving revision 1.90
>> diff -u -p -u -r1.90 kern_pledge.c
>> --- kern/kern_pledge.c   28 Oct 2015 17:38:52 -  1.90
>> +++ kern/kern_pledge.c   29 Oct 2015 16:23:04 -
>> @@ -58,6 +58,32 @@
>>  
>>  int canonpath(const char *input, char *buf, size_t bufsize);
>>  int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
>> +int pledge_cmp(const void *pi, const void *ph);
>> +
>> +/* taken from net80211/ieee80211_regdomain.c */
>> +const void *pledge_bsearch(const void *, const void *, size_t, size_t,
>> +int (*)(const void *, const void *));
>> +
>> +const void *
>> +pledge_bsearch(const void *key, const void *base0, size_t nmemb, size_t 
>> size,
>> +int (*compar)(const void *, const void *))
>> +{
>> +const char *base = base0;
>> +int lim, cmp;
>> +const void *p;
>> +
>> +for (lim = nmemb; lim != 0; lim >>= 1) {
>> +p = base + (lim >> 1) * size;
>> +cmp = (*compar)(key, p);
>> +if (cmp == 0)
>> +return ((const void *)p);
>> +if (cmp > 0) {  /* key > p: move right */
>> +base = (const char *)p + size;
>> +lim--;
>> +} /* else move left */
>> +}
>> +return (NULL);
>> +}
>>  
>>  const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
>>  [SYS_exit] = 0x,
>> @@ -256,40 +282,42 @@ const u_int pledge_syscalls[SYS_MAXSYSCA
>>  [SYS_swapctl] = PLEDGE_VMINFO,  /* XXX should limit to "get" operations 
>> */
>>  };
>>  
>> -static const struct {
>> +/* MUST be sorted! */
>> +static const struct PR {
>>  char *name;
>>  int flags;
>>  } pledgereq[] = {
>> -{ "stdio",  PLEDGE_STDIO },
>> -{ "rpath",  PLEDGE_RPATH },
>> -{ "wpath",  PLEDGE_WPATH },
>> -{ "tmppath",PLEDGE_TMPPATH },
>> -{ "inet",   PLEDGE_INET },
>> -{ "unix",   PLEDGE_UNIX },
>> +{ "abort",  0 },/* XXX reserve for later */
>> +{ "cpath",  PLEDGE_CPATH },
>>  { "dns",PLEDGE_DNS },
>> +{ "exec",   PLEDGE_EXEC },
>> +{ "fattr",  PLEDGE_FATTR },
>> +{ "flock",  PLEDGE_FLOCK },
>>  { "getpw",  PLEDGE_GETPW },
>> -{ "sendfd", PLEDGE_SENDFD },
>> -{ "recvfd", PLEDGE_RECVFD },
>> -{ "ioctl",  PLEDGE_IOCTL },
>>  { "id", PLEDGE_ID },
>> -{ "route",  PLEDGE_ROUTE },
>> +{ "inet",   PLEDGE_INET },
>> +{ "ioctl",  PLEDGE_IOCTL },
>>  { "mcast",  PLEDGE_MCAST },
>> -{ "tty",PLEDGE_TTY },
>>  { "proc",   PLEDGE_PROC },
>> -{ "exec",   PLEDGE_EXEC },
>> -{ "cpath",  PLEDGE_CPATH },
>> -{ "fattr",  PLEDGE_FATTR },
>>  { "prot_exec",  PLEDGE_PROTEXEC },
>> -{ "flock",  PLEDGE_FLOCK },
>>  { "ps", PLEDGE_PS },
>> -{ "vminfo", PLEDGE_VMINFO },
>> +{ "recvfd", PLEDGE_RECVFD },
>> +{ "route",  PLEDGE_ROUTE },
>> +{ "rpath",  PLEDGE_RPATH },
>> +{ "sendfd", PLEDGE_SENDFD },
>>  { "settime",PLEDGE_SETTIME },

Re: __predict_false for pledge

[Michael McConville]

Ted Unangst wrote:
> Michael McConville wrote:
> > Ted Unangst wrote:
> > > Michael McConville wrote:
> > > > We have a pretty strong guarantee that it can only happen once
> > > > per process...
> > > 
> > > I don't think this really matters. What does it do to the
> > > assmembly, and how does that make things faster?
> > 
> > It lets the compiler know that the body is very unlikely to run so
> > that it won't unroll loops, and will maybe bump the condition body
> > to the end of the procedure, etc. It can also be used to annotate
> > the branch with a hint instruction, but I don't know how many
> > architectures still use those.
> 
> I meant in this case specifically. What is the *demonstrated* benefit?
> 
> Generally, not many fans of the annotation for the sake of annontation
> in these parts. :)

So, I broke out objdump and looked at how these differ on amd64. No
profiling yet (I've never profiled syscalls before, advice welcome).

The condition evaluation is identical. The differences:

 * the annotated version jumps on error to the end of the syscall
   procedure, while the current version has the error body directly
   after the condition (in the middle of the procedure's hot path)

 * the annotated version is slightly smaller, assumedly because the
   compiler didn't bother doing expansion-based optimizations in the
   error body.

The current version prevents speculative execution from being useful and
puts unused fluff in the I-cache.

Not super important, but these annotations are easy and pretty
consistently beneficial.

Finally use ksh's areallocarray()

[Michael McConville]

With help from Theo Buehler. ok?


Index: c_sh.c
===
RCS file: /cvs/src/bin/ksh/c_sh.c,v
retrieving revision 1.53
diff -u -p -r1.53 c_sh.c
--- c_sh.c  21 Oct 2015 15:20:37 -  1.53
+++ c_sh.c  29 Oct 2015 18:15:25 -
@@ -617,7 +617,7 @@ c_set(char **wp)
while (*++wp != NULL)
*wp = str_save(*wp, >area);
l->argc = wp - owp - 1;
-   l->argv = alloc(sizeofN(char *, l->argc+2), >area);
+   l->argv = areallocarray(NULL, l->argc+2, sizeof(char *), 
>area);
for (wp = l->argv; (*wp++ = *owp++) != NULL; )
;
}
Index: edit.c
===
RCS file: /cvs/src/bin/ksh/edit.c,v
retrieving revision 1.49
diff -u -p -r1.49 edit.c
--- edit.c  21 Oct 2015 14:31:28 -  1.49
+++ edit.c  29 Oct 2015 18:15:25 -
@@ -475,7 +475,8 @@ x_command_glob(int flags, const char *st
int path_order = 0;
int i;
 
-   info = alloc(sizeof(struct path_order_info) * nwords, ATEMP);
+   info = areallocarray(NULL, nwords,
+   sizeof(struct path_order_info), ATEMP);
 
for (i = 0; i < nwords; i++) {
info[i].word = words[i];
Index: exec.c
===
RCS file: /cvs/src/bin/ksh/exec.c,v
retrieving revision 1.61
diff -u -p -r1.61 exec.c
--- exec.c  19 Oct 2015 14:42:16 -  1.61
+++ exec.c  29 Oct 2015 18:15:25 -
@@ -96,9 +96,9 @@ execute(struct op *volatile t,
flags &= ~XTIME;
 
if (t->ioact != NULL || t->type == TPIPE || t->type == TCOPROC) {
-   e->savefd = alloc(sizeofN(short, NUFILE), ATEMP);
+   e->savefd = areallocarray(NULL, NUFILE, sizeof(short), ATEMP);
/* initialize to not redirected */
-   memset(e->savefd, 0, sizeofN(short, NUFILE));
+   memset(e->savefd, 0, NUFILE * sizeof(short));
}
 
/* do redirection, to be restored in quitenv() */
Index: expand.h
===
RCS file: /cvs/src/bin/ksh/expand.h,v
retrieving revision 1.8
diff -u -p -r1.8 expand.h
--- expand.h17 Sep 2015 14:21:33 -  1.8
+++ expand.h29 Oct 2015 18:15:25 -
@@ -82,7 +82,7 @@ typedef struct XPtrV {
 
 #defineXPinit(x, n) do { \
void **vp__; \
-   vp__ = alloc(sizeofN(void*, n), ATEMP); \
+   vp__ = areallocarray(NULL, n, sizeof(void *), ATEMP); \
(x).cur = (x).beg = vp__; \
(x).end = vp__ + n; \
} while (0)
@@ -90,8 +90,8 @@ typedef struct XPtrV {
 #defineXPput(x, p) do { \
if ((x).cur >= (x).end) { \
int n = XPsize(x); \
-   (x).beg = (void**) aresize((void*) (x).beg, \
-  sizeofN(void*, n*2), ATEMP); 
\
+   (x).beg = (void**) areallocarray((void 
*)(x).beg, \
+  n*2, sizeof(void *), ATEMP); 
\
(x).cur = (x).beg + n; \
(x).end = (x).cur + n; \
} \
@@ -101,7 +101,7 @@ typedef struct XPtrV {
 #defineXPptrv(x)   ((x).beg)
 #defineXPsize(x)   ((x).cur - (x).beg)
 
-#defineXPclose(x)  (void**) aresize((void*)(x).beg, \
-sizeofN(void*, XPsize(x)), ATEMP)
+#defineXPclose(x)  (void**) areallocarray((void *)(x).beg, \
+XPsize(x), sizeof(void *), ATEMP)
 
 #defineXPfree(x)   afree((x).beg, ATEMP)
Index: history.c
===
RCS file: /cvs/src/bin/ksh/history.c,v
retrieving revision 1.51
diff -u -p -r1.51 history.c
--- history.c   21 Oct 2015 15:47:41 -  1.51
+++ history.c   29 Oct 2015 18:15:25 -
@@ -556,7 +556,7 @@ init_histvec(void)
 {
if (history == NULL) {
histsize = HISTORYSIZE;
-   history = alloc(histsize*sizeof (char *), APERM);
+   history = areallocarray(NULL, histsize, sizeof(char *), APERM);
histptr = history - 1;
}
 }
Index: lex.c
===
RCS file: /cvs/src/bin/ksh/lex.c,v
retrieving revision 1.61
diff -u -p -r1.61 lex.c
--- lex.c   19 Oct 2015 14:42:16 -  1.61
+++ lex.c   29 Oct 2015 18:15:26 -
@@ -1657,7 +1657,8 @@ getsc_bn(void)
 static Lex_state *
 push_state_(State_info *si, Lex_state *old_end)
 {
-   Lex_state   *new = alloc(sizeof(Lex_state) * 

Re: [patch] tcpdump - better BGP UPDATE AS_PATH size calculations

[Kevin Reay]

On Tue, Oct 27, 2015 at 12:59:20AM -0600, Kevin Reay wrote:
> I did add an additional check for "zero" ASNs to the 2-byte default,
> inspired by a quick glance at Wireshark's heuristics. I now flip
> through each segment's ASNs inside of bgp_attr_get_as_size(), looking
> for any zero-value 16bit ASNs that would indicate the set isn't valid
> for the assumed byte-length (ie. the first two bytes of a 4-byte ASN).
> I'd need to read through the RFCs in detail to ensure this is witout
> issue though.

I've attached the updated version of the previous tcpdump bgp AS_PATH
size discovery function.

It uses the original 2-byte size assumption with the addition of a
check for any zero-value ASNs. After some research, it appears this is
a valid approach according to RFC7607 (they aren't valid in an
AS_PATH). 

The additional check appears to further improve the success in my test
cases as well as obviously correcting the original in-tree behaviour.

Any feedback would be great.
Index: print-bgp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.18
diff -u -p -r1.18 print-bgp.c
--- print-bgp.c 20 Oct 2015 11:29:07 -  1.18
+++ print-bgp.c 29 Oct 2015 20:48:06 -
@@ -140,6 +140,9 @@ struct bgp_attr {
 #define BGP_CONFED_AS_SEQUENCE 3 /* draft-ietf-idr-rfc3065bis-01 */
 #define BGP_CONFED_AS_SET  4 /* draft-ietf-idr-rfc3065bis-01  */
 
+#define BGP_AS_SEG_TYPE_MINBGP_AS_SET
+#define BGP_AS_SEG_TYPE_MAXBGP_CONFED_AS_SET
+
 static struct tok bgp_as_path_segment_open_values[] = {
{ BGP_AS_SET,   " {" },
{ BGP_AS_SEQUENCE,  " " },
@@ -400,6 +403,64 @@ trunc:
 }
 #endif
 
+/*
+ * Try to determine the size of the ASs encoded in an AS-path. It is
+ * not obvious as both speaker types exchange AS-paths with the same
+ * path-attribute type.
+ */
+static int
+bgp_attr_get_as_size(u_int8_t bgpa_type, const u_char *dat, int len)
+{
+   const u_char *p;
+   int i;
+
+   p = dat;
+
+   /* AS4 path types are always encoded in 4-byte format */
+   if (bgpa_type == BGPTYPE_AS4_PATH)
+   return 4;
+
+   /*
+* Start by assuming 2-byte ASs. Iterate through the path data using the
+* segment length values. Switch to 4-bytes if we encounter an invalid
+* field value/if the AS-Path length is invalid for the assumed size.
+*/
+   while (p < dat + len) {
+   TCHECK(p[0]);
+
+   /* check segment type: invalid value means wrong size */
+   if (p[0] < BGP_AS_SEG_TYPE_MIN || p[0] > BGP_AS_SEG_TYPE_MAX)
+   goto trunc;
+
+   TCHECK2(p[1], 1 + p[1] * 2);
+
+   /* check segment length: invalid indicates wrong size */
+   if (p[1] == 0)
+   goto trunc;
+
+   /*
+* Look for zero-value ASs which are likely the first 2 bytes of
+* a 4-byte AS. RFC7607 asserts a zero in an AS_PATH is invalid.
+*/
+   for (i = 0; i < p[1]; i++) {
+   if (EXTRACT_16BITS([2 + i * 2]) == 0)
+   goto trunc;
+   }
+
+   p += 2 + p[1] * 2;
+   }
+
+   /* matching length: it's very likely the ASs were encoded as 2-bytes */
+   if (p == dat + len)
+   return 2;
+trunc:
+   /*
+* Either there was not enough data or we tried to decode 4-byte ASs
+* with an incorrect size of 2-bytes.
+*/
+   return 4;
+}
+
 static int
 bgp_attr_print(const struct bgp_attr *attr, const u_char *dat, int len)
 {
@@ -425,17 +486,7 @@ bgp_attr_print(const struct bgp_attr *at
}
break;
case BGPTYPE_AS4_PATH:
-   asn_bytes = 4;
-   /* FALLTHROUGH */
case BGPTYPE_AS_PATH:
-   /*
-* 2-byte speakers will receive AS4_PATH as well AS_PATH (2-byte).
-* 4-byte speakers will only receive AS_PATH but it will be 4-byte.
-* To identify which is the case, compare the length of the path
-* segment value in bytes, with the path segment length from the
-* message (counted in # of AS)
-*/
-
if (len % 2) {
printf(" invalid len");
break;
@@ -445,15 +496,13 @@ bgp_attr_print(const struct bgp_attr *at
printf(" empty");
break;
}
+   asn_bytes = bgp_attr_get_as_size(attr->bgpa_type, dat, len);
while (p < dat + len) {
TCHECK2(p[0], 2);
-   if (asn_bytes == 0) {
-   if (p[1] == 0) {
-   /* invalid: segment contains one or more AS */
-   printf(" malformed");
- 

Re: cron: get rid of strcmp_until

[Todd C. Miller]

On Wed, 28 Oct 2015 14:52:21 -0600, "Todd C. Miller" wrote:

> It was only used in env_set().  I've also removed the useless
> FACILITY define and fixed a sizeof().

New diff that converts env_get() into env_find() and uses it similarly
to how __findenv() is used in libc.  It's less code that way.

 - todd

Index: usr.sbin/cron/env.c
===
RCS file: /cvs/src/usr.sbin/cron/env.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 env.c
--- usr.sbin/cron/env.c 9 Feb 2015 22:35:08 -   1.29
+++ usr.sbin/cron/env.c 29 Oct 2015 20:39:27 -
@@ -22,7 +22,7 @@
 char **
 env_init(void)
 {
-   char **p = malloc(sizeof(char **));
+   char **p = malloc(sizeof(char *));
 
if (p != NULL)
p[0] = NULL;
@@ -63,49 +63,65 @@ env_copy(char **envp)
return (p);
 }
 
-char **
-env_set(char **envp, char *envstr)
+static char *
+env_find(char *name, char **envp0, size_t *count)
 {
-   int count, found;
-   char **p, *envtmp;
+   char **envp = envp0;
+   char *p, *q;
+   size_t len;
 
/*
-* count the number of elements, including the null pointer;
-* also set 'found' to -1 or index of entry if already in here.
+* Find name in envp and return its value along with the
+* index it was found at or the length of envp if not found.
+* We treat a '=' in name as end of string for env_set().
 */
-   found = -1;
-   for (count = 0; envp[count] != NULL; count++) {
-   if (!strcmp_until(envp[count], envstr, '='))
-   found = count;
-   }
-   count++;/* for the NULL */
-
-   if (found != -1) {
-   /*
-* it exists already, so just free the existing setting,
-* save our new one there, and return the existing array.
-*/
-   if ((envtmp = strdup(envstr)) == NULL)
-   return (NULL);
-   free(envp[found]);
-   envp[found] = envtmp;
-   return (envp);
+   for (p = name; *p && *p != '='; p++)
+   continue;
+   len = (size_t)(p - name);
+   while ((p = *envp++) != NULL) {
+   if (!(q = strchr(p, '=')))
+   continue;
+   if ((size_t)(q - p) == len && !strncmp(p, name, len)) {
+   p = q + 1;
+   break;
+   }
}
+   *count = (size_t)(envp - envp0);
+   return (p);
+}
 
-   /*
-* it doesn't exist yet, so resize the array, move null pointer over
-* one, save our string over the old null pointer, and return resized
-* array.
-*/
-   if ((envtmp = strdup(envstr)) == NULL)
+char *
+env_get(char *name, char **envp)
+{
+   size_t count;
+
+   return (env_find(name, envp, ));
+}
+
+char **
+env_set(char **envp, char *envstr)
+{
+   size_t count, len;
+   char **p, *envcopy;
+
+   if ((envcopy = strdup(envstr)) == NULL)
return (NULL);
-   p = reallocarray(envp, count+1, sizeof(char **));
+
+   /* Replace existing name if found. */
+   if (env_find(envstr, envp, ) != NULL) {
+   free(envp[count]);
+   envp[count] = envcopy;
+   return (envp);
+   }
+
+   /* Realloc envp and append new variable. */
+   p = reallocarray(envp, count + 2, sizeof(char **));
if (p == NULL) {
-   free(envtmp);
+   free(envcopy);
return (NULL);
}
-   p[count] = p[count-1];
-   p[count-1] = envtmp;
+   p[count++] = envcopy;
+   p[count] = NULL;
return (p);
 }
 
@@ -225,19 +241,4 @@ load_env(char *envstr, FILE *f)
if (snprintf(envstr, MAX_ENVSTR, "%s=%s", name, val) >= MAX_ENVSTR)
return (FALSE);
return (TRUE);
-}
-
-char *
-env_get(char *name, char **envp)
-{
-   int len = strlen(name);
-   char *p, *q;
-
-   while ((p = *envp++) != NULL) {
-   if (!(q = strchr(p, '=')))
-   continue;
-   if ((q - p) == len && !strncmp(p, name, len))
-   return (q+1);
-   }
-   return (NULL);
 }
Index: usr.sbin/cron/funcs.h
===
RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
retrieving revision 1.19
diff -u -p -u -r1.19 funcs.h
--- usr.sbin/cron/funcs.h   6 Oct 2015 14:58:37 -   1.19
+++ usr.sbin/cron/funcs.h   28 Oct 2015 20:09:39 -
@@ -50,7 +50,6 @@ int   job_runqueue(void),
load_env(char *, FILE *),
cron_pclose(FILE *, pid_t),
glue_strings(char *, size_t, const char *, const char *, char),
-   strcmp_until(const char *, const char *, char),
allowed(const char *, const char *, const char *),

cron: use vis() instead of home-grown version for logs

[Todd C. Miller]

Cron has a homegrown vis-like function for logging commands.
We have vis(3) so let's use it instead...

 - todd

Index: do_command.c
===
RCS file: /cvs/src/usr.sbin/cron/do_command.c,v
retrieving revision 1.50
diff -u -p -u -r1.50 do_command.c
--- do_command.c25 Oct 2015 21:30:11 -  1.50
+++ do_command.c29 Oct 2015 21:32:03 -
@@ -18,6 +18,7 @@
  */
 
 #include "cron.h"
+#include "vis.h"
 
 static voidchild_process(entry *, user *);
 
@@ -125,10 +126,11 @@ child_process(entry *e, user *u)
 * PID is part of the log message.
 */
if ((e->flags & DONT_LOG) == 0) {
-   char *x = mkprints((u_char *)e->cmd, strlen(e->cmd));
-
-   log_it(usernm, getpid(), "CMD", x);
-   free(x);
+   char *x;
+   if (stravis(, e->cmd, 0) != -1) {
+   log_it(usernm, getpid(), "CMD", x);
+   free(x);
+   }
}
 
/* that's the last thing we'll log.  close the log files.
Index: funcs.h
===
RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
retrieving revision 1.20
diff -u -p -u -r1.20 funcs.h
--- funcs.h 29 Oct 2015 21:19:09 -  1.20
+++ funcs.h 29 Oct 2015 21:32:03 -
@@ -59,13 +59,10 @@ int strtot(const char *nptr, char **end
 
 char   *env_get(char *, char **),
*arpadate(time_t *),
-   *mkprints(unsigned char *, unsigned int),
*first_word(char *, char *),
**env_init(void),
**env_copy(char **),
**env_set(char **, char *);
-
-void   mkprint(char *, unsigned char *, int);
 
 user   *load_user(int, struct passwd *, const char *),
*find_user(cron_db *, const char *);
Index: misc.c
===
RCS file: /cvs/src/usr.sbin/cron/misc.c,v
retrieving revision 1.62
diff -u -p -u -r1.62 misc.c
--- misc.c  29 Oct 2015 21:19:09 -  1.62
+++ misc.c  29 Oct 2015 21:32:03 -
@@ -307,57 +307,6 @@ first_word(char *s, char *t)
return (rb);
 }
 
-/* warning:
- * heavily ascii-dependent.
- */
-void
-mkprint(dst, src, len)
-   char *dst;
-   unsigned char *src;
-   int len;
-{
-   /*
-* XXX
-* We know this routine can't overflow the dst buffer because mkprints()
-* allocated enough space for the worst case.
-*/
-   while (len-- > 0)
-   {
-   unsigned char ch = *src++;
-
-   if (ch < ' ') { /* control character */
-   *dst++ = '^';
-   *dst++ = ch + '@';
-   } else if (ch < 0177) { /* printable */
-   *dst++ = ch;
-   } else if (ch == 0177) {/* delete/rubout */
-   *dst++ = '^';
-   *dst++ = '?';
-   } else {/* parity character */
-   snprintf(dst, 5, "\\%03o", ch);
-   dst += strlen(dst);
-   }
-   }
-   *dst = '\0';
-}
-
-/* warning:
- * returns a pointer to malloc'd storage, you must call free yourself.
- */
-char *
-mkprints(src, len)
-   unsigned char *src;
-   unsigned int len;
-{
-   char *dst = malloc(len*4 + 1);
-
-   if (dst)
-   mkprint(dst, src, len);
-
-   return (dst);
-}
-
-
 static gid_t save_egid;
 int swap_gids() { save_egid = getegid(); return (setegid(getgid())); }
 int swap_gids_back() { return (setegid(save_egid)); }

Re: Finally use ksh's areallocarray()

[Nicholas Marriott]

Like it, ok nicm


On Thu, Oct 29, 2015 at 02:25:10PM -0400, Michael McConville wrote:
> With help from Theo Buehler. ok?
> 
> 
> Index: c_sh.c
> ===
> RCS file: /cvs/src/bin/ksh/c_sh.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 c_sh.c
> --- c_sh.c21 Oct 2015 15:20:37 -  1.53
> +++ c_sh.c29 Oct 2015 18:15:25 -
> @@ -617,7 +617,7 @@ c_set(char **wp)
>   while (*++wp != NULL)
>   *wp = str_save(*wp, >area);
>   l->argc = wp - owp - 1;
> - l->argv = alloc(sizeofN(char *, l->argc+2), >area);
> + l->argv = areallocarray(NULL, l->argc+2, sizeof(char *), 
> >area);
>   for (wp = l->argv; (*wp++ = *owp++) != NULL; )
>   ;
>   }
> Index: edit.c
> ===
> RCS file: /cvs/src/bin/ksh/edit.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 edit.c
> --- edit.c21 Oct 2015 14:31:28 -  1.49
> +++ edit.c29 Oct 2015 18:15:25 -
> @@ -475,7 +475,8 @@ x_command_glob(int flags, const char *st
>   int path_order = 0;
>   int i;
>  
> - info = alloc(sizeof(struct path_order_info) * nwords, ATEMP);
> + info = areallocarray(NULL, nwords,
> + sizeof(struct path_order_info), ATEMP);
>  
>   for (i = 0; i < nwords; i++) {
>   info[i].word = words[i];
> Index: exec.c
> ===
> RCS file: /cvs/src/bin/ksh/exec.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 exec.c
> --- exec.c19 Oct 2015 14:42:16 -  1.61
> +++ exec.c29 Oct 2015 18:15:25 -
> @@ -96,9 +96,9 @@ execute(struct op *volatile t,
>   flags &= ~XTIME;
>  
>   if (t->ioact != NULL || t->type == TPIPE || t->type == TCOPROC) {
> - e->savefd = alloc(sizeofN(short, NUFILE), ATEMP);
> + e->savefd = areallocarray(NULL, NUFILE, sizeof(short), ATEMP);
>   /* initialize to not redirected */
> - memset(e->savefd, 0, sizeofN(short, NUFILE));
> + memset(e->savefd, 0, NUFILE * sizeof(short));
>   }
>  
>   /* do redirection, to be restored in quitenv() */
> Index: expand.h
> ===
> RCS file: /cvs/src/bin/ksh/expand.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 expand.h
> --- expand.h  17 Sep 2015 14:21:33 -  1.8
> +++ expand.h  29 Oct 2015 18:15:25 -
> @@ -82,7 +82,7 @@ typedef struct XPtrV {
>  
>  #define  XPinit(x, n) do { \
>   void **vp__; \
> - vp__ = alloc(sizeofN(void*, n), ATEMP); \
> + vp__ = areallocarray(NULL, n, sizeof(void *), ATEMP); \
>   (x).cur = (x).beg = vp__; \
>   (x).end = vp__ + n; \
>   } while (0)
> @@ -90,8 +90,8 @@ typedef struct XPtrV {
>  #define  XPput(x, p) do { \
>   if ((x).cur >= (x).end) { \
>   int n = XPsize(x); \
> - (x).beg = (void**) aresize((void*) (x).beg, \
> -sizeofN(void*, n*2), ATEMP); 
> \
> + (x).beg = (void**) areallocarray((void 
> *)(x).beg, \
> +n*2, sizeof(void *), ATEMP); 
> \
>   (x).cur = (x).beg + n; \
>   (x).end = (x).cur + n; \
>   } \
> @@ -101,7 +101,7 @@ typedef struct XPtrV {
>  #define  XPptrv(x)   ((x).beg)
>  #define  XPsize(x)   ((x).cur - (x).beg)
>  
> -#define  XPclose(x)  (void**) aresize((void*)(x).beg, \
> -  sizeofN(void*, XPsize(x)), ATEMP)
> +#define  XPclose(x)  (void**) areallocarray((void *)(x).beg, \
> +  XPsize(x), sizeof(void *), ATEMP)
>  
>  #define  XPfree(x)   afree((x).beg, ATEMP)
> Index: history.c
> ===
> RCS file: /cvs/src/bin/ksh/history.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 history.c
> --- history.c 21 Oct 2015 15:47:41 -  1.51
> +++ history.c 29 Oct 2015 18:15:25 -
> @@ -556,7 +556,7 @@ init_histvec(void)
>  {
>   if (history == NULL) {
>   histsize = HISTORYSIZE;
> - history = alloc(histsize*sizeof (char *), APERM);
> + history = areallocarray(NULL, histsize, sizeof(char *), APERM);
>   histptr = history - 1;
>   }
>  }
> Index: lex.c
> ===
> RCS file: /cvs/src/bin/ksh/lex.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 lex.c
> --- lex.c 19 Oct 2015 14:42:16 -  1.61
> +++ lex.c 

Re: Unable to run 'crontab -e' on recent snapshots

[Todd C. Miller]

I've added "proc" permissions to crontab so it can fork the editor.

 - todd

Re: preparing pfi_kif to MP world

[Alexandr Nedvedicky]

Hello,

On Fri, Oct 16, 2015 at 01:51:31PM +0200, Alexandr Nedvedicky wrote:
> On Fri, Oct 16, 2015 at 01:41:50PM +0200, Mike Belopuhov wrote:
> > On 16 October 2015 at 13:28, Alexandr Nedvedicky
> >  wrote:
> > >
> > > may be it's kind of bike shading...
> > > How about make kifs to stick to convention we see for other objects
> > > such as rulesets/anchors:
> > >
> > > pfi_kif_find()
> > > pfi_kif_find_or_create()
> > >
> > 
> > Personally I don't like "_or_create" style of function naming and
> > I would rather see those renamed to something else
> > 
> 

just blink of idea before I'll fall asleep, so using email to remember it...

we should eventually rename all pf_*_find_or_create() functions to
pf_*_create() and that's it, no change to current behavior.

regards
sasha

Unable to run 'crontab -e' on recent snapshots

[Raf Czlonka]

Hi all,

While running recent snapshots, I am no longer able to edit my crontab:

$ crontab -e
Abort trap

The end of dmesg:

crontab(17903): syscall 2 "proc"

BTW, displaying and installing the crontab, i.e.:

$ crontab -l | ssh machine crontab -

works just fine on both machines.

The same thing happens on i386 and amd64 - I don't have any other
hardware at hand.

kern.version=OpenBSD 5.8-current (GENERIC) #1284: Thu Oct 29 08:38:33 MDT 
2015
dera...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/GENERIC

kern.version=OpenBSD 5.8-current (GENERIC.MP) #1548: Thu Oct 29 09:46:20 
MDT 2015
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP

pledge(2) no doubt :^)

Regards,

Raf

Re: cron: use vis() instead of home-grown version for logs

[Nicholas Marriott]

ok nicm



On Thu, Oct 29, 2015 at 03:35:42PM -0600, Todd C. Miller wrote:
> Cron has a homegrown vis-like function for logging commands.
> We have vis(3) so let's use it instead...
> 
>  - todd
> 
> Index: do_command.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/do_command.c,v
> retrieving revision 1.50
> diff -u -p -u -r1.50 do_command.c
> --- do_command.c  25 Oct 2015 21:30:11 -  1.50
> +++ do_command.c  29 Oct 2015 21:32:03 -
> @@ -18,6 +18,7 @@
>   */
>  
>  #include "cron.h"
> +#include "vis.h"
>  
>  static void  child_process(entry *, user *);
>  
> @@ -125,10 +126,11 @@ child_process(entry *e, user *u)
>* PID is part of the log message.
>*/
>   if ((e->flags & DONT_LOG) == 0) {
> - char *x = mkprints((u_char *)e->cmd, strlen(e->cmd));
> -
> - log_it(usernm, getpid(), "CMD", x);
> - free(x);
> + char *x;
> + if (stravis(, e->cmd, 0) != -1) {
> + log_it(usernm, getpid(), "CMD", x);
> + free(x);
> + }
>   }
>  
>   /* that's the last thing we'll log.  close the log files.
> Index: funcs.h
> ===
> RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
> retrieving revision 1.20
> diff -u -p -u -r1.20 funcs.h
> --- funcs.h   29 Oct 2015 21:19:09 -  1.20
> +++ funcs.h   29 Oct 2015 21:32:03 -
> @@ -59,13 +59,10 @@ int   strtot(const char *nptr, char **end
>  
>  char *env_get(char *, char **),
>   *arpadate(time_t *),
> - *mkprints(unsigned char *, unsigned int),
>   *first_word(char *, char *),
>   **env_init(void),
>   **env_copy(char **),
>   **env_set(char **, char *);
> -
> -void mkprint(char *, unsigned char *, int);
>  
>  user *load_user(int, struct passwd *, const char *),
>   *find_user(cron_db *, const char *);
> Index: misc.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/misc.c,v
> retrieving revision 1.62
> diff -u -p -u -r1.62 misc.c
> --- misc.c29 Oct 2015 21:19:09 -  1.62
> +++ misc.c29 Oct 2015 21:32:03 -
> @@ -307,57 +307,6 @@ first_word(char *s, char *t)
>   return (rb);
>  }
>  
> -/* warning:
> - *   heavily ascii-dependent.
> - */
> -void
> -mkprint(dst, src, len)
> - char *dst;
> - unsigned char *src;
> - int len;
> -{
> - /*
> -  * XXX
> -  * We know this routine can't overflow the dst buffer because mkprints()
> -  * allocated enough space for the worst case.
> -  */
> - while (len-- > 0)
> - {
> - unsigned char ch = *src++;
> -
> - if (ch < ' ') { /* control character */
> - *dst++ = '^';
> - *dst++ = ch + '@';
> - } else if (ch < 0177) { /* printable */
> - *dst++ = ch;
> - } else if (ch == 0177) {/* delete/rubout */
> - *dst++ = '^';
> - *dst++ = '?';
> - } else {/* parity character */
> - snprintf(dst, 5, "\\%03o", ch);
> - dst += strlen(dst);
> - }
> - }
> - *dst = '\0';
> -}
> -
> -/* warning:
> - *   returns a pointer to malloc'd storage, you must call free yourself.
> - */
> -char *
> -mkprints(src, len)
> - unsigned char *src;
> - unsigned int len;
> -{
> - char *dst = malloc(len*4 + 1);
> -
> - if (dst)
> - mkprint(dst, src, len);
> -
> - return (dst);
> -}
> -
> -
>  static gid_t save_egid;
>  int swap_gids() { save_egid = getegid(); return (setegid(getgid())); }
>  int swap_gids_back() { return (setegid(save_egid)); }
> 

Re: Is cvsweb.openbsd.org down?

[Stuart Henderson]

On 2015/10/29 19:23, Rob Pierce wrote:
> It appears to be unavailable at the moment.
> 
> Regards,
> 
> Rob
> 

It's currently working for me.

Re: Is cvsweb.openbsd.org down?

[Raf Czlonka]

On Thu, Oct 29, 2015 at 11:23:29PM GMT, Rob Pierce wrote:

> It appears to be unavailable at the moment.

Hi Rob,

Works just fine here.

Raf

Re: Is cvsweb.openbsd.org down?

[Martin Schröder]

http://www.downforeveryoneorjustme.com/cvsweb.openbsd.org

Is cvsweb.openbsd.org down?

[Rob Pierce]

It appears to be unavailable at the moment.

Regards,

Rob

Re: Is cvsweb.openbsd.org down?

[Rob Pierce]

- Original Message -
> From: "Raf Czlonka" 
> To: "tech" 
> Sent: Thursday, October 29, 2015 7:33:31 PM
> Subject: Re: Is cvsweb.openbsd.org down?

> On Thu, Oct 29, 2015 at 11:23:29PM GMT, Rob Pierce wrote:
> 
>> It appears to be unavailable at the moment.
> 
> Hi Rob,
> 
> Works just fine here.
> 
> Raf

Sorry - I jumped the guy. Looks like a local problem.

Re: Is cvsweb.openbsd.org down?

[Rob Pierce]

- Original Message -
> From: "Rob Pierce" 
> To: "tech" 
> Sent: Thursday, October 29, 2015 7:46:11 PM
> Subject: Re: Is cvsweb.openbsd.org down?

> - Original Message -
>> From: "Raf Czlonka" 
>> To: "tech" 
>> Sent: Thursday, October 29, 2015 7:33:31 PM
>> Subject: Re: Is cvsweb.openbsd.org down?
> 
>> On Thu, Oct 29, 2015 at 11:23:29PM GMT, Rob Pierce wrote:
>> 
>>> It appears to be unavailable at the moment.
>> 
>> Hi Rob,
>> 
>> Works just fine here.
>> 
>> Raf
> 
> Sorry - I jumped the guy. Looks like a local problem.

s/guy/gun

Re: Unable to run 'crontab -e' on recent snapshots

[Raf Czlonka]

On Thu, Oct 29, 2015 at 10:47:04PM GMT, Todd C. Miller wrote:

> I've added "proc" permissions to crontab so it can fork the editor.

That was quick! :^)

Thanks, Todd.

Raf

Re: preparing pfi_kif to MP world

[Alexandr Nedvedicky]

On Thu, Oct 29, 2015 at 02:49:40AM +0100, Mike Belopuhov wrote:
> On 28 October 2015 at 18:41, Alexandr Nedvedicky
>  wrote:
> > Hello Mike,
> >
> > just a quick question:
> >
> > are you going to commit your pfi_kif_find() et. al.?
> > or more work is needed there?
> >
> 
> I need OKs
> 

OK for pf_if.c, I'm still not sure, what's going on in if_pfsync.c


thanks and
regards
sasha

ntpd doesn't work with -sv - syscall 5

[Gregory Edigarov]

Hello

ntpd_flags=-sv

Oct 29 11:19:32 lbld12 /bsd: ntpd(15132): syscall 5
Oct 29 11:19:32 lbld12 ntpd[697]: ntp_dispatch_imsg in ntp engine: pipe 
closed

Oct 29 11:19:32 lbld12 ntpd[10730]: dispatch_imsg in main: pipe closed

Without flags everithing works.

[PATCH] rcs: buf_free/rcsnum_free

[Michael W. Bombardieri]

Hi tech@,

In case it's considered useful...
Submitting patch to shave a few lines from rcs(1) by allowing
buf_free() and rcsnum_free() to ignore NULL pointer.

- Michael


Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 buf.c
--- buf.c   13 Jun 2015 20:15:21 -  1.25
+++ buf.c   29 Oct 2015 09:06:01 -
@@ -138,6 +138,8 @@ out:
 void
 buf_free(BUF *b)
 {
+   if (b == NULL)
+   return;
free(b->cb_buf);
free(b);
 }
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.222
diff -u -p -u -r1.222 ci.c
--- ci.c5 Sep 2015 09:38:23 -   1.222
+++ ci.c29 Oct 2015 09:06:01 -
@@ -369,12 +369,9 @@ checkin_diff_file(struct checkin_params 
 
return (b3);
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
+   buf_free(b1);
+   buf_free(b2);
+   buf_free(b3);
free(path1);
free(path2);
 
Index: diff3.c
===
RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
retrieving revision 1.37
diff -u -p -u -r1.37 diff3.c
--- diff3.c 5 Sep 2015 09:47:08 -   1.37
+++ diff3.c 29 Oct 2015 09:06:01 -
@@ -234,14 +234,10 @@ merge_diff3(char **av, int flags)
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
@@ -354,14 +350,10 @@ rcs_diff3(RCSFILE *rf, char *workfile, R
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
Index: ident.c
===
RCS file: /cvs/src/usr.bin/rcs/ident.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ident.c
--- ident.c 2 Oct 2014 06:23:15 -   1.30
+++ ident.c 29 Oct 2015 09:06:01 -
@@ -156,8 +156,7 @@ ident_line(FILE *fp)
 
found++;
 out:
-   if (bp != NULL)
-   buf_free(bp);
+   buf_free(bp);
 }
 
 __dead void
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 rcsclean.c
--- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
+++ rcsclean.c  29 Oct 2015 09:06:01 -
@@ -209,10 +209,8 @@ rcsclean_file(char *fname, const char *r
rcs_set_mtime(file, rcs_mtime);
 
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
if (file != NULL)
rcs_close(file);
 }
Index: rcsdiff.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
retrieving revision 1.83
diff -u -p -u -r1.83 rcsdiff.c
--- rcsdiff.c   13 Jun 2015 20:15:21 -  1.83
+++ rcsdiff.c   29 Oct 2015 09:06:01 -
@@ -354,10 +354,8 @@ rcsdiff_file(RCSFILE *file, RCSNUM *rev,
 out:
if (fd != -1)
(void)close(fd);
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
free(path1);
free(path2);
 
@@ -431,10 +429,8 @@ rcsdiff_rev(RCSFILE *file, RCSNUM *rev1,
ret = diffreg(path1, path2, NULL, dflags);
 
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
free(path1);
free(path2);
 
Index: rcsmerge.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsmerge.c,v
retrieving revision 1.55
diff -u -p -u -r1.55 rcsmerge.c
--- rcsmerge.c  16 Jan 2015 06:40:11 -  1.55
+++ rcsmerge.c  29 Oct 2015 09:06:01 -
@@ -173,12 +173,8 @@ rcsmerge_main(int argc, char **argv)
 
 out:
rcs_close(file);
-
-   if (rev1 != NULL)
-   rcsnum_free(rev1);
-   if (rev2 != NULL)
-   rcsnum_free(rev2);
-
+   rcsnum_free(rev1);
+   

Re: [PATCH] rcs: buf_free/rcsnum_free

[Nicholas Marriott]

Hi

You missed ci.c:316 and a few in rcs.c and rcsdiff.c



On Thu, Oct 29, 2015 at 04:50:56PM +0800, Michael W. Bombardieri wrote:
> Hi tech@,
> 
> In case it's considered useful...
> Submitting patch to shave a few lines from rcs(1) by allowing
> buf_free() and rcsnum_free() to ignore NULL pointer.
> 
> - Michael
> 
> 
> Index: buf.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/buf.c,v
> retrieving revision 1.25
> diff -u -p -u -r1.25 buf.c
> --- buf.c 13 Jun 2015 20:15:21 -  1.25
> +++ buf.c 29 Oct 2015 09:06:01 -
> @@ -138,6 +138,8 @@ out:
>  void
>  buf_free(BUF *b)
>  {
> + if (b == NULL)
> + return;
>   free(b->cb_buf);
>   free(b);
>  }
> Index: ci.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/ci.c,v
> retrieving revision 1.222
> diff -u -p -u -r1.222 ci.c
> --- ci.c  5 Sep 2015 09:38:23 -   1.222
> +++ ci.c  29 Oct 2015 09:06:01 -
> @@ -369,12 +369,9 @@ checkin_diff_file(struct checkin_params 
>  
>   return (b3);
>  out:
> - if (b1 != NULL)
> - buf_free(b1);
> - if (b2 != NULL)
> - buf_free(b2);
> - if (b3 != NULL)
> - buf_free(b3);
> + buf_free(b1);
> + buf_free(b2);
> + buf_free(b3);
>   free(path1);
>   free(path2);
>  
> Index: diff3.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
> retrieving revision 1.37
> diff -u -p -u -r1.37 diff3.c
> --- diff3.c   5 Sep 2015 09:47:08 -   1.37
> +++ diff3.c   29 Oct 2015 09:06:01 -
> @@ -234,14 +234,10 @@ merge_diff3(char **av, int flags)
>   warnx("warning: overlaps or other problems during merge");
>  
>  out:
> - if (b2 != NULL)
> - buf_free(b2);
> - if (b3 != NULL)
> - buf_free(b3);
> - if (d1 != NULL)
> - buf_free(d1);
> - if (d2 != NULL)
> - buf_free(d2);
> + buf_free(b2);
> + buf_free(b3);
> + buf_free(d1);
> + buf_free(d2);
>  
>   (void)unlink(path1);
>   (void)unlink(path2);
> @@ -354,14 +350,10 @@ rcs_diff3(RCSFILE *rf, char *workfile, R
>   warnx("warning: overlaps or other problems during merge");
>  
>  out:
> - if (b2 != NULL)
> - buf_free(b2);
> - if (b3 != NULL)
> - buf_free(b3);
> - if (d1 != NULL)
> - buf_free(d1);
> - if (d2 != NULL)
> - buf_free(d2);
> + buf_free(b2);
> + buf_free(b3);
> + buf_free(d1);
> + buf_free(d2);
>  
>   (void)unlink(path1);
>   (void)unlink(path2);
> Index: ident.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/ident.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 ident.c
> --- ident.c   2 Oct 2014 06:23:15 -   1.30
> +++ ident.c   29 Oct 2015 09:06:01 -
> @@ -156,8 +156,7 @@ ident_line(FILE *fp)
>  
>   found++;
>  out:
> - if (bp != NULL)
> - buf_free(bp);
> + buf_free(bp);
>  }
>  
>  __dead void
> Index: rcsclean.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
> retrieving revision 1.54
> diff -u -p -u -r1.54 rcsclean.c
> --- rcsclean.c16 Jan 2015 06:40:11 -  1.54
> +++ rcsclean.c29 Oct 2015 09:06:01 -
> @@ -209,10 +209,8 @@ rcsclean_file(char *fname, const char *r
>   rcs_set_mtime(file, rcs_mtime);
>  
>  out:
> - if (b1 != NULL)
> - buf_free(b1);
> - if (b2 != NULL)
> - buf_free(b2);
> + buf_free(b1);
> + buf_free(b2);
>   if (file != NULL)
>   rcs_close(file);
>  }
> Index: rcsdiff.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 rcsdiff.c
> --- rcsdiff.c 13 Jun 2015 20:15:21 -  1.83
> +++ rcsdiff.c 29 Oct 2015 09:06:01 -
> @@ -354,10 +354,8 @@ rcsdiff_file(RCSFILE *file, RCSNUM *rev,
>  out:
>   if (fd != -1)
>   (void)close(fd);
> - if (b1 != NULL)
> - buf_free(b1);
> - if (b2 != NULL)
> - buf_free(b2);
> + buf_free(b1);
> + buf_free(b2);
>   free(path1);
>   free(path2);
>  
> @@ -431,10 +429,8 @@ rcsdiff_rev(RCSFILE *file, RCSNUM *rev1,
>   ret = diffreg(path1, path2, NULL, dflags);
>  
>  out:
> - if (b1 != NULL)
> - buf_free(b1);
> - if (b2 != NULL)
> - buf_free(b2);
> + buf_free(b1);
> + buf_free(b2);
>   free(path1);
>   free(path2);
>  
> Index: rcsmerge.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/rcsmerge.c,v
> retrieving revision 1.55
> diff -u -p -u -r1.55 rcsmerge.c
> --- rcsmerge.c16 Jan 2015 

Re: preparing pfi_kif to MP world

[Martin Pieuchot]

On 29/10/15(Thu) 02:49, Mike Belopuhov wrote:
> On 28 October 2015 at 18:41, Alexandr Nedvedicky
>  wrote:
> > Hello Mike,
> >
> > just a quick question:
> >
> > are you going to commit your pfi_kif_find() et. al.?
> > or more work is needed there?
> >
> 
> I need OKs
> [...]
> >  pfsync_in_clr(caddr_t buf, int len, int count, int flags)
> >  {
> > struct pfsync_clr *clr;
> > -   int i;
> > -
> > struct pf_state *st, *nexts;
> > -   struct pf_state_key *sk, *nextsk;
> > -   struct pf_state_item *si;
> > +   struct pfi_kif *kif = NULL;
> > u_int32_t creatorid;
> > +   int i;
> >
> > for (i = 0; i < count; i++) {
> > clr = (struct pfsync_clr *)buf + len * i;
> > creatorid = clr->creatorid;
> > +   if (strlen(clr->ifname) &&
> > +   (kif = pfi_kif_find(clr->ifname)) == NULL)
> > +   continue;
> > -[...]
> > +   for (st = RB_MIN(pf_state_tree_id, _id); st; st = 
> > nexts) {
> > +   nexts = RB_NEXT(pf_state_tree_id, _id, st);
> > +   if (st->creatorid == creatorid &&
> > +   ((kif && st->kif == kif) || !kif)) {

I find this check confusing.

Since you're continuing when "kif == NULL" can't you change this check
into "st->kif == kif"?

Or is it possible for strlen(clr->ifname) to return 0 in which case you
might end up comparing a different ``kif''?

> > +   SET(st->state_flags, PFSTATE_NOSYNC);
> > +   pf_unlink_state(st);
> > }
> > }
> > }

Re: Wrong return type in uvm(9)

[Daniel Dickman]

On Thu, Oct 29, 2015 at 10:31 PM, Michael McConville  wrote:
> This one confused me for a few minutes. See sys/uvm/uvm_user.c for the
> function definition.
>
> ok?

looks like it's been out of sync since r1.10 in 2002.

ok.

>
>
> Index: share/man/man9/uvm.9
> ===
> RCS file: /cvs/src/share/man/man9/uvm.9,v
> retrieving revision 1.60
> diff -u -p -r1.60 uvm.9
> --- share/man/man9/uvm.915 Jan 2015 21:19:22 -  1.60
> +++ share/man/man9/uvm.930 Oct 2015 02:29:52 -
> @@ -131,7 +131,7 @@ function initialises the swap subsystem.
>  .Fn uvm_map_checkprot "vm_map_t map" "vaddr_t start" "vaddr_t end" 
> "vm_prot_t protection"
>  .Ft int
>  .Fn uvm_map_protect "vm_map_t map" "vaddr_t start" "vaddr_t end" "vm_prot_t 
> new_prot" "boolean_t set_max"
> -.Ft int
> +.Ft void
>  .Fn uvm_deallocate "vm_map_t map" "vaddr_t start" "vsize_t size"
>  .Ft struct vmspace *
>  .Fn uvmspace_alloc "vaddr_t min" "vaddr_t max" "boolean_t pageable" 
> "boolean_t remove_holes"
>

Re: Wrong return type in uvm(9)

[Todd C. Miller]

On Thu, 29 Oct 2015 22:31:33 -0400, Michael McConville wrote:

> This one confused me for a few minutes. See sys/uvm/uvm_user.c for the
> function definition.

OK millert@

 - todd

Re: [PATCH] rcs: buf_free/rcsnum_free

[Michael W. Bombardieri]

On Thu, Oct 29, 2015 at 10:54:09AM +, Nicholas Marriott wrote:
> Hi
> 
> You missed ci.c:316 and a few in rcs.c and rcsdiff.c

Sorry. Here is new diff. Hopefully I haven't missed anything else.



Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 buf.c
--- buf.c   13 Jun 2015 20:15:21 -  1.25
+++ buf.c   30 Oct 2015 01:11:28 -
@@ -138,6 +138,8 @@ out:
 void
 buf_free(BUF *b)
 {
+   if (b == NULL)
+   return;
free(b->cb_buf);
free(b);
 }
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.222
diff -u -p -u -r1.222 ci.c
--- ci.c5 Sep 2015 09:38:23 -   1.222
+++ ci.c30 Oct 2015 01:11:28 -
@@ -313,8 +313,7 @@ checkin_main(int argc, char **argv)
}
 
rcs_close(pb.file);
-   if (rev_str != NULL)
-   rcsnum_free(pb.newrev);
+   rcsnum_free(pb.newrev);
pb.newrev = NULL;
}
 
@@ -369,12 +368,9 @@ checkin_diff_file(struct checkin_params 
 
return (b3);
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
+   buf_free(b1);
+   buf_free(b2);
+   buf_free(b3);
free(path1);
free(path2);
 
Index: diff3.c
===
RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
retrieving revision 1.37
diff -u -p -u -r1.37 diff3.c
--- diff3.c 5 Sep 2015 09:47:08 -   1.37
+++ diff3.c 30 Oct 2015 01:11:28 -
@@ -234,14 +234,10 @@ merge_diff3(char **av, int flags)
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
@@ -354,14 +350,10 @@ rcs_diff3(RCSFILE *rf, char *workfile, R
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
Index: ident.c
===
RCS file: /cvs/src/usr.bin/rcs/ident.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ident.c
--- ident.c 2 Oct 2014 06:23:15 -   1.30
+++ ident.c 30 Oct 2015 01:11:28 -
@@ -156,8 +156,7 @@ ident_line(FILE *fp)
 
found++;
 out:
-   if (bp != NULL)
-   buf_free(bp);
+   buf_free(bp);
 }
 
 __dead void
Index: rcs.c
===
RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
retrieving revision 1.83
diff -u -p -u -r1.83 rcs.c
--- rcs.c   13 Jun 2015 20:15:21 -  1.83
+++ rcs.c   30 Oct 2015 01:11:28 -
@@ -177,10 +177,8 @@ rcs_close(RCSFILE *rfp)
free(rlp);
}
 
-   if (rfp->rf_head != NULL)
-   rcsnum_free(rfp->rf_head);
-   if (rfp->rf_branch != NULL)
-   rcsnum_free(rfp->rf_branch);
+   rcsnum_free(rfp->rf_head);
+   rcsnum_free(rfp->rf_branch);
 
if (rfp->rf_file != NULL)
fclose(rfp->rf_file);
@@ -1406,10 +1404,8 @@ rcs_freedelta(struct rcs_delta *rdp)
 {
struct rcs_branch *rb;
 
-   if (rdp->rd_num != NULL)
-   rcsnum_free(rdp->rd_num);
-   if (rdp->rd_next != NULL)
-   rcsnum_free(rdp->rd_next);
+   rcsnum_free(rdp->rd_num);
+   rcsnum_free(rdp->rd_next);
 
free(rdp->rd_author);
free(rdp->rd_locker);
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 rcsclean.c
--- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
+++ rcsclean.c  30 Oct 2015 01:11:28 -
@@ -209,10 +209,8 @@ rcsclean_file(char *fname, const char *r
rcs_set_mtime(file, rcs_mtime);
 
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
if (file != NULL)
rcs_close(file);
 }
Index: rcsdiff.c
===
RCS file: 

Wrong return type in uvm(9)

[Michael McConville]

This one confused me for a few minutes. See sys/uvm/uvm_user.c for the
function definition.

ok?


Index: share/man/man9/uvm.9
===
RCS file: /cvs/src/share/man/man9/uvm.9,v
retrieving revision 1.60
diff -u -p -r1.60 uvm.9
--- share/man/man9/uvm.915 Jan 2015 21:19:22 -  1.60
+++ share/man/man9/uvm.930 Oct 2015 02:29:52 -
@@ -131,7 +131,7 @@ function initialises the swap subsystem.
 .Fn uvm_map_checkprot "vm_map_t map" "vaddr_t start" "vaddr_t end" "vm_prot_t 
protection"
 .Ft int
 .Fn uvm_map_protect "vm_map_t map" "vaddr_t start" "vaddr_t end" "vm_prot_t 
new_prot" "boolean_t set_max"
-.Ft int
+.Ft void
 .Fn uvm_deallocate "vm_map_t map" "vaddr_t start" "vsize_t size"
 .Ft struct vmspace *
 .Fn uvmspace_alloc "vaddr_t min" "vaddr_t max" "boolean_t pageable" "boolean_t 
remove_holes"