smtpd.conf(5): comma in filter chains

[david+bsd]

After quite some debugging of why the heck my smtpd.conf was not
working after upgrading to 5.9 and substituting clamsmtp and dkim-
signer by smtpd(8) filters:

smtpd.conf(5) states:
  filter name chain filter [, ...]
but should say:
  filter name chain filter [...]

Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.156
diff -u -p -r1.156 smtpd.conf.5
--- smtpd.conf.57 Mar 2016 16:21:48 -   1.156
+++ smtpd.conf.59 Apr 2016 09:32:23 -
@@ -612,7 +612,7 @@ using the given filter
 Filters are used to hook into the SMTP dialog and provide additional
filtering
 options for
 .Xr smtpd 8 .
-.It Ic filter Ar name Ic chain Ar filter Op , Ar ...
+.It Ic filter Ar name Ic chain Ar filter Op Ar ...
 Specify a filter chain with the given
 .Ar name
 and filters.

Simplify in_pcblookup()

[Vincent Gross]

in_pcblookup() is always called with *:0 for the remote side.
Remove the useless bits, shuffle the tests around and it's much
easier to audit.

Ok ?

Index: netinet/in_pcb.c
===
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.201
diff -u -p -r1.201 in_pcb.c
--- netinet/in_pcb.c8 Apr 2016 14:34:21 -   1.201
+++ netinet/in_pcb.c9 Apr 2016 09:42:07 -
@@ -415,14 +415,13 @@ in_pcbaddrisavail(struct inpcb *inp, str
struct inpcb *t;
 
if (so->so_euid) {
-   t = in_pcblookup(table, _addr, 0,
-   >sin_addr, lport, INPLOOKUP_WILDCARD,
-   inp->inp_rtableid);
+   t = in_pcblookup_local(table, >sin_addr, lport,
+   INPLOOKUP_WILDCARD, inp->inp_rtableid);
if (t && (so->so_euid != t->inp_socket->so_euid))
return (EADDRINUSE);
}
-   t = in_pcblookup(table, _addr, 0,
-   >sin_addr, lport, wild, inp->inp_rtableid);
+   t = in_pcblookup_local(table, >sin_addr, lport,
+   wild, inp->inp_rtableid);
if (t && (reuseport & t->inp_socket->so_options) == 0)
return (EADDRINUSE);
}
@@ -475,8 +474,8 @@ in_pcbpickport(u_int16_t *lport, void *l
candidate = lower;
localport = htons(candidate);
} while (in_baddynamic(localport, so->so_proto->pr_protocol) ||
-   in_pcblookup(table, _addr, 0,
-   laddr, localport, wild, inp->inp_rtableid));
+   in_pcblookup_local(table, laddr, localport, wild,
+   inp->inp_rtableid));
*lport = localport;
 
return (0);
@@ -734,14 +733,14 @@ in_rtchange(struct inpcb *inp, int errno
 }
 
 struct inpcb *
-in_pcblookup(struct inpcbtable *table, void *faddrp, u_int fport_arg,
-void *laddrp, u_int lport_arg, int flags, u_int rdomain)
+in_pcblookup_local(struct inpcbtable *table, void *laddrp, u_int lport_arg,
+int flags, u_int rdomain)
 {
struct inpcb *inp, *match = NULL;
int matchwild = 3, wildcard;
-   u_int16_t fport = fport_arg, lport = lport_arg;
-   struct in_addr faddr = *(struct in_addr *)faddrp;
+   u_int16_t lport = lport_arg;
struct in_addr laddr = *(struct in_addr *)laddrp;
+   struct in6_addr *laddr6 = (struct in6_addr *)laddrp;
struct inpcbhead *head;
 
rdomain = rtable_l2(rdomain);   /* convert passed rtableid to rdomain */
@@ -753,60 +752,40 @@ in_pcblookup(struct inpcbtable *table, v
continue;
wildcard = 0;
 #ifdef INET6
-   if (flags & INPLOOKUP_IPV6) {
-   struct in6_addr *laddr6 = (struct in6_addr *)laddrp;
-   struct in6_addr *faddr6 = (struct in6_addr *)faddrp;
-
-   if (!(inp->inp_flags & INP_IPV6))
+   if (ISSET(flags, INPLOOKUP_IPV6)) {
+   if (!ISSET(inp->inp_flags, INP_IPV6))
continue;
 
-   if (!IN6_IS_ADDR_UNSPECIFIED(>inp_laddr6)) {
-   if (IN6_IS_ADDR_UNSPECIFIED(laddr6))
-   wildcard++;
-   else if (!IN6_ARE_ADDR_EQUAL(>inp_laddr6, 
laddr6))
-   continue;
-   } else {
-   if (!IN6_IS_ADDR_UNSPECIFIED(laddr6))
-   wildcard++;
-   }
+   if (!IN6_IS_ADDR_UNSPECIFIED(>inp_faddr6))
+   wildcard++;
 
-   if (!IN6_IS_ADDR_UNSPECIFIED(>inp_faddr6)) {
-   if (IN6_IS_ADDR_UNSPECIFIED(faddr6))
+   if (!IN6_ARE_ADDR_EQUAL(>inp_laddr6, laddr6)) {
+   if (IN6_IS_ADDR_UNSPECIFIED(>inp_laddr6) ||
+   IN6_IS_ADDR_UNSPECIFIED(laddr6))
wildcard++;
-   else if (!IN6_ARE_ADDR_EQUAL(>inp_faddr6,
-   faddr6) || inp->inp_fport != fport)
+   else
continue;
-   } else {
-   if (!IN6_IS_ADDR_UNSPECIFIED(faddr6))
-   wildcard++;
}
+
} else
 #endif /* INET6 */
{
 #ifdef INET6
-   if (inp->inp_flags & INP_IPV6)
+   if (ISSET(inp->inp_flags, INP_IPV6))
continue;
 #endif /* INET6 */
 
-   if (inp->inp_faddr.s_addr != INADDR_ANY) {
-

patch: serialize multiple threads calling pledge(2)

[Sebastien Marie]

Hi,

The following diff makes the effect of multiple threads calling
pledge(2) to be serializable.

It adds a loop (with tsleep(9)) at pledge(2) entrance if another thread
is already inside (due to sleep), changes return to goto statment, and
wakeup other threads at end.

The check for looping or continue is done using a new flag PLEDGE_BUSY,
which mark a thread of the current process is currently inside
sys_pledge().

This diff was done with the help of deraadt@ and guenther@.

Comments or OK ?
-- 
Sebastien Marie


Index: sys/kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.162
diff -u -p -r1.162 kern_pledge.c
--- sys/kern/kern_pledge.c  30 Mar 2016 07:49:11 -  1.162
+++ sys/kern/kern_pledge.c  9 Apr 2016 08:34:16 -
@@ -399,8 +399,23 @@ sys_pledge(struct proc *p, void *v, regi
syscallarg(const char *)request;
syscallarg(const char **)paths;
} */*uap = v;
-   uint64_t flags = 0;
-   int error;
+   uint64_t flags = PLEDGE_BUSY;
+   int error = 0;
+
+   /*
+* makes the effect of multiple threads calling pledge(2) to be
+* serializable. We are under KERN_LOCK, but pledge(2) could sleep due
+* to malloc(M_WAITOK) usage.
+*
+* so wait to `PLEDGE_BUSY' be cleared.
+*/
+   while (ISSET(p->p_p->ps_pledge, PLEDGE_BUSY)) {
+   error = tsleep(>p_p->ps_pledge, PUSER, "pledge", 0);
+   if (error)
+   return error;
+   }
+   SET(p->p_p->ps_pledge, PLEDGE_BUSY);
+
 
if (SCARG(uap, request)) {
size_t rbuflen;
@@ -412,7 +427,7 @@ sys_pledge(struct proc *p, void *v, regi
);
if (error) {
free(rbuf, M_TEMP, MAXPATHLEN);
-   return (error);
+   goto out;
}
 #ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
@@ -428,20 +443,24 @@ sys_pledge(struct proc *p, void *v, regi
 
if ((f = pledgereq_flags(rp)) == 0) {
free(rbuf, M_TEMP, MAXPATHLEN);
-   return (EINVAL);
+   error = EINVAL;
+   goto out;
}
flags |= f;
}
free(rbuf, M_TEMP, MAXPATHLEN);
 
-   if (flags & ~PLEDGE_USERSET)
-   return (EINVAL);
+   if (flags & ~PLEDGE_USERSET) {
+   error = EINVAL;
+   goto out;
+   }
 
if ((p->p_p->ps_flags & PS_PLEDGE)) {
/* Already pledged, only allow reductions */
if (((flags | p->p_p->ps_pledge) & PLEDGE_USERSET) !=
(p->p_p->ps_pledge & PLEDGE_USERSET)) {
-   return (EPERM);
+   error = EPERM;
+   goto out;
}
 
flags &= p->p_p->ps_pledge;
@@ -451,7 +470,8 @@ sys_pledge(struct proc *p, void *v, regi
 
if (SCARG(uap, paths)) {
 #if 1
-   return (EINVAL);
+   error = EINVAL;
+   goto out;
 #else
const char **u = SCARG(uap, paths), *sp;
struct whitepaths *wl;
@@ -545,7 +565,13 @@ sys_pledge(struct proc *p, void *v, regi
p->p_p->ps_flags |= PS_PLEDGE;
}
 
-   return (0);
+out:
+   /*
+* unbusying pledge, and wakeup other threads if any.
+*/
+   CLR(p->p_p->ps_pledge, PLEDGE_BUSY);
+   wakeup(>p_p->ps_pledge);
+   return error;
 }
 
 int
Index: sys/sys/pledge.h
===
RCS file: /cvs/src/sys/sys/pledge.h,v
retrieving revision 1.27
diff -u -p -r1.27 pledge.h
--- sys/sys/pledge.h9 Jan 2016 06:13:44 -   1.27
+++ sys/sys/pledge.h9 Apr 2016 08:34:16 -
@@ -56,6 +56,7 @@
 #define PLEDGE_DPATH   0x1000ULL   /* mknod & mkfifo */
 #define PLEDGE_DRM 0x2000ULL   /* drm ioctls */
 #define PLEDGE_VMM 0x4000ULL   /* vmm ioctls */
+#define PLEDGE_BUSY0x8000ULL   /* pledge in progress */
 
 /*
  * Bits outside PLEDGE_USERSET are used by the kernel itself

Re: arm: new FDT-enabled mainbus

[Patrick Wildt]

On Fri, Apr 08, 2016 at 09:38:25PM +0200, Mark Kettenis wrote:
> > Date: Fri, 8 Apr 2016 20:26:14 +0200
> > From: Patrick Wildt 
> > 
> > +void
> > +mainbus_iterate(struct device *self, struct device *match, int node)
> > +{
> > +   for (;
> > +   node != 0;
> > +   node = OF_peer(node))
> > +   {
> > +   /* skip nodes that are already handled by some driver */
> > +   if (!mainbus_is_attached(node))
> > +   mainbus_attach_node(self, match, node);
> > +
> > +   mainbus_iterate(self, match, OF_child(node));
> > +   }
> > +}
> 
> Sorry, but this really is wrong.  You're completely ignoring the tree
> aspect if the FDT.  That is not going to work for more complex designs
> with i2c busses or pci busses.  And it is going to be essential if you
> want to support suspend/resume.
> 
> The autoconf framework is really quite clever and flexible.  It is
> possible to re-use glue code quite easily.  As files.conf(5)
> documents, device don't directly attach to parents, but to attributes.
> You can re-use those attributes, and parents can define multiple
> attributes.
> 
> You could for example define a generic fdt attribute.  Simple devices
> can then attach directly to that attribute.  And you could also have
> some soc-specific attributes, that you then use to attach devices that
> need some additional glue.
> 
> It'll take a bit of time to figure out the best way to define your
> attributes.  But the end result will be worth it.  And don't be afraid
> to make mistakes.  As long as you drivers use the same "attach_args"
> structure, we can move around attributes without the need to change
> the code.  So it will be easy to adjust the design as we learn more.
> 
> Cheers,
> 
> Mark
> 

I have created an attribute called ofwbus and made mainbus depend on
that tag.  Now devices can attach to mainbus or ofwbus.  I added a
simplebus which does about the same as mainbus.  Using simplebus* at
ofwbus? it should allow attaching a simplebus to a simplebus, thus
creating tree-like-attachment.  I guess this is more like the approach
you had in mind?

diff --git sys/arch/arm/conf/files.arm sys/arch/arm/conf/files.arm
index cb11960..7ecdd06 100644
--- sys/arch/arm/conf/files.arm
+++ sys/arch/arm/conf/files.arm
@@ -16,15 +16,24 @@ filearch/arm/arm/disassem.c ddb
 file   arch/arm/arm/fiq.c  fiq
 file   arch/arm/arm/fiq_subr.S fiq
 
+define ofwbus {}
+
 # mainbus files
-device mainbus {}
+device mainbus: ofwbus
 attach mainbus at root
 file   arch/arm/mainbus/mainbus.c  mainbus
 
+device simplebus: ofwbus
+attach simplebus at ofwbus
+file   arch/arm/simplebus/simplebus.c  simplebus
+
+# FDT support
+file   dev/ofw/fdt.c
+
 include "arch/arm/cortex/files.cortex"
 
 device cpu {}
-attach cpu at mainbus with cpu_mainbus
+attach cpu at ofwbus with cpu_mainbus
 file   arch/arm/mainbus/cpu_mainbus.c  cpu_mainbus
 
 
diff --git sys/arch/arm/cortex/files.cortex sys/arch/arm/cortex/files.cortex
index c0f4359..f6f1852 100644
--- sys/arch/arm/cortex/files.cortex
+++ sys/arch/arm/cortex/files.cortex
@@ -2,7 +2,7 @@
 
 # ARM core
 device cortex {}
-attach cortex at mainbus
+attach cortex at ofwbus
 file   arch/arm/cortex/cortex.ccortex
 
 device ampintc
diff --git sys/arch/arm/mainbus/mainbus.c sys/arch/arm/mainbus/mainbus.c
index 6ad3e8f..2c083fc 100644
--- sys/arch/arm/mainbus/mainbus.c
+++ sys/arch/arm/mainbus/mainbus.c
@@ -1,124 +1,251 @@
-/* $OpenBSD: mainbus.c,v 1.7 2013/05/30 16:15:01 deraadt Exp $ */
-/* $NetBSD: mainbus.c,v 1.3 2001/06/13 17:52:43 nathanw Exp $ */
-
+/* $OpenBSD$ */
 /*
- * Copyright (c) 1994,1995 Mark Brinicombe.
- * Copyright (c) 1994 Brini.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. All advertising materials mentioning features or use of this software
- *must display the following acknowledgement:
- * This product includes software developed by Brini.
- * 4. The name of the company nor the name of the author may be used to
- *endorse or promote products derived from this software without specific
- *prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY BRINI ``AS IS'' AND ANY EXPRESS OR IMPLIED
- * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL BRINI OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
- * 

Re: smtpd.conf(5): comma in filter chains

[Jason McIntyre]

On Sat, Apr 09, 2016 at 11:42:58AM +0200, david+bsd@dahlberg.cologne wrote:
> After quite some debugging of why the heck my smtpd.conf was not
> working after upgrading to 5.9 and substituting clamsmtp and dkim-
> signer by smtpd(8) filters:
> 
> smtpd.conf(5) states:
> ?? filter name chain filter [, ...]
> but should say:
> ?? filter name chain filter [...]
> 

hi.

could you explain what works and what doesn;t? from reading this, i'd
expect the doc is trying to say you can specify one filter like this:

filter x chain y

and multiple filters:

filter x chain y,z

are you speciying one argument to "chain" or multiple?

jmc

> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.156
> diff -u -p -r1.156 smtpd.conf.5
> --- smtpd.conf.57 Mar 2016 16:21:48 -??1.156
> +++ smtpd.conf.59 Apr 2016 09:32:23 -
> @@ -612,7 +612,7 @@ using the given filter
> ??Filters are used to hook into the SMTP dialog and provide additional
> filtering
> ??options for
> ??.Xr smtpd 8 .
> -.It Ic filter Ar name Ic chain Ar filter Op , Ar ...
> +.It Ic filter Ar name Ic chain Ar filter Op Ar ...
> ??Specify a filter chain with the given
> ??.Ar name
> ??and filters.
> 

Re: patch: serialize multiple threads calling pledge(2)

[Sebastien Marie]

On Sun, Apr 10, 2016 at 12:25:55AM +0200, Mark Kettenis wrote:
> 
> I really hope people won't deliberately write code that allows for
> simultanious execution of pledge(2) in multiple threads.  In fact the
> only justification for calling pledge(2) in a multi-threaded process
> is if you wanted to create threads and then call pledge(2) to prevent
> the creation of more threads.
>
> So the purpose of this diff is to prevent an attacker from exploiting
> the race between multiple pledge(2) calls to circumvent the pledge?
> Currently that risk isn't really there.  Or at least it could be
> avoided by slightly reorganizing the code.  But I guess things get
> more complicated once the whitelist stuff gets activated.
> 
> I agree with Ted's remark about using rwlock(9) though.
> 

First, thanks for your comments.

Yes, whitelist stuff is the reason for this diff, even if theorically
there is already a race in pledge(2) due to malloc(M_WAITOK), which
could sleep, so a thread inside sys_pledge() could release KERN_LOCK,
letting another thread of same process enter also in sys_pledge().

I am trying to move the current complexity of the algo for checking
whitelisted paths from namei() to sys_pledge(), in order to reduce the
performance impact at namei() time (and the possible use of pledge(2)
for attacking locally the system).

As result, sys_pledge() would have more work to do, and to avoid doing
all his stuff in one time under KERN_LOCK (and becoming a vector attack
for DoSing locally), it could have to yield() in the middle of the
processing (or completely release KERN_LOCK for processing and grab it
before commiting ? I will think about that too).

It is mainly parsing the list of paths and do preprocessing on it in
order to organize them in a way namei() could be O(log n) concerning
whitelisted paths validation.




About the use of pledge(2) in multithread programs, I agree that
pledge(2) isn't intented to be used in this way. But this syscall has
process-wide effect and it should behave consistently if called from
multithreaded programs (as if pledge(2) calls occured in some serial
order).

Currently it isn't exactly the case: you could enter in pledge(2), sleep
(with current code, the probability to sleep is low: it would be due to
malloc(M_WAITOK)), and return 0 (promises applied), and finally have
others promises applied (from other thread).

On the other side, the problem concerns only consistent behaviour for
concurrent pledge(2) calls: if sleep occurs on middle of this syscall,
shared structures (ps_pledge or ps_pledgepaths) are still consistents,
and restrictions should be still correctly applied.

It has to be noted also that some programming languages use threading by
default, letting pledge(2) possible use to occurs necessary in one
thread (haskell with ghci for example). So just deny pledge(2) usage in
threaded context isn't really a solution.



About the use of tsleep(9), it was a way proposed to me for resolving
the "atomic" view of pledge(2) from userland, with getblk() as example.
But as I am far to understand all internals in kernel, I have no problem
to look at another way to resolv this.

Thanks.
-- 
Sebastien Marie

Re: tcpdump man page

[Ted Unangst]

Edgar Pettijohn wrote:
> The -i flag doesn't appear to do what the man page suggests. Correcting 
> the source is above my paygrade, but the man page isn't.

hmm? what do you think tcpdump does if -i isn't specified?

> 
> Index: tcpdump.8
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
> retrieving revision 1.88
> diff -u -p -u -r1.88 tcpdump.8
> --- tcpdump.85 Nov 2015 09:56:21 -1.88
> +++ tcpdump.810 Apr 2016 02:44:27 -
> @@ -121,13 +121,6 @@ Print the interface on each dump line.
>   .It Fl i Ar interface
>   Listen on
>   .Ar interface .
> -If unspecified,
> -.Nm
> -searches the system interface list for the lowest numbered, configured
> -.Dq up
> -interface
> -.Pq excluding loopback .
> -Ties are broken by choosing the earliest match.
>   .It Fl L
>   List the supported data link types for the interface and exit.
>   .It Fl l
> 

Re: tcpdump man page

[Edgar Pettijohn]

On 04/09/16 21:51, Ted Unangst wrote:

Edgar Pettijohn wrote:

The -i flag doesn't appear to do what the man page suggests. Correcting
the source is above my paygrade, but the man page isn't.

hmm? what do you think tcpdump does if -i isn't specified?


totally misread. thought it meant more like so:

# tcpdump -i

would act like

# tcpdump

Index: tcpdump.8
===
RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
retrieving revision 1.88
diff -u -p -u -r1.88 tcpdump.8
--- tcpdump.85 Nov 2015 09:56:21 -1.88
+++ tcpdump.810 Apr 2016 02:44:27 -
@@ -121,13 +121,6 @@ Print the interface on each dump line.
   .It Fl i Ar interface
   Listen on
   .Ar interface .
-If unspecified,
-.Nm
-searches the system interface list for the lowest numbered, configured
-.Dq up
-interface
-.Pq excluding loopback .
-Ties are broken by choosing the earliest match.
   .It Fl L
   List the supported data link types for the interface and exit.
   .It Fl l

Re: pledge in /bin/mv

[lists]

Sat, 9 Apr 2016 21:30:39 +0200 Theo Buehler 
> On Sat, Apr 09, 2016 at 03:45:58PM +, Héctor Luis Gimbatti wrote:
> > Is ok?  
> 
> no, the ?chmod and ?chown calls would require "fattr" as well, but since
> pledge restricts these calls substantially, pledging mv is currently
> impossible.
> 
> try moving a file between two different partitions with your mv.
> 
> > Index: mv.c
> > ===
> > RCS file: /cvs/src/bin/mv/mv.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 mv.c
> > --- mv.c17 Nov 2015 18:34:00 -  1.43
> > +++ mv.c9 Apr 2016 15:44:59 -
> > @@ -72,6 +72,9 @@ main(int argc, char *argv[])
> > struct stat sb;
> > int ch;
> > char path[PATH_MAX];
> > +
> > +   if (pledge("stdio rpath wpath cpath", NULL) == -1)
> > +   err(1, "pledge");
> > 
> > while ((ch = getopt(argc, argv, "if")) != -1)
> > switch (ch) {
> > 
> > 
> > --- HLG

Hi Hector,

Looks like you're trying to pledge some programs, and ports are in need
of achieving pledged state.  Maybe you can simply try it out with ports?

Regards,
Anton

Re: ksh: \" inside double quoted `..`

[lists]

Thu, 7 Apr 2016 19:15:51 +0200 Christian Weisgerber 
> Let's illustrate the issue:
> 
> $ sh  -c 'echo "`echo \"hi\"`"'
> hi
> $ sh -o posix -c 'echo "`echo \"hi\"`"'
> "hi"

It is important this is consistently verified in evaluations, here
strings, here docs, filters, pipes, streams, aliases, functions and
other (less) advanced (k)sh constructs.  Thank you for considering
these other use cases and related changes to escaping with quotes.

> I'll just quote ksh.1:
> 
>  o   Occurrences of \" inside double quoted `..` command substitutions.
>  In POSIX mode, the \" is interpreted when the command is interpreted;
>  in non-POSIX mode, the backslash is stripped before the command
>  substitution is interpreted.  For example, echo "`echo \"hi\"`"
>  produces ``"hi"'' in POSIX mode, ``hi'' in non-POSIX mode.  To avoid
>  problems, use the $(...) form of command substitution.
> 
> From time to time there's a configure script in ports that sets -o
> posix but expects the other behavior, because other popular shells
> do not share this interpretation of what POSIX mandates.  bash,
> whether in POSIX mode or not, and FreeBSD's sh always strip the
> backslashes.

Of course, if the general movement is towards changing a particular
point in the standard, based on wide adoption and movement in that
direction already, there is no point in waiting for POSIX to react.

While I particularly have no strong preference, it is also important
for for all shell script users and applications to have a consistent
expected behaviour on our system, and not have ports or other systems
introduce changes to us for a sidetrack from standards that would 
eventually create more points of deviation and mind load user end.

> On reading the POSIX text, I find the ksh author's interpretation
> far-fetched.  See also:
> http://austingroupbugs.net/view.php?id=1015
> 
> Regardless of what you contend the standard text says, I think it
> is clear that the other interpretation has won out in practice and
> I propose to align our ksh with that:
> 
> Index: lex.c
> ===
> RCS file: /cvs/src/bin/ksh/lex.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 lex.c
> --- lex.c 4 Mar 2016 09:37:23 -   1.68
> +++ lex.c 7 Apr 2016 16:18:13 -
> @@ -427,40 +427,22 @@ yylex(int cf)
>   /* Need to know if we are inside double quotes
>* since sh/at translate the \" to " in
>* "`..\"..`".
> -  * This is not done in posix mode (section
> -  * 3.2.3, Double Quotes: "The backquote shall
> -  * retain its special meaning introducing the
> -  * other form of command substitution (see
> -  * 3.6.3). The portion of the quoted string
> -  * from the initial backquote and the
> -  * characters up to the next backquote that
> -  * is not preceded by a backslash (having
> -  * escape characters removed) defines that
> -  * command whose output replaces `...` when
> -  * the word is expanded."
> -  * Section 3.6.3, Command Substitution:
> -  * "Within the backquoted style of command
> -  * substitution, backslash shall retain its
> -  * literal meaning, except when followed by
> -  * $ ` \.").
>*/
>   statep->ls_sbquote.indquotes = 0;
> - if (!Flag(FPOSIX)) {
> - Lex_state *s = statep;
> - Lex_state *base = state_info.base;
> - while (1) {
> - for (; s != base; s--) {
> - if (s->ls_state == 
> SDQUOTE) {
> - 
> statep->ls_sbquote.indquotes = 1;
> - break;
> - }
> - }
> - if (s != base)
> - break;
> - if (!(s = s->ls_info.base))
> + Lex_state *s = statep;
> + Lex_state *base = state_info.base;
> + while (1) {
> + for (; s != base; s--) {
> +

Re: patch: serialize multiple threads calling pledge(2)

[Mark Kettenis]

> Date: Sat, 9 Apr 2016 10:47:14 +0200
> From: Sebastien Marie 
> 
> Hi,
> 
> The following diff makes the effect of multiple threads calling
> pledge(2) to be serializable.
> 
> It adds a loop (with tsleep(9)) at pledge(2) entrance if another thread
> is already inside (due to sleep), changes return to goto statment, and
> wakeup other threads at end.
> 
> The check for looping or continue is done using a new flag PLEDGE_BUSY,
> which mark a thread of the current process is currently inside
> sys_pledge().
> 
> This diff was done with the help of deraadt@ and guenther@.
> 
> Comments or OK ?

I really hope people won't deliberately write code that allows for
simultanious execution of pledge(2) in multiple threads.  In fact the
only justification for calling pledge(2) in a multi-threaded process
is if you wanted to create threads and then call pledge(2) to prevent
the creation of more threads.

So the purpose of this diff is to prevent an attacker from exploiting
the race between multiple pledge(2) calls to circumvent the pledge?
Currently that risk isn't really there.  Or at least it could be
avoided by slightly reorganizing the code.  But I guess things get
more complicated once the whitelist stuff gets activated.

I agree with Ted's remark about using rwlock(9) though.

pledge in /bin/mv

[Héctor Luis Gimbatti]

Is ok?

Index: mv.c
===
RCS file: /cvs/src/bin/mv/mv.c,v
retrieving revision 1.43
diff -u -p -r1.43 mv.c
--- mv.c17 Nov 2015 18:34:00 -  1.43
+++ mv.c9 Apr 2016 15:44:59 -
@@ -72,6 +72,9 @@ main(int argc, char *argv[])
struct stat sb;
int ch;
char path[PATH_MAX];
+
+   if (pledge("stdio rpath wpath cpath", NULL) == -1)
+   err(1, "pledge");

while ((ch = getopt(argc, argv, "if")) != -1)
switch (ch) {


--- HLG

pledge in /usr.bin/arch

[Héctor Luis Gimbatti]

Index: arch.c
===
RCS file: /cvs/src/usr.bin/arch/arch.c,v
retrieving revision 1.17
diff -u -p -r1.17 arch.c
--- arch.c  7 Dec 2015 18:11:37 -   1.17
+++ arch.c  9 Apr 2016 15:04:14 -
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 

 static void __dead usage(void);

@@ -40,6 +41,9 @@ main(int argc, char *argv[])
extern char *__progname;
int short_form = 0, c;
char *arch, *opts;
+
+   if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");

machine = strcmp(__progname, "machine") == 0;
if (machine) {
--- HLG

tcpdump man page

[Edgar Pettijohn]

The -i flag doesn't appear to do what the man page suggests. Correcting 
the source is above my paygrade, but the man page isn't.


Index: tcpdump.8
===
RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
retrieving revision 1.88
diff -u -p -u -r1.88 tcpdump.8
--- tcpdump.85 Nov 2015 09:56:21 -1.88
+++ tcpdump.810 Apr 2016 02:44:27 -
@@ -121,13 +121,6 @@ Print the interface on each dump line.
 .It Fl i Ar interface
 Listen on
 .Ar interface .
-If unspecified,
-.Nm
-searches the system interface list for the lowest numbered, configured
-.Dq up
-interface
-.Pq excluding loopback .
-Ties are broken by choosing the earliest match.
 .It Fl L
 List the supported data link types for the interface and exit.
 .It Fl l

Re: patch: serialize multiple threads calling pledge(2)

[Ted Unangst]

Sebastien Marie wrote:
> Hi,
> 
> The following diff makes the effect of multiple threads calling
> pledge(2) to be serializable.
> 
> It adds a loop (with tsleep(9)) at pledge(2) entrance if another thread
> is already inside (due to sleep), changes return to goto statment, and
> wakeup other threads at end.

I am strongly opposed to adding new instances of the flag/tsleep locking
pattern. There are inevitably bugs, or at the very least it creates more
work later to make SMP safe.

Don't invent new locks. Use the ones we have (rwlock).

Re: sys_process.c: remove relebad

[Michal Mazurek]

On 10:13:06,  7.04.16, Martin Natano wrote:
> On Wed, Apr 06, 2016 at 09:47:35AM +0200, Michal Mazurek wrote:
> > relebad used to have more body:
> > relebad:
> > PRELE(t);
> > return (error);
> > But then PRELE(t); was removed. This diff gets rid of what remains of
> > relebad.
> 
> Looks good to me.

I was told parens in 'return' are now to be eliminated, here is an
updated diff:

Index: sys_process.c
===
RCS file: /cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.68
diff -u -p -r1.68 sys_process.c
--- sys_process.c   24 Sep 2015 20:35:18 -  1.68
+++ sys_process.c   9 Apr 2016 13:59:18 -
@@ -454,7 +454,7 @@ sys_ptrace(struct proc *p, void *v, regi
/* If the address parameter is not (int *)1, set the pc. */
if ((int *)SCARG(uap, addr) != (int *)1)
if ((error = process_set_pc(t, SCARG(uap, addr))) != 0)
-   goto relebad;
+   return error;
 
 #ifdef PT_STEP
/*
@@ -462,7 +462,7 @@ sys_ptrace(struct proc *p, void *v, regi
 */
error = process_sstep(t, req == PT_STEP);
if (error)
-   goto relebad;
+   return error;
 #endif
goto sendsig;
 
@@ -492,7 +492,7 @@ sys_ptrace(struct proc *p, void *v, regi
 */
error = process_sstep(t, 0);
if (error)
-   goto relebad;
+   return error;
 #endif
 
/* give process back to original parent or init */
@@ -522,9 +522,6 @@ sys_ptrace(struct proc *p, void *v, regi
}
 
return (0);
-
-   relebad:
-   return (error);
 
case  PT_KILL:
if (SCARG(uap, pid) < THREAD_PID_OFFSET && tr->ps_single)

-- 
Michal Mazurek

Re: pledge in /bin/mv

[Theo Buehler]

On Sat, Apr 09, 2016 at 03:45:58PM +, Héctor Luis Gimbatti wrote:
> Is ok?

no, the ?chmod and ?chown calls would require "fattr" as well, but since
pledge restricts these calls substantially, pledging mv is currently
impossible.

try moving a file between two different partitions with your mv.

> Index: mv.c
> ===
> RCS file: /cvs/src/bin/mv/mv.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 mv.c
> --- mv.c17 Nov 2015 18:34:00 -  1.43
> +++ mv.c9 Apr 2016 15:44:59 -
> @@ -72,6 +72,9 @@ main(int argc, char *argv[])
> struct stat sb;
> int ch;
> char path[PATH_MAX];
> +
> +   if (pledge("stdio rpath wpath cpath", NULL) == -1)
> +   err(1, "pledge");
> 
> while ((ch = getopt(argc, argv, "if")) != -1)
> switch (ch) {
> 
> 
> --- HLG
> 
>