Re: dhclient: use /dev/bpf

2016-05-07 Thread Martin Natano
On Fri, May 06, 2016 at 07:17:50PM +0100, Stuart Henderson wrote:
> 
> Personally, my preferred course of action would be to switch to
> looking for /dev/bpf0 for now (bpf0 only: *not* revert to the old
> loop, that's not necessary), and then switch to /dev/bpf
> immediately after 6.0 is released. The recent few version
> upgrades have been relatively painful, with good reason, but
> this is something where we can easily reduce the pain with
> minimal downside.

I agree, this sounds like a good plan. Diff below implements it.

Ok?


Index: sbin/dhclient/bpf.c
===
RCS file: /cvs/src/sbin/dhclient/bpf.c,v
retrieving revision 1.39
diff -u -p -r1.39 bpf.c
--- sbin/dhclient/bpf.c 3 May 2016 07:47:26 -   1.39
+++ sbin/dhclient/bpf.c 7 May 2016 08:04:03 -
@@ -77,13 +77,13 @@ if_register_bpf(void)
struct ifreq ifr;
int sock;
 
-   if ((sock = open("/dev/bpf", O_RDWR | O_CLOEXEC)) == -1)
+   if ((sock = open("/dev/bpf0", O_RDWR | O_CLOEXEC)) == -1)
error("Can't open bpf: %s", strerror(errno));
 
/* Set the BPF device to point at this interface. */
strlcpy(ifr.ifr_name, ifi->name, IFNAMSIZ);
if (ioctl(sock, BIOCSETIF, &ifr) < 0)
-   error("Can't attach interface %s to /dev/bpf: %s",
+   error("Can't attach interface %s to /dev/bpf0: %s",
ifi->name, strerror(errno));
 
return (sock);
Index: lib/libpcap/pcap-bpf.c
===
RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v
retrieving revision 1.33
diff -u -p -r1.33 pcap-bpf.c
--- lib/libpcap/pcap-bpf.c  3 May 2016 07:38:38 -   1.33
+++ lib/libpcap/pcap-bpf.c  7 May 2016 08:04:04 -
@@ -216,9 +216,9 @@ bpf_open(pcap_t *p)
 {
int fd;
 
-   fd = open("/dev/bpf", O_RDWR);
+   fd = open("/dev/bpf0", O_RDWR);
if (fd == -1 && errno == EACCES)
-   fd = open("/dev/bpf", O_RDONLY);
+   fd = open("/dev/bpf0", O_RDONLY);
 
if (fd == -1) {
if (errno == EACCES)
Index: usr.sbin/tcpdump/privsep_pcap.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep_pcap.c,v
retrieving revision 1.20
diff -u -p -r1.20 privsep_pcap.c
--- usr.sbin/tcpdump/privsep_pcap.c 3 May 2016 07:41:24 -   1.20
+++ usr.sbin/tcpdump/privsep_pcap.c 7 May 2016 08:04:04 -
@@ -182,7 +182,7 @@ pcap_live(const char *device, int snaple
if (device == NULL || snaplen <= 0)
return (-1);
 
-   if ((fd = open("/dev/bpf", O_RDONLY)) == -1)
+   if ((fd = open("/dev/bpf0", O_RDONLY)) == -1)
return (-1);
 
v = 32768;  /* XXX this should be a user-accessible hook */
Index: usr.sbin/tcpdump/tcpdump.8
===
RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
retrieving revision 1.90
diff -u -p -r1.90 tcpdump.8
--- usr.sbin/tcpdump/tcpdump.8  3 May 2016 07:41:24 -   1.90
+++ usr.sbin/tcpdump/tcpdump.8  7 May 2016 08:04:04 -
@@ -44,7 +44,7 @@
 prints out the headers of packets on a network interface that match the boolean
 .Ar expression .
 You must have read access to
-.Pa /dev/bpf .
+.Pa /dev/bpf0 .
 .Pp
 The options are as follows:
 .Bl -tag -width "-c count"



Re: rtadvd.conf.5: document route preference flags

2016-05-07 Thread Martin Pieuchot
On 06/05/16(Fri) 21:42, Jeremie Courreges-Anglas wrote:
> 
> Also motivated by http://marc.info/?l=openbsd-misc&m=146239072929264&w=2
> 
> RFC4191 defines route preference flags not only in Route Information
> options, but also in Router Advertisement messages.  Let's try to make
> this clear.  Also, try to document the possible values in a slightly
> more useful way...
> 
> Comments / oks?

I wish this config file could be sane and use keywords just like our
other deamons.  I use rtadvd for debugging purpose and I find its syntax
horrible.

That said I'm ok with your diff.

> Index: rtadvd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/rtadvd/rtadvd.conf.5,v
> retrieving revision 1.35
> diff -u -p -p -u -r1.35 rtadvd.conf.5
> --- rtadvd.conf.5 21 Apr 2015 16:32:24 -  1.35
> +++ rtadvd.conf.5 6 May 2016 19:25:15 -
> @@ -116,6 +116,22 @@ and Bit 6
>  .Li 0x40
>  .Pc
>  means Other stateful configuration flag bit.
> +Bit 4
> +.Po
> +.Li 0x10
> +.Pc
> +and
> +Bit 3
> +.Po
> +.Li 0x08
> +.Pc
> +are used to encode the route preference for the route as follows:
> +.Bl -column "0x08" "High" -offset indent
> +.It 0x08 Ta "High"
> +.It 0x00 Ta "Medium" Pq default
> +.It 0x18 Ta "Low"
> +.El
> +.Pp
>  The default value is 0.
>  .It Cm \&rltime
>  (num) Router lifetime field
> @@ -252,18 +268,7 @@ The default value is 64.
>  .It Cm \&rtflags
>  (str or num) A 8-bit flags field in route information option.
>  Currently only the preference values are defined.
> -The notation is same as that of the raflags field.
> -Bit 4
> -.Po
> -.Li 0x10
> -.Pc
> -and
> -Bit 3
> -.Po
> -.Li 0x08
> -.Pc
> -are used to encode the route preference for the route.
> -The default value is 0x00, i.e., medium preference.
> +The notation for those is same as that of the raflags field.
>  .It Cm \&rtltime
>  (num) route lifetime field in route information option.
>  .Pq unit: seconds .
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 



Re: ip6_input: Double check for mapped v4 addresses

2016-05-07 Thread Martin Pieuchot
On 06/05/16(Fri) 14:22, Stuart Henderson wrote:
> On 2016/05/06 15:05, Martin Pieuchot wrote:
> > This look like a bad merge from 2000s.  This check is present twice in
> > ip6_input(), so let's remove the late one, ok?
> > 
> > Index: netinet6/ip6_input.c
> > ===
> > RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
> > retrieving revision 1.158
> > diff -u -p -r1.158 ip6_input.c
> > --- netinet6/ip6_input.c19 Apr 2016 08:23:13 -  1.158
> > +++ netinet6/ip6_input.c6 May 2016 13:01:34 -
> > @@ -600,21 +600,6 @@ ip6_input(struct mbuf *m)
> > ip6 = mtod(m, struct ip6_hdr *);
> 
> OK, but I believe you can also remove this mtod.

Thanks for checking!

I left it for the moment.  I know this has been introduce in the same
commit but I believe some code executed before rely on this.

I have *a lot* of upcoming refactoring for ip6_input() to be able to
only grab the KERNEL_LOCK for locally delivered packets so this will be
addressed along the way.

> > /*
> > -* Malicious party may be able to use IPv4 mapped addr to confuse
> > -* tcp/udp stack and bypass security checks (act as if it was from
> > -* 127.0.0.1 by using IPv6 src :::127.0.0.1).  Be cautious.
> > -*
> > -* For SIIT end node behavior, you may want to disable the check.
> > -* However, you will  become vulnerable to attacks using IPv4 mapped
> > -* source.
> > -*/
> > -   if (IN6_IS_ADDR_V4MAPPED(&ip6->ip6_src) ||
> > -   IN6_IS_ADDR_V4MAPPED(&ip6->ip6_dst)) {
> > -   ip6stat.ip6s_badscope++;
> > -   goto bad;
> > -   }
> > -
> > -   /*
> >  * Tell launch routine the next header
> >  */
> > ip6stat.ip6s_delivered++;
> > 
> 



Re: xclock patch

2016-05-07 Thread Edgar Pettijohn


Sent from my iPhone

> On May 6, 2016, at 11:49 PM, Sebastien Marie  wrote:
> 
> On Fri, May 06, 2016 at 01:29:11PM -0500, Edgar Pettijohn wrote:
>>> On May 6, 2016, at 12:13 PM, Matthieu Herrb  wrote:
>>> 
 On Tue, May 03, 2016 at 11:40:31AM -0500, Edgar Pettijohn wrote:
 I'll look at it this evening.
>>> 
>>> Ping...
>> 
>> Sorry. I tried to get it to reproduce and couldn't. Looked through the code 
>> and couldn't find anything.
> 
> if you still have the xclock.core file (and the same xclock binary), you
> could get a backtrace using gdb.
> 

Unfortunately I don't.
> 
> else, if it isn't in xclock code, it could be a open(2) call in some
> X11R6 library.
> 
I looked all I found was access() which if I'm reading pledge correctly it 
doesn't need pledge. Is that right?

> One possibility is error code path: in xterm, the commit message for
> justifying "rpath" is: for X11 error ("X Error of failed request: ...")
> which read at least /usr/X11R6/share/X11/XErrorDB.
> 
> Maybe we should identify correctly these open(2) call in X11R6 libraries ?
> -- 
> Sebastien Marie



Re: gif tunnel and IPv6 ND

2016-05-07 Thread Martin Pieuchot
On 27/04/16(Wed) 18:13, Martin Pieuchot wrote:
> gif(4) is the only p2p interface for which the kernel does some kind of
> link-layer address resolution when it comes to IPv6 & ND.
> 
> I don't believe this is necessary because we do not install any cloning
> route on p2p interfaces.  However the rt_checkgate() call *is* necessary
> because your default IPv6 route, or any gateway route, might go through
> your tunnel.
> 
> So the diff below removes gif(4) interfaces from the list of interfaces
> that need a link-layer cache and move the check *after* calling
> rt_checkgate().  This way all the p2p-specific code in nd6_output()
> can go away.
> 
> I'd like to hear from people using such setup to know if this break
> anything.

Here's a non broken diff, thanks to naddy@ for finding that.

Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.178
diff -u -p -r1.178 nd6.c
--- netinet6/nd6.c  27 Apr 2016 14:47:27 -  1.178
+++ netinet6/nd6.c  7 May 2016 11:49:28 -
@@ -1512,9 +1512,6 @@ nd6_output(struct ifnet *ifp, struct mbu
if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr))
goto sendpkt;
 
-   if (nd6_need_cache(ifp) == 0)
-   goto sendpkt;
-
/*
 * next hop determination.
 */
@@ -1524,21 +1521,11 @@ nd6_output(struct ifnet *ifp, struct mbu
m_freem(m);
return (error);
}
-
-   /*
-* We skip link-layer address resolution and NUD
-* if the gateway is not a neighbor from ND point
-* of view, regardless of the value of nd_ifinfo.flags.
-* The second condition is a bit tricky; we skip
-* if the gateway is our own address, which is
-* sometimes used to install a route to a p2p link.
-*/
-   if ((ifp->if_flags & IFF_POINTOPOINT) &&
-   ((nd6_is_addr_neighbor(satosin6(rt_key(rt)), ifp) == 0) ||
-   in6ifa_ifpwithaddr(ifp, &satosin6(rt_key(rt))->sin6_addr)))
-   goto sendpkt;
}
 
+   if (nd6_need_cache(ifp) == 0)
+   goto sendpkt;
+
/*
 * Address resolution or Neighbor Unreachability Detection
 * for the next hop.
@@ -1565,8 +1552,7 @@ nd6_output(struct ifnet *ifp, struct mbu
}
}
if (ln == NULL || rt == NULL) {
-   if ((ifp->if_flags & IFF_POINTOPOINT) == 0 &&
-   !(ND_IFINFO(ifp)->flags & ND6_IFF_PERFORMNUD)) {
+   if ((ND_IFINFO(ifp)->flags & ND6_IFF_PERFORMNUD) == 0) {
char addr[INET6_ADDRSTRLEN];
 
log(LOG_DEBUG, "%s: can't allocate llinfo for %s "
@@ -1591,13 +1577,6 @@ nd6_output(struct ifnet *ifp, struct mbu
TAILQ_REMOVE(&nd6_list, ln, ln_list);
TAILQ_INSERT_HEAD(&nd6_list, ln, ln_list);
 
-   /* We don't have to do link-layer address resolution on a p2p link. */
-   if ((ifp->if_flags & IFF_POINTOPOINT) != 0 &&
-   ln->ln_state < ND6_LLINFO_REACHABLE) {
-   ln->ln_state = ND6_LLINFO_STALE;
-   nd6_llinfo_settimer(ln, (long)nd6_gctimer * hz);
-   }
-
/*
 * The first time we send a packet to a neighbor whose entry is
 * STALE, we have to change the state to DELAY and a sets a timer to
@@ -1658,11 +1637,8 @@ nd6_need_cache(struct ifnet *ifp)
 */
switch (ifp->if_type) {
case IFT_ETHER:
-   case IFT_IEEE1394:
-   case IFT_PROPVIRTUAL:
case IFT_IEEE80211:
case IFT_CARP:
-   case IFT_GIF:   /* XXX need more cases? */
return (1);
default:
return (0);



Re: xclock patch

2016-05-07 Thread Sebastien Marie
On Sat, May 07, 2016 at 06:48:58AM -0500, Edgar Pettijohn wrote:
> > 
> > else, if it isn't in xclock code, it could be a open(2) call in some
> > X11R6 library.
> > 
> I looked all I found was access() which if I'm reading pledge correctly it 
> doesn't need pledge. Is that right?
> 

What I mean is the error code path in libX11 (for example).

After checking the code source:
  - the default error handler (for not fatal) error is _XDefaultError 
(xenocara/lib/libX11/src/XlibInt.c:1375)
  - it calls _XPrintDefaultError (xenocara/lib/libX11/src/XlibInt.c:1263)
  - it calls XGetErrorDatabaseText (xenocara/lib/libX11/src/ErrDes.c:139)
  - it calls XrmGetFileDatabase (xenocara/lib/libX11/src/Xrm.c:1669)
  - it calls ReadInFile (xenocara/lib/libX11/src/Xrm.c:1573)
  - it calls _XOpenFile (xenocara/lib/libX11/src/XlibInt.c:1830)
  - ... and _XOpenFile will do access(2) and open(2) on filename, and
kernel will kill it due to pledge.

I am still unsure if adding "rpath" is the good way to deal with that.

But the problem you saw could be related to some X11 error that trigger
a access(2) or a open(2) for showing error message.
-- 
Sebastien Marie



Re: xclock patch

2016-05-07 Thread Sebastien Marie
On Sat, May 07, 2016 at 06:48:58AM -0500, Edgar Pettijohn wrote:
> > 
> > else, if it isn't in xclock code, it could be a open(2) call in some
> > X11R6 library.
> > 
> I looked all I found was access() which if I'm reading pledge correctly it 
> doesn't need pledge. Is that right?
> 

now, I am unsure about correctly understood your message :)

access(2) requires "rpath" too. But some paths could be whitelisted
(only "/etc/localtime" and "/var/run/ypbind.lock").
-- 
Sebastien Marie



[patch] memory leak in vi

2016-05-07 Thread Martijn van Duren
Hello tech@,

There's a memory leak in vi in the REALLOC{,ARRAY} macro.
This patch should fix the leak. A similar diff is already
present in FreeBSD's vi.

OK?

martijn@

Index: common/mem.h
===
RCS file: /cvs/src/usr.bin/vi/common/mem.h,v
retrieving revision 1.8
diff -u -p -r1.8 mem.h
--- common/mem.h3 Feb 2016 01:47:25 -   1.8
+++ common/mem.h7 May 2016 13:07:43 -
@@ -143,13 +143,21 @@
 }
 
 #defineREALLOC(sp, p, size) {  
\
-   if (((p) = (realloc((p), (size == NULL) \
+   void *tmpp; \
+   if (((tmpp) = (realloc((p), (size == NULL) {\
msgq((sp), M_SYSERR, NULL); \
+   free(p);\
+   }   \
+   p = tmpp;   \
 }
 
 #defineREALLOCARRAY(sp, p, nelem, size) {  
\
-   if (((p) = (reallocarray((p), (nelem), (size == NULL)   \
+   void *tmpp; \
+   if (((tmpp) = (reallocarray((p), (nelem), (size == NULL) {  \
msgq((sp), M_SYSERR, NULL); \
+   free(p);\
+   }   \
+   p = tmpp;   \
 }
 
 /*



Re: xclock patch

2016-05-07 Thread Theo de Raadt
> On Sat, May 07, 2016 at 06:48:58AM -0500, Edgar Pettijohn wrote:
> > > 
> > > else, if it isn't in xclock code, it could be a open(2) call in some
> > > X11R6 library.
> > > 
> > I looked all I found was access() which if I'm reading pledge correctly it 
> > doesn't need pledge. Is that right?
> > 
> 
> What I mean is the error code path in libX11 (for example).
> 
> After checking the code source:
>   - the default error handler (for not fatal) error is _XDefaultError 
> (xenocara/lib/libX11/src/XlibInt.c:1375)
>   - it calls _XPrintDefaultError (xenocara/lib/libX11/src/XlibInt.c:1263)
>   - it calls XGetErrorDatabaseText (xenocara/lib/libX11/src/ErrDes.c:139)
>   - it calls XrmGetFileDatabase (xenocara/lib/libX11/src/Xrm.c:1669)
>   - it calls ReadInFile (xenocara/lib/libX11/src/Xrm.c:1573)
>   - it calls _XOpenFile (xenocara/lib/libX11/src/XlibInt.c:1830)
>   - ... and _XOpenFile will do access(2) and open(2) on filename, and
> kernel will kill it due to pledge.
> 
> I am still unsure if adding "rpath" is the good way to deal with that.
> 
> But the problem you saw could be related to some X11 error that trigger
> a access(2) or a open(2) for showing error message.

So, /usr/X11R6/share/X11/XErrorDB is the file in question?  That is
being parsed very late.  That is quite an outdated model... like, when
something has goes wrong, let's open a file and parse lots of strings!
Alas, that is the story of X.

Sigh.

However I've said it before, and I'll say it again.  pledge should
only be used in commands when the command is fully understood top to
bottom, and fully tested as well.  This command uses a library that
was not studied.  The result was not fully tested.  A bit of
optimism-driven-development is fine, but when the complexity is high
it cannot be the only foundation.

I am not really surprised to see the resulting failure.

The result is that xclock is broken in 5.9.  If a program does not
fully perform the actions it previously promised to do, then the
addition of pledge BROKE IT.

So the next optimism-driven development will be "add rpath".  Without
a full study of the large libraries it links against.  After rpath is
added, I expect more breakage in xclock will be found, because there
will be something else in the libraries.

I don't believe that xclock callpath & reach into libX11 has been
audited & verified to operate only using the POSIX-subset provided by
pledge "stdio".  Nor, do I believe the same for "stdio rpath".  It is
a lot of code, I just cannot be sure.

So I don't understand why pledge is being added here.  The addition of
pledge is breaking xclock.  A pledge addition should not break a
program (coredump) in any circumstance, it should only serve to secure
it in conditions of true & undetected misbehaviour.  Sure, pledge can
make programs safer, but only if they also keep working!!  Otherwise
this is very much throwing the baby out with the bathwater.



ifconfig(8): Do not print MPSAFE

2016-05-07 Thread Martin Pieuchot
IFXF_MPSAFE is a hint for the network stack.  When it is set, the kernel
knows that it doesn't need to grab the KERNEL_LOCK() to call the start()
routine of an interface.

Exposing this flag to users in ifconfig(8) does not make sense.  This is
a side effect of having re-used the IFXF_TXREADY bit.

I don't even think we should export this bit to userland, developers
should not write different code for MPSAFE and !MPSAFE interfaces. 

ok?

Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.430
diff -u -p -r1.430 if.c
--- sys/net/if.c3 May 2016 14:52:39 -   1.430
+++ sys/net/if.c7 May 2016 14:45:11 -
@@ -1668,7 +1668,7 @@ ifioctl(struct socket *so, u_long cmd, c
break;
 
case SIOCGIFXFLAGS:
-   ifr->ifr_flags = ifp->if_xflags;
+   ifr->ifr_flags = ifp->if_xflags & ~IFXF_MPSAFE;
break;
 
case SIOCGIFMETRIC:
Index: sbin/ifconfig/brconfig.h
===
RCS file: /cvs/src/sbin/ifconfig/brconfig.h,v
retrieving revision 1.9
diff -u -p -r1.9 brconfig.h
--- sbin/ifconfig/brconfig.h7 Jan 2016 15:33:56 -   1.9
+++ sbin/ifconfig/brconfig.h7 May 2016 14:35:29 -
@@ -68,7 +68,7 @@ int bridge_rule(int, char **, int);
 #defineIFFBITS 
\
"\024\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6NOTRAILERS" \
"\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\
-   "\15LINK0\16LINK1\17LINK2\20MULTICAST\21MPSAFE" \
+   "\15LINK0\16LINK1\17LINK2\20MULTICAST"  \
"\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6"
 
 void printb(char *, unsigned int, unsigned char *);



Re: ifconfig(8): Do not print MPSAFE

2016-05-07 Thread Mark Kettenis
> Date: Sat, 7 May 2016 16:56:02 +0200
> From: Martin Pieuchot 
> 
> IFXF_MPSAFE is a hint for the network stack.  When it is set, the kernel
> knows that it doesn't need to grab the KERNEL_LOCK() to call the start()
> routine of an interface.
> 
> Exposing this flag to users in ifconfig(8) does not make sense.  This is
> a side effect of having re-used the IFXF_TXREADY bit.
> 
> I don't even think we should export this bit to userland, developers
> should not write different code for MPSAFE and !MPSAFE interfaces. 
> 
> ok?

ok kettenis@

> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.430
> diff -u -p -r1.430 if.c
> --- sys/net/if.c  3 May 2016 14:52:39 -   1.430
> +++ sys/net/if.c  7 May 2016 14:45:11 -
> @@ -1668,7 +1668,7 @@ ifioctl(struct socket *so, u_long cmd, c
>   break;
>  
>   case SIOCGIFXFLAGS:
> - ifr->ifr_flags = ifp->if_xflags;
> + ifr->ifr_flags = ifp->if_xflags & ~IFXF_MPSAFE;
>   break;
>  
>   case SIOCGIFMETRIC:
> Index: sbin/ifconfig/brconfig.h
> ===
> RCS file: /cvs/src/sbin/ifconfig/brconfig.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 brconfig.h
> --- sbin/ifconfig/brconfig.h  7 Jan 2016 15:33:56 -   1.9
> +++ sbin/ifconfig/brconfig.h  7 May 2016 14:35:29 -
> @@ -68,7 +68,7 @@ int bridge_rule(int, char **, int);
>  #define  IFFBITS 
> \
>   "\024\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6NOTRAILERS" \
>   "\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\
> - "\15LINK0\16LINK1\17LINK2\20MULTICAST\21MPSAFE" \
> + "\15LINK0\16LINK1\17LINK2\20MULTICAST"  \
>   "\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6"
>  
>  void printb(char *, unsigned int, unsigned char *);
> 
> 



Re: ifconfig(8): Do not print MPSAFE

2016-05-07 Thread Peter Hessler
SINCE THIS IS UNDER HEAVY DEVELOPMENT: as a user, I want to know if my
interface is marked as MP safe or not.

I don't care if this is done in dmesg, or in ifconfig.

once it stabilizes, I'm happy for the notice to go away.


On 2016 May 07 (Sat) at 16:56:02 +0200 (+0200), Martin Pieuchot wrote:
:IFXF_MPSAFE is a hint for the network stack.  When it is set, the kernel
:knows that it doesn't need to grab the KERNEL_LOCK() to call the start()
:routine of an interface.
:
:Exposing this flag to users in ifconfig(8) does not make sense.  This is
:a side effect of having re-used the IFXF_TXREADY bit.
:
:I don't even think we should export this bit to userland, developers
:should not write different code for MPSAFE and !MPSAFE interfaces. 
:
:ok?
:
:Index: sys/net/if.c
:===
:RCS file: /cvs/src/sys/net/if.c,v
:retrieving revision 1.430
:diff -u -p -r1.430 if.c
:--- sys/net/if.c   3 May 2016 14:52:39 -   1.430
:+++ sys/net/if.c   7 May 2016 14:45:11 -
:@@ -1668,7 +1668,7 @@ ifioctl(struct socket *so, u_long cmd, c
:   break;
: 
:   case SIOCGIFXFLAGS:
:-  ifr->ifr_flags = ifp->if_xflags;
:+  ifr->ifr_flags = ifp->if_xflags & ~IFXF_MPSAFE;
:   break;
: 
:   case SIOCGIFMETRIC:
:Index: sbin/ifconfig/brconfig.h
:===
:RCS file: /cvs/src/sbin/ifconfig/brconfig.h,v
:retrieving revision 1.9
:diff -u -p -r1.9 brconfig.h
:--- sbin/ifconfig/brconfig.h   7 Jan 2016 15:33:56 -   1.9
:+++ sbin/ifconfig/brconfig.h   7 May 2016 14:35:29 -
:@@ -68,7 +68,7 @@ int bridge_rule(int, char **, int);
: #define   IFFBITS 
\
:   "\024\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6NOTRAILERS" \
:   "\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\
:-  "\15LINK0\16LINK1\17LINK2\20MULTICAST\21MPSAFE" \
:+  "\15LINK0\16LINK1\17LINK2\20MULTICAST"  \
:   "\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6"
: 
: void printb(char *, unsigned int, unsigned char *);
:

-- 
Incumbent, n.:
Person of liveliest interest to the outcumbents.
-- Ambrose Bierce, "The Devil's Dictionary"



Re: ifconfig(8): Do not print MPSAFE

2016-05-07 Thread Theo de Raadt
> SINCE THIS IS UNDER HEAVY DEVELOPMENT: as a user, I want to know if my
> interface is marked as MP safe or not.
> 
> I don't care if this is done in dmesg, or in ifconfig.
> 
> once it stabilizes, I'm happy for the notice to go away.

Disagree strongly.

That mindset would drive us towards putting visible markers all over
the user interface, and drive usage patterns differently from
development directions.

And you already know what drivers are marked like that.  It is the
ones you are using.  Assume all of them.



Re: ifconfig(8): Do not print MPSAFE

2016-05-07 Thread Mark Kettenis
> Date: Sat, 7 May 2016 18:44:34 +0200
> From: Peter Hessler 
> 
> SINCE THIS IS UNDER HEAVY DEVELOPMENT: as a user, I want to know if my
> interface is marked as MP safe or not.
> 
> I don't care if this is done in dmesg, or in ifconfig.
> 
> once it stabilizes, I'm happy for the notice to go away.

It does not mean what you think it means.



Re: xclock patch

2016-05-07 Thread Edgar Pettijohn


Sent from my iPhone

> On May 7, 2016, at 8:03 AM, Sebastien Marie  wrote:
> 
> On Sat, May 07, 2016 at 06:48:58AM -0500, Edgar Pettijohn wrote:
>>> 
>>> else, if it isn't in xclock code, it could be a open(2) call in some
>>> X11R6 library.
>> I looked all I found was access() which if I'm reading pledge correctly it 
>> doesn't need pledge. Is that right?
> 
> now, I am unsure about correctly understood your message :)
> 
I understand the manual now. When I saw the word "whitelist" it made me think 
access() was white listed but it's only certain files white listed.


> access(2) requires "rpath" too. But some paths could be whitelisted
> (only "/etc/localtime" and "/var/run/ypbind.lock").
> -- 
> Sebastien Marie
> 



Re: ld patch that greatly speeds up linking large programs with debug symbols

2016-05-07 Thread Stefan Kempf
Aaron Miller wrote:
> Hi All,
> 
> I was experiencing ~8 minute linking times for a large C++ application
> I have been working on when running -current on amd64. It turns out
> that the decade-old version of ld in the OpenBSD source tree has a bug
> that causes quadratic complexity for some linking operations when
> debug symbols are present. I found a patch from 2006 in a mailing list
> archive that fixes this; I adapted it to work with OpenBSD by editing
> files that are normally regenerated (I couldn't figure out how to do
> automatically regenerate them).
> 
> Here is the original patch:
> https://sourceware.org/ml/binutils/2006-08/msg00334.html
> 
> I have rebuilt the kernel and system with this ld and everything runs
> identically to the system linked with the unpatched ld.
> 
> Please test this and let me know if this could get into the tree, and
> if not, what changes I need to make. Thanks!

Interesting!

About the autogenerated files: it looks you already made the additions
to bfd/linker.c, bfd/targets.c and bfd/libbfd.h that should end up
in bfd-in2.h and libbfd-in.h.

To regenerate them, make sure you first did a "make obj" in the root of
your source tree. Then run "make depend" from gnu/usr.bin. Afterwards,
to to gnu/usr.bin/binutils-2.17/obj/bfd and run "make headers".
headers". That did the trick for me.

PS: It appears that gmail mangled your diff. It does not apply cleanly
here. You probably need to use a different mail client or try sending the
diff as a plain text attachment. Can you please resend an un-mangled
diff?
 
> Index: bfd/bfd-in2.h
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/bfd/bfd-in2.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 bfd-in2.h
> --- bfd/bfd-in2.h25 May 2015 14:56:26 -1.4
> +++ bfd/bfd-in2.h17 Apr 2016 05:14:15 -
> @@ -5068,7 +5068,7 @@ typedef struct bfd_target
> 
>/* Check if SEC has been already linked during a reloceatable or
>   final link.  */
> -  void (*_section_already_linked) (bfd *, struct bfd_section *);
> +  void (*_section_already_linked) (bfd *, struct bfd_section *,
> struct bfd_link_info *);
> 
>/* Routines to handle dynamic symbols and relocs.  */
>  #define BFD_JUMP_TABLE_DYNAMIC(NAME) \
> @@ -5128,10 +5128,10 @@ bfd_boolean bfd_link_split_section (bfd
>  #define bfd_link_split_section(abfd, sec) \
> BFD_SEND (abfd, _bfd_link_split_section, (abfd, sec))
> 
> -void bfd_section_already_linked (bfd *abfd, asection *sec);
> +void bfd_section_already_linked (bfd *abfd, asection *sec, struct
> bfd_link_info *);
> 
> -#define bfd_section_already_linked(abfd, sec) \
> -   BFD_SEND (abfd, _section_already_linked, (abfd, sec))
> +#define bfd_section_already_linked(abfd, sec, info) \
> +   BFD_SEND (abfd, _section_already_linked, (abfd, sec, info))
> 
>  /* Extracted from simple.c.  */
>  bfd_byte *bfd_simple_get_relocated_section_contents
> Index: bfd/elf-bfd.h
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/bfd/elf-bfd.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 elf-bfd.h
> --- bfd/elf-bfd.h25 Aug 2015 02:24:49 -1.4
> +++ bfd/elf-bfd.h17 Apr 2016 05:14:16 -
> @@ -1371,6 +1371,9 @@ struct elf_obj_tdata
> 
>/* Used to determine if we are creating an executable.  */
>bfd_boolean executable;
> +
> +  /* Symbol buffer.  */
> +  Elf_Internal_Sym *symbuf;
>  };
> 
>  #define elf_tdata(bfd)((bfd) -> tdata.elf_obj_data)
> @@ -1503,11 +1506,11 @@ extern bfd_boolean _bfd_elf_match_sectio
>  extern bfd_boolean bfd_elf_is_group_section
>(bfd *, const struct bfd_section *);
>  extern void _bfd_elf_section_already_linked
> -  (bfd *, struct bfd_section *);
> +  (bfd *, struct bfd_section *, struct bfd_link_info *);
>  extern void bfd_elf_set_group_contents
>(bfd *, asection *, void *);
>  extern asection *_bfd_elf_check_kept_section
> -  (asection *);
> +  (asection *, struct bfd_link_info *);
>  extern void _bfd_elf_link_just_syms
>(asection *, struct bfd_link_info *);
>  extern bfd_boolean _bfd_elf_copy_private_header_data
> @@ -1703,7 +1706,7 @@ extern bfd_boolean _bfd_elf_symbol_refs_
>(struct elf_link_hash_entry *, struct bfd_link_info *, bfd_boolean);
> 
>  extern bfd_boolean bfd_elf_match_symbols_in_sections
> -  (asection *sec1, asection *sec2);
> +  (asection *, asection *, struct bfd_link_info *);
> 
>  extern bfd_boolean _bfd_elf_setup_sections
>(bfd *);
> Index: bfd/elf.c
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/bfd/elf.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 elf.c
> --- bfd/elf.c13 Jan 2015 20:05:43 -1.7
> +++ bfd/elf.c17 Apr 2016 05:14:18 -
> @@ -3101,7 +3101,7 @@ assign_section_numbers (bfd *abfd, struc
>   s, s->owner);
>/* Point to the kept section if it has the same
>   

Fwd: CVS: cvs.openbsd.org: src

2016-05-07 Thread Philip Guenther
For those who don't following source-changes but build -current from source...

I just committed a major ABI change; threaded binaries from before
late-March will no longer work and you need to follow the instructions
I just committed to the FAQ's "following -current" page!

This is probably a good time to wait for a snapshot and install that.
That'll give ports builds a chance to catch up too.  :-)


Philip Guenther


-- Forwarded message --
From: Philip Guenther 
Date: Sat, May 7, 2016 at 12:05 PM
Subject: CVS: cvs.openbsd.org: src
To: source-chan...@openbsd.org


CVSROOT:/cvs
Module name:src
Changes by: guent...@cvs.openbsd.org2016/05/07 13:05:24

Modified files:


Log message:
Use a Thread Information Block in both single and multi-threaded programs.
This stores errno, the cancelation flags, and related bits for each thread
and is allocated by ld.so or libc.a.  This is an ABI break from 5.9-stable!

Make libpthread dlopen'able by moving the cancelation wrappers into libc
and doing locking and fork/errno handling via callbacks that libpthread
registers when it first initializes.  'errno' *must* be declared via
 now!

Clean up libpthread's symbol exports like libc.

On powerpc, offset the TIB/TCB/TLS data from the register per the ELF spec.

Testing by various, particularly sthen@ and patrick@
ok kettenis@



Re: rtadvd.conf.5: document route preference flags

2016-05-07 Thread Jeremie Courreges-Anglas
Martin Pieuchot  writes:

> On 06/05/16(Fri) 21:42, Jeremie Courreges-Anglas wrote:
>> 
>> Also motivated by http://marc.info/?l=openbsd-misc&m=146239072929264&w=2
>> 
>> RFC4191 defines route preference flags not only in Route Information
>> options, but also in Router Advertisement messages.  Let's try to make
>> this clear.  Also, try to document the possible values in a slightly
>> more useful way...
>> 
>> Comments / oks?
>
> I wish this config file could be sane and use keywords just like our
> other deamons.  I use rtadvd for debugging purpose and I find its syntax
> horrible.

Indeed.

> That said I'm ok with your diff.

Committed with a couple improvements from jmc@

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: xclock patch

2016-05-07 Thread Theo de Raadt
> > So I don't understand why pledge is being added here.
> 
> Because you suggested it back in november...

Of course I suggested it: I believe I said it would be nice if
the simplest of X programs were investigated.

But everywhere else pledge was inserted, that requires (a) full
understanding of what is being done, and (b) full testing.

It is not like I said "pledge xclock, noone gives a fuck if it
works".
 
> > The addition of
> > pledge is breaking xclock.  A pledge addition should not break a
> > program (coredump) in any circumstance, it should only serve to secure
> > it in conditions of true & undetected misbehaviour.  Sure, pledge can
> > make programs safer, but only if they also keep working!!  Otherwise
> > this is very much throwing the baby out with the bathwater.
> 
> I see only one solution here: remove the pledge until Xlib can be
> changed to read XErrorDB early and then use the in-memory version for
> error reporting.
> 
> ok?

Probably.

Don't hold me responsible for the shortcuts.



Re: xclock patch

2016-05-07 Thread Matthieu Herrb
On Sat, May 07, 2016 at 07:15:00AM -0600, Theo de Raadt wrote:
> > On Sat, May 07, 2016 at 06:48:58AM -0500, Edgar Pettijohn wrote:
> > > > 
> > > > else, if it isn't in xclock code, it could be a open(2) call in some
> > > > X11R6 library.
> > > > 
> > > I looked all I found was access() which if I'm reading pledge correctly 
> > > it doesn't need pledge. Is that right?
> > > 
> > 
> > What I mean is the error code path in libX11 (for example).
> > 
> > After checking the code source:
> >   - the default error handler (for not fatal) error is _XDefaultError 
> > (xenocara/lib/libX11/src/XlibInt.c:1375)
> >   - it calls _XPrintDefaultError (xenocara/lib/libX11/src/XlibInt.c:1263)
> >   - it calls XGetErrorDatabaseText (xenocara/lib/libX11/src/ErrDes.c:139)
> >   - it calls XrmGetFileDatabase (xenocara/lib/libX11/src/Xrm.c:1669)
> >   - it calls ReadInFile (xenocara/lib/libX11/src/Xrm.c:1573)
> >   - it calls _XOpenFile (xenocara/lib/libX11/src/XlibInt.c:1830)
> >   - ... and _XOpenFile will do access(2) and open(2) on filename, and
> > kernel will kill it due to pledge.
> > 
> > I am still unsure if adding "rpath" is the good way to deal with that.
> > 
> > But the problem you saw could be related to some X11 error that trigger
> > a access(2) or a open(2) for showing error message.
> 
> So, /usr/X11R6/share/X11/XErrorDB is the file in question?  That is
> being parsed very late.  That is quite an outdated model... like, when
> something has goes wrong, let's open a file and parse lots of strings!
> Alas, that is the story of X.
> 
> Sigh.
> 
> However I've said it before, and I'll say it again.  pledge should
> only be used in commands when the command is fully understood top to
> bottom, and fully tested as well.  This command uses a library that
> was not studied.  The result was not fully tested.  A bit of
> optimism-driven-development is fine, but when the complexity is high
> it cannot be the only foundation.
> 
> I am not really surprised to see the resulting failure.

Well, XErrorDB is only accessed if an X protocol error happens. Given
the nature of xclock, it's quite hard to cause such an error in
regular use of the application. I still have to find how to cause the
kind of error that Edgar encountered.

> 
> The result is that xclock is broken in 5.9.  If a program does not
> fully perform the actions it previously promised to do, then the
> addition of pledge BROKE IT.

I agree, but few people will hit the broken part.
> 
> So the next optimism-driven development will be "add rpath".  Without
> a full study of the large libraries it links against.  After rpath is
> added, I expect more breakage in xclock will be found, because there
> will be something else in the libraries.
> 
> I don't believe that xclock callpath & reach into libX11 has been
> audited & verified to operate only using the POSIX-subset provided by
> pledge "stdio".  Nor, do I believe the same for "stdio rpath".  It is
> a lot of code, I just cannot be sure.
> 
> So I don't understand why pledge is being added here.

Because you suggested it back in november...

> The addition of
> pledge is breaking xclock.  A pledge addition should not break a
> program (coredump) in any circumstance, it should only serve to secure
> it in conditions of true & undetected misbehaviour.  Sure, pledge can
> make programs safer, but only if they also keep working!!  Otherwise
> this is very much throwing the baby out with the bathwater.

I see only one solution here: remove the pledge until Xlib can be
changed to read XErrorDB early and then use the in-memory version for
error reporting.

ok?
-- 
Matthieu Herrb


signature.asc
Description: PGP signature


Re: [patch] memory leak in vi

2016-05-07 Thread Todd C. Miller
On Sat, 07 May 2016 15:10:24 +0200, Martijn van Duren wrote:

> There's a memory leak in vi in the REALLOC{,ARRAY} macro.
> This patch should fix the leak. A similar diff is already
> present in FreeBSD's vi.

OK millert@

 - todd



libc/sparc64 ABI cleanup

2016-05-07 Thread Philip Guenther

Currently, the soft-float bits in libc on sparc64 export not just the 
sparc ABI symbols _Q_* and _Qp_*, but also the internal implementation 
bits, __fpu_*

AFAICT, the __fpu_* names are not required by our tool chain; in contrast, 
calls to the _Q* names are generated by gcc.

Diff below cleans this up, hiding the __fpu_* symbols, removing five 
__fpu_* functions that are unused, and making sure that (almost) all the 
internal references to _Q* go direct via hidden internal names.  This 
reduces the PLT to just seven entries.


Am I missing some secret requirement for __fpu_* to be exported?  
Assuming not: oks?


Philip

Index: arch/sparc64/Symbols.list
===
RCS file: /cvs/src/lib/libc/arch/sparc64/Symbols.list,v
retrieving revision 1.3
diff -u -p -r1.3 Symbols.list
--- arch/sparc64/Symbols.list   13 Sep 2015 08:31:47 -  1.3
+++ arch/sparc64/Symbols.list   8 May 2016 05:51:42 -
@@ -51,32 +51,6 @@ _Qp_uxtoq
 _Qp_xtoq
 __builtin_saveregs
 __dtoul
-__fpu_add
-__fpu_compare
-__fpu_div
-__fpu_dtof
-__fpu_explode
-__fpu_ftod
-__fpu_ftoi
-__fpu_ftoq
-__fpu_ftos
-__fpu_ftox
-__fpu_getreg32
-__fpu_getreg64
-__fpu_implode
-__fpu_itof
-__fpu_mul
-__fpu_newnan
-__fpu_norm
-__fpu_qtof
-__fpu_setreg32
-__fpu_setreg64
-__fpu_shr
-__fpu_sqrt
-__fpu_stof
-__fpu_uitof
-__fpu_uxtof
-__fpu_xtof
 __ftoul
 __plt_end
 __plt_start
Index: arch/sparc64/fpu/Makefile.inc
===
RCS file: /cvs/src/lib/libc/arch/sparc64/fpu/Makefile.inc,v
retrieving revision 1.1
diff -u -p -r1.1 Makefile.inc
--- arch/sparc64/fpu/Makefile.inc   21 Jul 2003 18:41:30 -  1.1
+++ arch/sparc64/fpu/Makefile.inc   8 May 2016 05:51:42 -
@@ -1,4 +1,5 @@
 #  $OpenBSD: Makefile.inc,v 1.1 2003/07/21 18:41:30 jason Exp $
 SRCS += fpu_add.c fpu_compare.c fpu_div.c fpu_explode.c fpu_implode.c \
-   fpu_mul.c fpu_qp.c fpu_q.c fpu_sqrt.c fpu_subr.c fpu_reg.c
+   fpu_mul.c fpu_qp.c fpu_q.c fpu_sqrt.c fpu_subr.c
+# fpu_reg.c
 .PATH: ${.CURDIR}/arch/sparc64/fpu
Index: arch/sparc64/fpu/fpu_explode.c
===
RCS file: /cvs/src/lib/libc/arch/sparc64/fpu/fpu_explode.c,v
retrieving revision 1.7
diff -u -p -r1.7 fpu_explode.c
--- arch/sparc64/fpu/fpu_explode.c  5 Dec 2012 23:19:59 -   1.7
+++ arch/sparc64/fpu/fpu_explode.c  8 May 2016 05:51:42 -
@@ -292,6 +292,7 @@ __fpu_qtof(fp, i, j, k, l)
FP_TOF(exp, EXT_EXP_BIAS, frac, f0, f1, f2, f3);
 }
 
+#if 0  /* __fpu_explode is unused */
 /*
  * Explode the contents of a / regpair / regquad.
  * If the input is a signalling NaN, an NV (invalid) exception
@@ -362,3 +363,4 @@ __fpu_explode(fe, fp, type, reg)
DUMPFPN(FPE_REG, fp);
DPRINTF(FPE_REG, ("\n"));
 }
+#endif
Index: arch/sparc64/fpu/fpu_extern.h
===
RCS file: /cvs/src/lib/libc/arch/sparc64/fpu/fpu_extern.h,v
retrieving revision 1.3
diff -u -p -r1.3 fpu_extern.h
--- arch/sparc64/fpu/fpu_extern.h   26 Jun 2008 05:42:05 -  1.3
+++ arch/sparc64/fpu/fpu_extern.h   8 May 2016 05:51:42 -
@@ -40,6 +40,7 @@ union instr;
 struct fpemu;
 struct fpn;
 
+__BEGIN_HIDDEN_DECLS
 /* fpu.c */
 int __fpu_exception(struct utrapframe *tf);
 
@@ -86,5 +87,6 @@ int __fpu_shr(register struct fpn *, reg
 void __fpu_norm(register struct fpn *);
 /* Build a new Quiet NaN (sign=0, frac=all 1's). */
 struct fpn *__fpu_newnan(register struct fpemu *);
+__END_HIDDEN_DECLS
 
 #endif /* !_SPARC64_FPU_FPU_EXTERN_H_ */
Index: arch/sparc64/fpu/fpu_q.h
===
RCS file: /cvs/src/lib/libc/arch/sparc64/fpu/fpu_q.h,v
retrieving revision 1.2
diff -u -p -r1.2 fpu_q.h
--- arch/sparc64/fpu/fpu_q.h3 Feb 2004 17:18:13 -   1.2
+++ arch/sparc64/fpu/fpu_q.h8 May 2016 05:51:42 -
@@ -70,3 +70,23 @@ int _Qp_fgt(long double *, long double *
 int _Qp_flt(long double *, long double *);
 int _Qp_fne(long double *, long double *);
 void _Qp_sqrt(long double *, long double *);
+
+PROTO_NORMAL(_Qp_add);
+PROTO_NORMAL(_Qp_div);
+PROTO_NORMAL(_Qp_dtoq);
+PROTO_NORMAL(_Qp_feq);
+PROTO_NORMAL(_Qp_fge);
+PROTO_NORMAL(_Qp_fgt);
+PROTO_NORMAL(_Qp_fle);
+PROTO_NORMAL(_Qp_flt);
+PROTO_NORMAL(_Qp_fne);
+PROTO_NORMAL(_Qp_itoq);
+PROTO_NORMAL(_Qp_mul);
+PROTO_NORMAL(_Qp_qtod);
+PROTO_NORMAL(_Qp_qtoi);
+PROTO_NORMAL(_Qp_qtos);
+PROTO_NORMAL(_Qp_qtoui);
+PROTO_NORMAL(_Qp_sqrt);
+PROTO_NORMAL(_Qp_stoq);
+PROTO_NORMAL(_Qp_sub);
+PROTO_NORMAL(_Qp_uitoq);
Index: arch/sparc64/fpu/fpu_qp.c
===
RCS file: /cvs/src/lib/libc/arch/sparc64/fpu/fpu_qp.c,v
retrieving revision 1.5
diff -u -p -r1.5 fpu_qp.c
--- arch/sparc64/fpu/fpu_qp.c   17 Apr 2014 09:01:25 -  1.5
+++ arch/sparc64/fpu/fpu_qp.c   8 May 2016 05:51:42 -
@@ -37,7 +37,8 @@ __FBSDID("$FreeBSD: src/