dhcpd - pf table handler child not cleaned up

[Adam Wolk]

Hi tech@,

sthen@ pointed out to me that dhcpd doesn't properly terminate the pf table
handler. 

I reproduced the issue both on 6.1 and -current.

Minimal config I used on my server:
/etc/dhcpd.conf
 subnet 45.63.9.186 netmask 255.255.255.224 {
   range 45.63.9.186 45.63.9.186;
 }

enabled dhcpd and used the -A flag to trigger the pf handler being spawned:
 rcctl enable dhcpd
 rcctl set dhcpd flags -A test

and performed the test

# rcctl start dhcpd; rcctl stop dhcpd; pgrep -lf dhcpd
dhcpd(ok)
dhcpd(ok)
96584 dhcpd: pf table handler

which demonstrates that indeed the pf table handler is left running.

Both the main program and the child spin in a for(;;) loop, there is no
code anticipating the need to signal a child to quit, there is also
no attempt to detect the parent dying by the child.

The child also uses a notreached exit(3) after the loop.

When the parent is not around the unattended child proceeds to puke all over
the logfiles:

Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe

brynet@ noticed the exit(3) instead of _exit(2) and he also pointed
out that the pf table handler is a bit optimistic on warnings instead of bailing
out with an error and terminating.

Examples:
55if ((fd = open(_PATH_DEV_PF, O_RDWR|O_NOFOLLOW, 0660)) == -1)
56log_warn("can't open pf device");

this child is useless if it can't pf

57if (chroot(_PATH_VAREMPTY) == -1)
58log_warn("chroot %s", _PATH_VAREMPTY);
59if (chdir("/") == -1)
60log_warn("chdir(\"/\")");
61if (setgroups(1, >pw_gid) ||
62setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
63setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
64log_warn("can't drop privileges");

not able to chroot and drop privileges seems like good enough reason to die.

76if (nfds > 0 && (pfd[0].revents & POLLHUP))
77log_warnx("pf pipe closed");

we won't get any work to do as the pipe is closed, should we die?

Now, I did try a quick diff changing log_warn on those to fatal()
https://junk.tintagel.pl/dhcpd-pf-handler.diff

This of course results in the child dying when the parent is gone
but we believe it's not commitable as the following needs to be
decided:

 - if the handler fails (ie. not able to open pf) should it signal
   the main process to die or just stop working (like it did now)?
 - fatal() can't be used as it uses exit(3) instead of _exit(2)
 - anything else that we might have missed

looking for pointers/suggestions on how to move forward with this.

regards,
Adam

Re: pfctl: make functions return void, merge two ifs

[Adam Wolk]

On Tue, Jun 13, 2017 at 12:43:51AM +0200, Adam Wolk wrote:
> On Mon, Jun 12, 2017 at 11:43:44PM +0200, Alexandr Nedvedicky wrote:
> > Hello Adam,
> > 
> > 
> > 
> > > It was a rainy evening here, so here's the updated pfctl diff.
> > 
> > I'm sorry to hear about the rainy weather [1].
> > anyway, you might want to run regression test for pfctl.
> > 
> > cd $SRC/src/regress/sbin/pfctl
> > cat Makefile
> > # follow instructions
> > 
> > just for sure.
> 
> Ran the tests both on the unmodified and changed pfctl using a stock 
> unmodified
> GENERIC kernel.
> 
> One test case fails pfcmd1.
> 

Yet another rainy day, so revisiting the diff.

1. Re-basing the diff on the recently committed change from the OP

2. Removed the redundant error reporting from:
 - pfctl_clear_tables
 - pfctl_show_tables

3. pfctl_show_ifaces back to using radix_perror for output consistency

4. regress test altered, we are passing in the -n which just results in the
rules being parsed.

The definition for the pfctlcmd testing group in the Makefile is:
# pfcmd: test pfctl command line parsing
after the change the test failed not because of command line parsing but
from the attempt of trying to clear tables from a non existing anchor.

with the -n flag we still test if the anchor command can be combined
with -Fa but don't actually attempt to run the clearing code.

This regress modification passes both with the pfctl before this change as well
as the newly modified one.

Feedback, OK's?
? openbsd-daily-pfctl-1.diff
? openbsd-daily-pfctl-2.diff
? openbsd-daily-pfctl-3.diff
? parse.c
? pfctl
? rain.diff
Index: pfctl.h
===
RCS file: /cvs/src/sbin/pfctl/pfctl.h,v
retrieving revision 1.53
diff -u -p -r1.53 pfctl.h
--- pfctl.h 19 Jan 2015 23:52:02 -  1.53
+++ pfctl.h 16 Jun 2017 21:05:38 -
@@ -75,12 +75,12 @@ int  pfi_get_ifaces(const char *, struct
 int pfi_clr_istats(const char *, int *, int);
 
 voidpfctl_print_title(char *);
-int pfctl_clear_tables(const char *, int);
-int pfctl_show_tables(const char *, int);
+voidpfctl_clear_tables(const char *, int);
+voidpfctl_show_tables(const char *, int);
 int pfctl_command_tables(int, char *[], char *, const char *, char *,
const char *, int);
 voidwarn_namespace_collision(const char *);
-int pfctl_show_ifaces(const char *, int);
+voidpfctl_show_ifaces(const char *, int);
 FILE   *pfctl_fopen(const char *, const char *);
 
 /*
Index: pfctl_table.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_table.c,v
retrieving revision 1.75
diff -u -p -r1.75 pfctl_table.c
--- pfctl_table.c   13 Apr 2017 07:30:21 -  1.75
+++ pfctl_table.c   16 Jun 2017 21:05:38 -
@@ -102,16 +102,18 @@ static const char *istats_text[2][2][2] 
table.pfrt_flags &= ~PFR_TFLAG_PERSIST; \
} while(0)
 
-int
+void
 pfctl_clear_tables(const char *anchor, int opts)
 {
-   return pfctl_table(0, NULL, NULL, "-F", NULL, anchor, opts);
+   if (pfctl_table(0, NULL, NULL, "-F", NULL, anchor, opts) == -1)
+   exit(1);
 }
 
-int
+void
 pfctl_show_tables(const char *anchor, int opts)
 {
-   return pfctl_table(0, NULL, NULL, "-s", NULL, anchor, opts);
+   if (pfctl_table(0, NULL, NULL, "-s", NULL, anchor, opts) == -1)
+   exit(1);
 }
 
 int
@@ -594,7 +596,7 @@ xprintf(int opts, const char *fmt, ...)
 
 /* interface stuff */
 
-int
+void
 pfctl_show_ifaces(const char *filter, int opts)
 {
struct pfr_bufferb;
@@ -608,7 +610,7 @@ pfctl_show_ifaces(const char *filter, in
b.pfrb_size = b.pfrb_msize;
if (pfi_get_ifaces(filter, b.pfrb_caddr, _size)) {
radix_perror();
-   return (1);
+   exit(1);
}
if (b.pfrb_size <= b.pfrb_msize)
break;
@@ -618,7 +620,6 @@ pfctl_show_ifaces(const char *filter, in
pfctl_print_title("INTERFACES:");
PFRB_FOREACH(p, )
print_iface(p, opts);
-   return (0);
 }
 
 void
? pfctl-regress-2.diff
? pfctl-regress.diff
Index: pfcmd1.opts
===
RCS file: /cvs/src/regress/sbin/pfctl/pfcmd1.opts,v
retrieving revision 1.1
diff -u -p -r1.1 pfcmd1.opts
--- pfcmd1.opts 3 Jul 2010 02:32:45 -   1.1
+++ pfcmd1.opts 16 Jun 2017 21:34:29 -
@@ -1 +1 @@
--a regress/does_not_exist -Fa
+-n -a regress/does_not_exist -Fa

Re: smtpd session hang

[adam . wolk]

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

Tested the diff on -current OpenSMTPD running on a 6.1 server.
Sent 11 emails with ~2MB of base64 encoded garbage (like with our previous 
tests)
all emails were delivered with no FD leaks.

I am also willing to test an errata diff on 6.1 if this qualifies 
(reliability/performance).

> 
> Index: usr.sbin/smtpd/smtp_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.303
> diff -u -p -r1.303 smtp_session.c
> --- usr.sbin/smtpd/smtp_session.c 17 May 2017 14:00:06 -  1.303
> +++ usr.sbin/smtpd/smtp_session.c 16 Jun 2017 14:56:11 -
> @@ -1474,12 +1474,12 @@ smtp_data_io(struct io *io, int evt, voi
>   break;
>  
>   case IO_LOWAT:
> - if (s->tx->dataeom && io_queued(s->tx->oev) == 0) {
> - smtp_data_io_done(s);
> - } else if (io_paused(s->io, IO_IN)) {
> + if (io_paused(s->io, IO_IN)) {
>   log_debug("debug: smtp: %p: filter congestion over: 
> resuming session", s);
>   io_resume(s->io, IO_IN);
>   }
> + if (s->tx->dataeom && io_queued(s->tx->oev) == 0)
> + smtp_data_io_done(s);
>   break;
>  
>   default:
> 

Re: pfctl: make functions return void, merge two ifs

[Adam Wolk]

On Mon, Jun 12, 2017 at 11:43:44PM +0200, Alexandr Nedvedicky wrote:
> Hello Adam,
> 
> 
> 
> > It was a rainy evening here, so here's the updated pfctl diff.
> 
> I'm sorry to hear about the rainy weather [1].
> anyway, you might want to run regression test for pfctl.
> 
>   cd $SRC/src/regress/sbin/pfctl
>   cat Makefile
>   # follow instructions
> 
> just for sure.

Ran the tests both on the unmodified and changed pfctl using a stock unmodified
GENERIC kernel.

One test case fails pfcmd1.

Passing test redirected to a file:
# make > /root/pfctl-old.log


cp: /usr/src/regress/sbin/pfctl/pf95.include and
/usr/src/regress/sbin/pfctl/pf95.include are identical (not copied).
cp: /usr/src/regress/sbin/pfctl/pf103.include and
/usr/src/regress/sbin/pfctl/pf103.include are identical (not copied).

Loading anchor x from pf103.include
rules cleared
pfctl: Anchor or Ruleset does not exist.
# echo $?
0
#

Failing test:
# make > /root/pfctl-new.log 
cp: /usr/src/regress/sbin/pfctl/pf95.include and
/usr/src/regress/sbin/pfctl/pf95.include are identical (not copied).
cp: /usr/src/regress/sbin/pfctl/pf103.include and
/usr/src/regress/sbin/pfctl/pf103.include are identical (not copied).

Loading anchor x from pf103.include
rules cleared
pfctl: Anchor or Ruleset does not exist.
pfctl: pfctl_clear_tables: Anchor or Ruleset does not exist
*** Error 1 in . (Makefile:238 'pfcmd1')
# echo $?
0
# 

differences in output:
# diff -u pfctl-old.log pfctl-new.log


--- pfctl-old.log   Tue Jun 13 00:07:57 2017
+++ pfctl-new.log   Tue Jun 13 00:09:19 2017
@@ -720,6 +720,5 @@
 /usr/bin/doas ifconfig tun100 create
 /usr/bin/doas ifconfig tun101 create
 /usr/bin/doas pfctl `cat /usr/src/regress/sbin/pfctl/pfcmd1.opts`  -f
/usr/src/regress/sbin/pfctl/pfcmd1.in
-/usr/bin/doas ifconfig lo100 destroy
-/usr/bin/doas ifconfig tun100 destroy
-/usr/bin/doas ifconfig tun101 destroy
+FAILED
+*** Error 1 in target 'regress' (ignored)


The input data:
-a regress/does_not_exist -Fa

I did not account for the -a anchor command being able to be combined with
other commands.

I also ran the regress tests on the original diff sent to the list (without my
modifications):

# make > /root/pfctl-op.log 
cp: /usr/src/regress/sbin/pfctl/pf95.include and
/usr/src/regress/sbin/pfctl/pf95.include are identical (not copied).
  
cp: /usr/src/regress/sbin/pfctl/pf103.include and
/usr/src/regress/sbin/pfctl/pf103.include are identical (not copied).
 

Loading anchor x from pf103.include 
rules cleared   
pfctl: Anchor or Ruleset does not exist.
# echo $?   
0 
#

differences in output:
# diff -u /root/pfctl-old.log /root/pfctl-op.log
#

They result with no change.

> 
> regards
> sasha
> 
> [1] https://www.youtube.com/watch?v=51Kof78YBVM
> 

Thanks, this made my day :)

Re: pfctl: make functions return void, merge two ifs

[Adam Wolk]

On Mon, Jun 12, 2017 at 01:59:07PM +0200, Mike Belopuhov wrote:
> On Sun, Jun 11, 2017 at 15:03 +0100, Raymond wrote:
> > Transform the following functions (which never return anything other than 
> > 0, and whose return value is never used) to void:
> > 
> > * pfctl_clear_stats, pfctl_clear_interface_flags, pfctl_clear_rules, 
> > pfctl_clear_src_nodes, pfctl_clear_states
> > * pfctl_kill_src_nodes, pfctl_net_kill_states, pfctl_label_kill_states, 
> > pfctl_id_kill_states, pfctl_key_kill_states
> > 
> > inside main: merge two identical if conditions next to each other into one.
> > 
> > credit to
> > - awolk@ for the code reading
> > - mikeb for pointing out we can void all _clear_ functions
> > - ghostyy for pointing out all _kill_ functions can be voided
> >
> 
> Looks good to me.  I was going to point out that pfctl_clear_tables
> should also be converted, but leave that for a rainy day since some
> extra return value checking of pfctl_table call is probably in order.
> 

It was a rainy evening here, so here's the updated pfctl diff.

Note that it's based on top of the original diff from this thread.
The additionally modified files are pfctl_table.c and pfctl.h.

Changes:
 voided:
  - pfctl_clear_tables
  - pfctl_show_tables
  - pfctl_show_ifaces

Tested on amd64 -current with a kernel modified to output the following
errors for the matching ioctls:
  - DIOCRCLRTABLES -> ESRCH
  - DIOCRGETTABLES -> ESRCH
  - DIOCIGETIFACES -> ENOENT

Attaching the pf_ioctl.diff just for reference.

Looking for feedback, and OK's.

# show interfaces

$ doas pfctl -sI
pfctl: Anchor or Ruleset does not exist.
$ echo $?
0

# show tables
$ doas pfctl -sT
pfctl: Table does not exist.
$ echo $?
0

# clear tables
$ doas pfctl -F Tables
pfctl: Table does not exist.
$ echo $?
0

# show all
$ doas pfctl -sa
... trimmed output ...
pfctl: Table does not exist.
... trimmed output ...
$ echo $?
0

Behavior of the modified pfctl binary

# show interfaces
$ doas ./pfctl -sI
pfctl: pfctl_show_ifaces: Anchor or Ruleset does not exist
$ echo $?
1

# show tables
$ doas ./pfctl -sT
pfctl: Table does not exist.
pfctl: pfctl_show_tables: Table does not exist
$ echo $?
1

# clear tables
$ doas ./pfctl -F Tables
pfctl: Table does not exist.
pfctl: pfctl_show_tables: Table does not exist
$ echo $?
1

# show all
$ doas ./pfctl -sa
pfctl: Table does not exist.
pfctl: pfctl_show_tables: Table does not exist
$ echo $?
1

? openbsd-daily-pfctl-1.diff
? parse.c
? pfctl
? rain.diff
Index: pfctl.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.344
diff -u -p -r1.344 pfctl.c
--- pfctl.c 30 May 2017 12:13:04 -  1.344
+++ pfctl.c 12 Jun 2017 19:14:27 -
@@ -61,17 +61,17 @@ void usage(void);
 int pfctl_enable(int, int);
 int pfctl_disable(int, int);
 voidpfctl_clear_queues(struct pf_qihead *);
-int pfctl_clear_stats(int, const char *, int);
-int pfctl_clear_interface_flags(int, int);
-int pfctl_clear_rules(int, int, char *);
-int pfctl_clear_src_nodes(int, int);
-int pfctl_clear_states(int, const char *, int);
+voidpfctl_clear_stats(int, const char *, int);
+voidpfctl_clear_interface_flags(int, int);
+voidpfctl_clear_rules(int, int, char *);
+voidpfctl_clear_src_nodes(int, int);
+voidpfctl_clear_states(int, const char *, int);
 voidpfctl_addrprefix(char *, struct pf_addr *);
-int pfctl_kill_src_nodes(int, const char *, int);
-int pfctl_net_kill_states(int, const char *, int, int);
-int pfctl_label_kill_states(int, const char *, int, int);
-int pfctl_id_kill_states(int, int);
-int pfctl_key_kill_states(int, const char *, int, int);
+voidpfctl_kill_src_nodes(int, const char *, int);
+voidpfctl_net_kill_states(int, const char *, int, int);
+voidpfctl_label_kill_states(int, const char *, int, int);
+voidpfctl_id_kill_states(int, int);
+voidpfctl_key_kill_states(int, const char *, int, int);
 int pfctl_parse_host(char *, struct pf_rule_addr *);
 voidpfctl_init_options(struct pfctl *);
 int pfctl_load_options(struct pfctl *);
@@ -278,7 +278,7 @@ pfctl_disable(int dev, int opts)
return (0);
 }
 
-int
+void
 pfctl_clear_stats(int dev, const char *iface, int opts)
 {
struct pfioc_iface pi;
@@ -296,10 +296,9 @@ pfctl_clear_stats(int dev, const char *i
fprintf(stderr, " for interface %s", iface);
fprintf(stderr, "\n");
}
-   return (0);
 }
 
-int
+void
 pfctl_clear_interface_flags(int dev, int opts)
 {
struct pfioc_iface  pi;
@@ -313,10 +312,9 @@ pfctl_clear_interface_flags(int dev, int
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "pf: interface flags reset\n");
}
-   return (0);
 }
 
-int
+void
 pfctl_clear_rules(int dev, int opts, char *anchorname)
 {
struct pfr_buffer t;
@@ -329,20 +327,18 @@ 

Re: pfctl: make functions return void, merge two ifs

[Adam Wolk]

On Sun, Jun 11, 2017 at 03:03:56PM +0100, Raymond wrote:
> Transform the following functions (which never return anything other than 0, 
> and whose return value is never used) to void:
> 
> * pfctl_clear_stats, pfctl_clear_interface_flags, pfctl_clear_rules, 
> pfctl_clear_src_nodes, pfctl_clear_states
> * pfctl_kill_src_nodes, pfctl_net_kill_states, pfctl_label_kill_states, 
> pfctl_id_kill_states, pfctl_key_kill_states
> 
> inside main: merge two identical if conditions next to each other into one.
> 
> credit to
> - awolk@ for the code reading
> - mikeb for pointing out we can void all _clear_ functions
> - ghostyy for pointing out all _kill_ functions can be voided
> 

OK awolk@, would like to have an OK from a pf person before committing.

Re: usr/bin/ktrace: replace snprintf(3)/write(2) with #define and write(2)

[Adam Wolk]

On Sun, Jun 11, 2017 at 11:10:30AM -0600, Theo de Raadt wrote:
> +   write(STDERR_FILENO, NO_KTRACE, sizeof(NO_KTRACE));
> 
> Naw, I dislike that sizeof.
> 
> You can use dprintf, it is signal-safe in OpenBSD as long as the format
> string doesn't contain floating-point strings.

Attaching updated diff.

? ktrace
? ktrace.1.diff
? ktrace.diff
Index: ktrace.c
===
RCS file: /cvs/src/usr.bin/ktrace/ktrace.c,v
retrieving revision 1.33
diff -u -p -r1.33 ktrace.c
--- ktrace.c18 Jul 2016 09:36:50 -  1.33
+++ ktrace.c11 Jun 2017 17:17:59 -
@@ -250,10 +250,7 @@ usage(void)
 static void
 no_ktrace(int signo)
 {
-   char buf[8192];
-
-   snprintf(buf, sizeof(buf),
+   dprintf(STDERR_FILENO,
 "error:\tktrace() system call not supported in the running 
kernel\n\tre-compile kernel with 'option KTRACE'\n");
-   write(STDERR_FILENO, buf, strlen(buf));
_exit(1);
 }

usr/bin/ktrace: replace snprintf(3)/write(2) with #define and write(2)

[Adam Wolk]

Hi tech@,

Using the GREATSCOTT[1] pattern to output in the ktrace signal handler,
dropping the need for an snprintf and the 8k stack buffer.

Brought to attention by BlackFrog on #openbsd-daily

Feedback, OK's?

Regards,
Adam

[1] - https://marc.info/?l=openbsd-tech=149613049920485=2
Index: ktrace.c
===
RCS file: /cvs/src/usr.bin/ktrace/ktrace.c,v
retrieving revision 1.33
diff -u -p -r1.33 ktrace.c
--- ktrace.c18 Jul 2016 09:36:50 -  1.33
+++ ktrace.c11 Jun 2017 16:58:32 -
@@ -250,10 +250,7 @@ usage(void)
 static void
 no_ktrace(int signo)
 {
-   char buf[8192];
-
-   snprintf(buf, sizeof(buf),
-"error:\tktrace() system call not supported in the running 
kernel\n\tre-compile kernel with 'option KTRACE'\n");
-   write(STDERR_FILENO, buf, strlen(buf));
+#define NO_KTRACE "error:\tktrace() system call not supported in the running 
kernel\n\tre-compile kernel with 'option KTRACE'\n"
+   write(STDERR_FILENO, NO_KTRACE, sizeof(NO_KTRACE));
_exit(1);
 }

Re: nc: missing rpath pledge for -P

[Adam Wolk]

On Sat, Jun 10, 2017 at 12:45:01AM +0200, Theo Buehler wrote:
> On Fri, Jun 09, 2017 at 11:59:44PM +0200, Theo Buehler wrote:
> > On Fri, Jun 09, 2017 at 11:55:26PM +0200, Adam Wolk wrote:
> > > On Fri, Jun 09, 2017 at 11:54:03PM +0200, Adam Wolk wrote:
> > > > On Fri, Jun 09, 2017 at 09:28:29PM +, ra...@openmailbox.org wrote:
> > > > > Hello!
> > > > > 
> > > > > Here is a patch with a pledge bugfix in netcat and some minor style
> > > > > improvements.
> > > > > 
> > > > > An example of how to trigger the bug:
> > > > > 
> > > > > $ nc -Ptest -v -c blog.tintagel.pl 443
> > > > > nc: pledge: Operation not permitted
> > > > > 
> > > > > credits to
> > > > > * awolk@ for drawing attention to netcat.
> > > > > * Juuso Lapinlampi for suggesting to alphabetically order the 
> > > > > #includes.
> > > > > * rajak for pointing out the missing space in the error message.
> > > > > * brynet for pledge style improvements.
> > > > > 
> > > > > 
> > > > 
> > > > OK awolk@ for the updated diff (I'm attaching it inline).
> > > 
> > > forgot the diff
> > 
> > I'm ok with the diff, although I really wish there was a way to simplify
> > the convoluted mess that is the pledge logic in this program.
> > 
> > How many codepaths are there in which the second group of pledge calls
> > actually does anything? Are there any?
> > 
> 
> The first group of pledges is this:
> 
>   if (family == AF_UNIX) { // if (usetls) -> bail out on line 393
>   if (pledge("stdio rpath wpath cpath tmppath unix", NULL) == -1)
>   err(1, "pledge");
>   } else if (Fflag) { // if (usetls) -> bail out on line 397
>   if (Pflag) {
>   if (pledge("stdio inet dns sendfd tty", NULL) == -1)
>   err(1, "pledge");
>   } else if (pledge("stdio inet dns sendfd", NULL) == -1)
>   err(1, "pledge");
>   } else if (Pflag) {
>   if (pledge("stdio inet dns tty", NULL) == -1)
>   err(1, "pledge");
>   } else if (usetls) {
>   if (pledge("stdio rpath inet dns", NULL) == -1)
>   err(1, "pledge");
>   } else if (pledge("stdio inet dns", NULL) == -1)
>   err(1, "pledge");
> 
> So we can only reach the second group of pledges if usetls is set.
> Thus, I think the following diff would be better:
> 

OK with me as discussed on IRC.

Re: nc: missing rpath pledge for -P

[Adam Wolk]

On Fri, Jun 09, 2017 at 11:54:03PM +0200, Adam Wolk wrote:
> On Fri, Jun 09, 2017 at 09:28:29PM +, ra...@openmailbox.org wrote:
> > Hello!
> > 
> > Here is a patch with a pledge bugfix in netcat and some minor style
> > improvements.
> > 
> > An example of how to trigger the bug:
> > 
> > $ nc -Ptest -v -c blog.tintagel.pl 443
> > nc: pledge: Operation not permitted
> > 
> > credits to
> > * awolk@ for drawing attention to netcat.
> > * Juuso Lapinlampi for suggesting to alphabetically order the #includes.
> > * rajak for pointing out the missing space in the error message.
> > * brynet for pledge style improvements.
> > 
> > 
> 
> OK awolk@ for the updated diff (I'm attaching it inline).

forgot the diff
Index: usr.bin/nc/netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.178
diff -u -p -u -p -r1.178 netcat.c
--- usr.bin/nc/netcat.c 9 Mar 2017 13:58:00 -   1.178
+++ usr.bin/nc/netcat.c 9 Jun 2017 21:16:25 -
@@ -53,8 +53,8 @@
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include "atomicio.h"
 
 #define PORT_MAX   65535
@@ -340,7 +340,7 @@ main(int argc, char *argv[])
} else if (pledge("stdio inet dns sendfd", NULL) == -1)
err(1, "pledge");
} else if (Pflag) {
-   if (pledge("stdio inet dns tty", NULL) == -1)
+   if (pledge("stdio rpath inet dns tty", NULL) == -1)
err(1, "pledge");
} else if (usetls) {
if (pledge("stdio rpath inet dns", NULL) == -1)
@@ -461,9 +461,9 @@ main(int argc, char *argv[])
 
if (usetls) {
if (Pflag) {
-   if (pledge("stdio inet dns tty rpath", NULL) == -1)
+   if (pledge("stdio rpath inet dns tty", NULL) == -1)
err(1, "pledge");
-   } else if (pledge("stdio inet dns rpath", NULL) == -1)
+   } else if (pledge("stdio rpath inet dns", NULL) == -1)
err(1, "pledge");
 
if (tls_init() == -1)
@@ -492,7 +492,7 @@ main(int argc, char *argv[])
if (TLSopt & TLS_NOVERIFY) {
if (tls_expecthash != NULL)
errx(1, "-H and -T noverify may not be used"
-   "together");
+   " together");
tls_config_insecure_noverifycert(tls_cfg);
}
if (TLSopt & TLS_MUSTSTAPLE)

Re: nc: missing rpath pledge for -P

[Adam Wolk]

On Fri, Jun 09, 2017 at 09:28:29PM +, ra...@openmailbox.org wrote:
> Hello!
> 
> Here is a patch with a pledge bugfix in netcat and some minor style
> improvements.
> 
> An example of how to trigger the bug:
> 
> $ nc -Ptest -v -c blog.tintagel.pl 443
> nc: pledge: Operation not permitted
> 
> credits to
> * awolk@ for drawing attention to netcat.
> * Juuso Lapinlampi for suggesting to alphabetically order the #includes.
> * rajak for pointing out the missing space in the error message.
> * brynet for pledge style improvements.
> 
> 

OK awolk@ for the updated diff (I'm attaching it inline).
Would like a second OK on this.

Testing results, pre diff:
$ nc -H 123 -T noverify -c localhost 22 
nc: -H and -T noverify may not be usedtogether  
$ nc -Ptest -v -c blog.tintagel.pl 443  
nc: pledge: Operation not permitted 
$ 

Post diff:
$ ./nc -H 123 -T noverify -c localhost 22   
nc: -H and -T noverify may not be used together 
$ ./nc -Ptest -v -c blog.tintagel.pl 443
Connection to blog.tintagel.pl 443 port [tcp/https] succeeded!  

TLS handshake negotiated TLSv1.2/ECDHE-RSA-AES256-GCM-SHA384 with host 
blog.tintagel.pl 
Peer name: blog.tintagel.pl 
Subject: /CN=tintagel.pl
Issuer: /C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3 

Valid From: Thu Apr 27 04:01:00 2017
Valid Until: Wed Jul 26 04:01:00 2017   
Cert Hash: 
SHA256:1746b1d2ecdf8ad1fb7e06a6c97154b2c1a87eee65f5654824d0a0dc0af4ba98 
 
OCSP URL: http://ocsp.int-x3.letsencrypt.org/   
^C  
$

Test on amd64 -current

Regards,
Adam

doas: add confirm to prompt the user on what is to be executed

[Adam Wolk]

Hi tech@

This is a feture that came up in a chat I had with Kurt Mosiejczuk. I have been
recently reading source daily as a learning experience and decided that
implementing the feature we discussed would be a nice exercise.

The attached diff extends the configuration syntax with a new option 'confirm'
which when present on a rule triggers doas to print what command is about to
run, by whom and as what user with a question asking if the execution should
continue.

Rationale for adding this are scripts that shell out doas commands showing just
a prompt with no way for the user to tell what command he is authenticating.

with the following rule
permit confirm persist mulander as root

running a script that calls doas results in the following behavior:

$ sh test.sh
mulander wants to run 'whoami' as root. Continue? [yN]
doas: aborted by user

allowing the user to bail out or proceed. This re-uses variables already there
for syslog logging (except we don't use pwd as that would require moving getcwd
calls early and we had to introduce `first` for getchar checking).

brynet@ pointed out -n (non interactive mode), when -n is present we bail out
with the following message:

$ doas.new -n whoami
doas: Confirmation required

This email is a request for comment, roughly I want to know if others see this
feature as valuable. The diff currently lacks manpage changes, I will work on
those if the general decision is to include this feature.

I won't cry if we decide to drop this. I would have implemented it
anyways for fun :)

Regards,
Adam
Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.72
diff -u -p -r1.72 doas.c
--- doas.c  27 May 2017 09:51:07 -  1.72
+++ doas.c  8 Jun 2017 17:37:20 -
@@ -256,7 +256,7 @@ main(int argc, char **argv)
uid_t target = 0;
gid_t groups[NGROUPS_MAX + 1];
int ngroups;
-   int i, ch;
+   int i, ch, first;
int sflag = 0;
int nflag = 0;
char cwdpath[PATH_MAX];
@@ -355,6 +355,20 @@ main(int argc, char **argv)
syslog(LOG_AUTHPRIV | LOG_NOTICE,
"failed command for %s: %s", myname, cmdline);
errc(1, EPERM, NULL);
+   }
+
+   if (rule->options & CONFIRM) {
+   if (nflag)
+   errx(1, "Confirmation required");
+
+   printf("%s wants to run '%s' as %s. Continue? [yN] ",
+   myname, cmdline, pw->pw_name);
+   fflush(stdout);
+   first = ch = getchar();
+   while (ch != '\n' && ch != EOF)
+   ch = getchar();
+   if (first != 'y' && first != 'Y')
+   errx(1, "aborted by user");
}
 
if (!(rule->options & NOPASS)) {
Index: doas.h
===
RCS file: /cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.13
diff -u -p -r1.13 doas.h
--- doas.h  6 Apr 2017 21:12:06 -   1.13
+++ doas.h  8 Jun 2017 17:37:20 -
@@ -37,3 +37,5 @@ char **prepenv(const struct rule *);
 #define NOPASS 0x1
 #define KEEPENV0x2
 #define PERSIST0x4
+#define CONFIRM0x8
+
Index: parse.y
===
RCS file: /cvs/src/usr.bin/doas/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- parse.y 2 Jan 2017 01:40:20 -   1.26
+++ parse.y 8 Jun 2017 17:37:20 -
@@ -69,7 +69,7 @@ arraylen(const char **arr)
 
 %}
 
-%token TPERMIT TDENY TAS TCMD TARGS
+%token TPERMIT TDENY TAS TCMD TCONFIRM TARGS
 %token TNOPASS TPERSIST TKEEPENV TSETENV
 %token TSTRING
 
@@ -136,6 +136,9 @@ options:/* none */ {
 option:TNOPASS {
$$.options = NOPASS;
$$.envlist = NULL;
+   } | TCONFIRM {
+   $$.options = CONFIRM;
+   $$.envlist = NULL;
} | TPERSIST {
$$.options = PERSIST;
$$.envlist = NULL;
@@ -209,6 +212,7 @@ static struct keyword {
{ "cmd", TCMD },
{ "args", TARGS },
{ "nopass", TNOPASS },
+   { "confirm", TCONFIRM },
{ "persist", TPERSIST },
{ "keepenv", TKEEPENV },
{ "setenv", TSETENV },

Re: htpasswd: use crypt_newhash instead of bcrypt API

[Adam Wolk]

On Tue, Jun 06, 2017 at 08:29:23PM +, Florian Obser wrote:
> On Tue, Jun 06, 2017 at 08:49:32PM +0200, Adam Wolk wrote:
> > On Tue, Jun 06, 2017 at 12:28:59PM -0600, Theo de Raadt wrote:
> > > > The only thing against using automatic rounds would be having them 
> > > > guessed on a
> > > > weaker machine and used on a more powerful server - doubt though that 
> > > > would ever
> > > > pick something below 8 rounds.
> > > 
> > > I don't see the concern.  It has a lower bound.
> > 
> > Attaching the diff with rounds changed to auto, results with 9 rounds on my 
> > server.
> > 
> 
> OK florian@
> 

Committed, thanks!

Re: htpasswd: use crypt_newhash instead of bcrypt API

[Adam Wolk]

On Tue, Jun 06, 2017 at 12:28:59PM -0600, Theo de Raadt wrote:
> > The only thing against using automatic rounds would be having them guessed 
> > on a
> > weaker machine and used on a more powerful server - doubt though that would 
> > ever
> > pick something below 8 rounds.
> 
> I don't see the concern.  It has a lower bound.

Attaching the diff with rounds changed to auto, results with 9 rounds on my 
server.

? htpasswd
Index: htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.15
diff -u -p -r1.15 htpasswd.c
--- htpasswd.c  5 Nov 2015 20:07:15 -   1.15
+++ htpasswd.c  6 Jun 2017 18:46:39 -
@@ -47,7 +47,7 @@ int nagcount;
 int
 main(int argc, char** argv)
 {
-   char salt[_PASSWORD_LEN], tmpl[sizeof("/tmp/htpasswd-XX")];
+   char tmpl[sizeof("/tmp/htpasswd-XX")];
char hash[_PASSWORD_LEN], pass[1024], pass2[1024];
char *line = NULL, *login = NULL, *tok;
int c, fd, loginlen, batch = 0;
@@ -133,10 +133,8 @@ main(int argc, char** argv)
explicit_bzero(pass2, sizeof(pass2));
}
 
-   if (strlcpy(salt, bcrypt_gensalt(8), sizeof(salt)) >= sizeof(salt))
-   errx(1, "salt too long");
-   if (strlcpy(hash, bcrypt(pass, salt), sizeof(hash)) >= sizeof(hash))
-   errx(1, "hash too long");
+   if (crypt_newhash(pass, "bcrypt,a", hash, sizeof(hash)) != 0)
+   err(1, "can't generate hash");
explicit_bzero(pass, sizeof(pass));
 
if (file == NULL)

Re: htpasswd: use crypt_newhash instead of bcrypt API

[Adam Wolk]

On Tue, Jun 06, 2017 at 02:20:38PM -0400, Bryan Steele wrote:
> >  
> > -   if (strlcpy(salt, bcrypt_gensalt(8), sizeof(salt)) >= sizeof(salt))
> > -   errx(1, "salt too long");
> > -   if (strlcpy(hash, bcrypt(pass, salt), sizeof(hash)) >= sizeof(hash))
> > -   errx(1, "hash too long");
> > +   if (crypt_newhash(pass, "bcrypt,8", hash, sizeof(hash)) != 0)
> > +   err(1, "can't generate hash");
> > explicit_bzero(pass, sizeof(pass));
> >  
> > if (file == NULL)
> 
> It should be possible to use the automatic rouds, i.e: "bcrypt,a" instead
> of just hardcoding 8.
> 

I wasn't sure about introducing that change, went the minimal changes from
existing behavior.

The only thing against using automatic rounds would be having them guessed on a
weaker machine and used on a more powerful server - doubt though that would ever
pick something below 8 rounds.

Roughly, if the general consensus is to move to automatic rounds then I'm all
for it.

Side note, does anyone know why crypt_newhash defaults to blowfish? The
docs don't mention it.

htpasswd: use crypt_newhash instead of bcrypt API

[Adam Wolk]

Hi tech@

While reading htpasswd and htpasswd handling in httpd I noticed that both use
different APIs to handle encrypting/decrypting the passwords.

- htpasswd uses the bcrypt API
- httpd uses the new crypt API

The documentation for bcrypt states:
 These functions are deprecated in favor of crypt_checkpass(3) and
 crypt_newhash(3).

I'm attaching a diff moving htpasswd to the new API. Tested with httpd from
6.1 with a htpasswd generated with the diff applied on current.

Feedback? OK's?

Regards,
Adam
? htpasswd
Index: htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.15
diff -u -p -r1.15 htpasswd.c
--- htpasswd.c  5 Nov 2015 20:07:15 -   1.15
+++ htpasswd.c  6 Jun 2017 17:26:31 -
@@ -47,7 +47,7 @@ int nagcount;
 int
 main(int argc, char** argv)
 {
-   char salt[_PASSWORD_LEN], tmpl[sizeof("/tmp/htpasswd-XX")];
+   char tmpl[sizeof("/tmp/htpasswd-XX")];
char hash[_PASSWORD_LEN], pass[1024], pass2[1024];
char *line = NULL, *login = NULL, *tok;
int c, fd, loginlen, batch = 0;
@@ -133,10 +133,8 @@ main(int argc, char** argv)
explicit_bzero(pass2, sizeof(pass2));
}
 
-   if (strlcpy(salt, bcrypt_gensalt(8), sizeof(salt)) >= sizeof(salt))
-   errx(1, "salt too long");
-   if (strlcpy(hash, bcrypt(pass, salt), sizeof(hash)) >= sizeof(hash))
-   errx(1, "hash too long");
+   if (crypt_newhash(pass, "bcrypt,8", hash, sizeof(hash)) != 0)
+   err(1, "can't generate hash");
explicit_bzero(pass, sizeof(pass));
 
if (file == NULL)

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

On Sat, May 27, 2017 at 10:58:40PM +0100, Jason McIntyre wrote:
> On Sat, May 27, 2017 at 11:45:43PM +0200, Adam Wolk wrote:
> > Index: chown.8
> > ===
> > RCS file: /cvs/src/bin/chmod/chown.8,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 chown.8
> > --- chown.8 31 Dec 2015 23:38:16 -  1.20
> > +++ chown.8 27 May 2017 21:37:48 -
> > @@ -166,7 +166,12 @@ Previous versions of the
> >  utility used the dot
> >  .Pq Sq \&.
> >  character to distinguish the group name.
> > -This has been changed to be a colon
> > +This has been changed when the utility was first
> 
> s/has been/was/
> 
> > +standardised in
> > +.St -p1003.2-92
> > +to be a colon
> >  .Pq Sq \&:
> > -character so that user and
> > -group names may contain the dot character.
> > +character so that user and group names may contain the dot
> 
> s/may/could/
> or
> s/so that user and group names may/to allow user and group names to/
> 
> > +character, however the dot separator still remains supported
> 
> s/however/though/
> 
> > +due to widely required backwards compatibility.
> > +
> 
> jmc
> 

Thanks!

Included updated diffs with suggested changes applied.

Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 22:04:37 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 22:04:37 -
@@ -197,14 +197,16 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
-   /* UID and GID are separated by a dot and UID exists. */
+   /*
+* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever
+*/
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 -  1.20
+++ chown.8 27 May 2017 22:04:37 -
@@ -166,7 +166,11 @@ Previous versions of the
 utility used the dot
 .Pq Sq \&.
 character to distinguish the group name.
-This has been changed to be a colon
+This was changed when the utility was first standardised in
+.St -p1003.2-92
+to be a colon
 .Pq Sq \&:
-character so that user and
-group names may contain the dot character.
+character to allow user and group names to contain the dot
+character, though the dot separator still remains supported
+due to widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

On Sat, May 27, 2017 at 11:01:29PM +0200, Adam Wolk wrote:
> On Sat, May 27, 2017 at 01:42:45PM -0600, Theo de Raadt wrote:
> > I agree with you.  Maybe change the comment
> > 
> > /* UID and GID are separated by a dot and UID exists. */
> > 
> > to say a bit more on the matter, to prevent a zealot from arriving 2-3
> > years from now and proposing removal. Just a few words to hint . support
> > will stay forever.
> > 
> > It seems the sentences in the man page could be changed a bit.  Rather
> > than speaking about Previous versions, it could say POSIX (rev?)
> > deprecated '.' and introduced ':' as the default seperator, however '.'
> > seperator support remains for widely required backwards compat.  The current
> > sentences speak a bit too strongly about '.' actually being gone.
> > 
> > 
> 
> Updated the man page and expanded the comment in code.
> 
> Attaching updated diffs, OK?
> 

- style(9) the chmod.c comment
- use .St syntax to mark the standard in the man page instead of manually
  hard coding the name

both issues pointed out by brynet@, thanks!

Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 21:37:48 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 21:37:48 -
@@ -197,14 +197,16 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
-   /* UID and GID are separated by a dot and UID exists. */
+   /*
+* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever
+*/
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 -  1.20
+++ chown.8 27 May 2017 21:37:48 -
@@ -166,7 +166,12 @@ Previous versions of the
 utility used the dot
 .Pq Sq \&.
 character to distinguish the group name.
-This has been changed to be a colon
+This has been changed when the utility was first
+standardised in
+.St -p1003.2-92
+to be a colon
 .Pq Sq \&:
-character so that user and
-group names may contain the dot character.
+character so that user and group names may contain the dot
+character, however the dot separator still remains supported
+due to widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

On Sat, May 27, 2017 at 01:42:45PM -0600, Theo de Raadt wrote:
> I agree with you.  Maybe change the comment
> 
> /* UID and GID are separated by a dot and UID exists. */
> 
> to say a bit more on the matter, to prevent a zealot from arriving 2-3
> years from now and proposing removal. Just a few words to hint . support
> will stay forever.
> 
> It seems the sentences in the man page could be changed a bit.  Rather
> than speaking about Previous versions, it could say POSIX (rev?)
> deprecated '.' and introduced ':' as the default seperator, however '.'
> seperator support remains for widely required backwards compat.  The current
> sentences speak a bit too strongly about '.' actually being gone.
> 
> 

Updated the man page and expanded the comment in code.

Attaching updated diffs, OK?

Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 20:53:36 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 20:53:36 -
@@ -197,14 +197,14 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
-   /* UID and GID are separated by a dot and UID exists. */
+   /* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever */
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 -  1.20
+++ chown.8 27 May 2017 20:53:36 -
@@ -166,7 +166,12 @@ Previous versions of the
 utility used the dot
 .Pq Sq \&.
 character to distinguish the group name.
-This has been changed to be a colon
+This has been changed when the utility was first
+standardised in POSIX.2 (IEEE Std 1003.2-1992)
+to be a colon
 .Pq Sq \&:
 character so that user and
-group names may contain the dot character.
+group names may contain the dot character, however
+the dot separator still remains supported due to
+widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

Hi tech@,

I stumbled on SUPPORT_DOT while reading /usr/src/bin/chmod.c, got curious
and started doing some research.

POSIX changed the separator from . to : to make the utility properly work with
usernames containing a dot. The standard doesn't forbid keeping the dot handling
for backwards compatiblity.

The code is currently #ifdef'ed in. I assume the reason was to phase it out
sometime in the future.

The code was there and enabled with CFLAGS back in 1995
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/chown/Attic/Makefile?rev=1.1=text/x-cvsweb-markup

There were some attempts to weed it out but as far as I see they were abandonned
or stopped.

Back in 2001, by disabling the compat and trying to build the base system, no
followup email (that I can find): 
https://marc.info/?l=openbsd-tech=99647882113533=2

Discussion that brought SUPPORT_DOT into the topic. Mostly people argumenting if
man pages shold be altered (sorry, can't find the thread on marc.info):

http://misc.openbsd.narkive.com/4ejjhI6O/in-username

I think it's unlikely at this point that this backwards support will go away.
Linux, Mac, NetBSD and FreeBSD all support the compat, people seem to be using a
mix of both (including in our base where ie. /etc/netstart uses the dot
notation).

I suggest dropping the ifdef and define. It's been built enabled by default for
22 years. I'm also adding a diff for /etc/netstart to switch it to the :
separator. It's one less strchr call, though that obviously doesn't make
much difference performance wise in this case.

Feedback? OK's?
? chmod
? support_dot.diff
Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 18:39:17 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 18:39:17 -
@@ -197,14 +197,12 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
/* UID and GID are separated by a dot and UID exists. */
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: man.openbsd.org links on FAQ pages should point to -release

[Adam Wolk]

On Tue, Dec 06, 2016 at 07:46:31PM +0100, Adam Wolk wrote:
> Hi tech@
> 
> _gypcio on IRC reported that pkg_sign uses a -s signify flag that was renamed 
> in
> -current to signify2. The entry in the FAQ showing that example also linked 
> to a
> pkg_sign man page from -current which lead to the confusion.
> 
> Here is a diff generated with:
>   perl -pi.bak -e 
> 's|man.openbsd.org/(?!OpenBSD-6.0)|man.openbsd.org/OpenBSD-6.0/|g' faq*.html
> 
> that turns all links in faq*.html from untagged links resolving to -current to
> explicit OpenBSD-6.0. I did not alter the ports & pf faq's, current & upgrade
> guides. The regex skipps links already tagged with OpenBSD-6.0 but don't run 
> it
> blindly as it doens't protect from explicitly tagged 3.5, 3.6 etc (I did not 
> hit
> any in the faq*.html files).
> 
> Diff follows. On a side note it would be nice to have a OpenBSD-release
> shorthand on man.openbsd.org pointing to the latest release. That would help
> avoid a similar diff churn after 6.1 gets released.
> 
> Feedback? OK's?

A smaller diff was committed for the specific reported problem.
Pinging so people don't spend time going over a huge diff.

the commit: https://marc.info/?l=openbsd-cvs=148105690111936=2

Regards
Adam

Re: man.openbsd.org links on FAQ pages should point to -release

[Adam Wolk]

On Tue, Dec 06, 2016 at 07:46:31PM +0100, Adam Wolk wrote:
> Hi tech@
> 
> _gypcio on IRC reported that pkg_sign uses a -s signify flag that was renamed 
> in
> -current to signify2. The entry in the FAQ showing that example also linked 
> to a
> pkg_sign man page from -current which lead to the confusion.
> 
> Here is a diff generated with:
>   perl -pi.bak -e 
> 's|man.openbsd.org/(?!OpenBSD-6.0)|man.openbsd.org/OpenBSD-6.0/|g' faq*.html
> 
> that turns all links in faq*.html from untagged links resolving to -current to
> explicit OpenBSD-6.0. I did not alter the ports & pf faq's, current & upgrade
> guides. The regex skipps links already tagged with OpenBSD-6.0 but don't run 
> it
> blindly as it doens't protect from explicitly tagged 3.5, 3.6 etc (I did not 
> hit
> any in the faq*.html files).
> 
> Diff follows. On a side note it would be nice to have a OpenBSD-release
> shorthand on man.openbsd.org pointing to the latest release. That would help
> avoid a similar diff churn after 6.1 gets released.
> 
> Feedback? OK's?

I accidentally broke 2 explicit links against OpenBSD-current.
One in faq4 and one in faq8 - if the faq is against release I think they should
also be changed to OpenBSD-6.0.

man.openbsd.org links on FAQ pages should point to -release

[Adam Wolk]

Hi tech@

_gypcio on IRC reported that pkg_sign uses a -s signify flag that was renamed in
-current to signify2. The entry in the FAQ showing that example also linked to a
pkg_sign man page from -current which lead to the confusion.

Here is a diff generated with:
  perl -pi.bak -e 
's|man.openbsd.org/(?!OpenBSD-6.0)|man.openbsd.org/OpenBSD-6.0/|g' faq*.html

that turns all links in faq*.html from untagged links resolving to -current to
explicit OpenBSD-6.0. I did not alter the ports & pf faq's, current & upgrade
guides. The regex skipps links already tagged with OpenBSD-6.0 but don't run it
blindly as it doens't protect from explicitly tagged 3.5, 3.6 etc (I did not hit
any in the faq*.html files).

Diff follows. On a side note it would be nice to have a OpenBSD-release
shorthand on man.openbsd.org pointing to the latest release. That would help
avoid a similar diff churn after 6.1 gets released.

Feedback? OK's?
Index: faq1.html
===
RCS file: /cvs/www/faq/faq1.html,v
retrieving revision 1.201
diff -u -p -r1.201 faq1.html
--- faq1.html   24 Sep 2016 03:22:12 -  1.201
+++ faq1.html   6 Dec 2016 18:31:28 -
@@ -311,7 +311,7 @@ Some of the more popular lists are:
   announce - Project announcements and errata notices.
   This is a low-volume list.
   bugs - Bugs received via
-  http://man.openbsd.org/sendbug;>sendbug(1)
+  http://man.openbsd.org/OpenBSD-6.0/sendbug;>sendbug(1)
   and discussion about them.
   misc - General user questions and answers.
   This is the most active list, and should be the "default" for most
@@ -332,7 +332,7 @@ While it might be the first time you hav
 others on the mailing lists may have seen the same question several times in 
the
 last week, and may not appreciate seeing it again.
 If asking a question possibly related to hardware, always include a full
-http://man.openbsd.org/dmesg;>dmesg(8).
+http://man.openbsd.org/OpenBSD-6.0/dmesg;>dmesg(8).
 
 
 You can find several archives, other guidelines and more information on the
@@ -351,30 +351,30 @@ The man pages are the authoritative sour
 Here is a list of some useful manual pages for new users:
 
 
-  http://man.openbsd.org/afterboot;>afterboot(8)
+  http://man.openbsd.org/OpenBSD-6.0/afterboot;>afterboot(8)
   - things to check after the first complete boot.
-  http://man.openbsd.org/help;>help(1)
+  http://man.openbsd.org/OpenBSD-6.0/help;>help(1)
   - help for new users and administrators.
-  http://man.openbsd.org/hier;>hier(7)
+  http://man.openbsd.org/OpenBSD-6.0/hier;>hier(7)
   - layout of filesystems.
-  http://man.openbsd.org/man;>man(1)
+  http://man.openbsd.org/OpenBSD-6.0/man;>man(1)
   - display the manual pages.
-  http://man.openbsd.org/adduser;>adduser(8)
+  http://man.openbsd.org/OpenBSD-6.0/adduser;>adduser(8)
   - add new users.
-  http://man.openbsd.org/reboot;>reboot(8), halt(8) and
-  http://man.openbsd.org/shutdown;>shutdown(8)
+  http://man.openbsd.org/OpenBSD-6.0/reboot;>reboot(8), 
halt(8) and
+  http://man.openbsd.org/OpenBSD-6.0/shutdown;>shutdown(8)
   - stop and restart the system.
-  http://man.openbsd.org/dmesg;>dmesg(8)
+  http://man.openbsd.org/OpenBSD-6.0/dmesg;>dmesg(8)
   - redisplay the kernel boot messages.
-  http://man.openbsd.org/doas;>doas(1)
+  http://man.openbsd.org/OpenBSD-6.0/doas;>doas(1)
   - don't log in as root, but run commands as root.
-  http://man.openbsd.org/tmux;>tmux(1)
+  http://man.openbsd.org/OpenBSD-6.0/tmux;>tmux(1)
   - terminal multiplexer.
-  http://man.openbsd.org/ifconfig;>ifconfig(8)
+  http://man.openbsd.org/OpenBSD-6.0/ifconfig;>ifconfig(8)
   - configure network interface parameters.
-  http://man.openbsd.org/login.conf;>login.conf(5)
+  http://man.openbsd.org/OpenBSD-6.0/login.conf;>login.conf(5)
   - format of the login class configuration file.
-  http://man.openbsd.org/sendbug;>sendbug(1)
+  http://man.openbsd.org/OpenBSD-6.0/sendbug;>sendbug(1)
   - report a bug you've found.
 
 
@@ -440,10 +440,10 @@ Replace ps with pdf if
 What are info files?
 
 Some of the documentation for OpenBSD comes in the form of
-http://man.openbsd.org/info;>info(1) files.
+http://man.openbsd.org/OpenBSD-6.0/info;>info(1) files.
 This is an alternative form of documentation provided by GNU.
 For example, to view information about the GNU compiler,
-http://man.openbsd.org/gcc;>gcc(1), type:
+http://man.openbsd.org/OpenBSD-6.0/gcc;>gcc(1), type:
 
 
 $ info gcc
@@ -453,7 +453,7 @@ After using info, you will really apprec
 
 How do I write my own manual page?
 
-Consult http://man.openbsd.org/mdoc;>mdoc(7).
+Consult http://man.openbsd.org/OpenBSD-6.0/mdoc;>mdoc(7).
 
 Reporting bugs
 
@@ -495,14 +495,14 @@ See this pagehttp://man.openbsd.org/sendbug;>sendbug(1) to report
+Please use http://man.openbsd.org/OpenBSD-6.0/sendbug;>sendbug(1) 
to report
 your problems whenever possible, otherwise please include at least the
-http://man.openbsd.org/dmesg;>dmesg(8) output of your system.

merge usbd_open_pipe.9 & usbd_close_pipe.9 into a single manpage

[Adam Wolk]

Hi tech@,

I have been going through usbdi recently and I believe that the mentioned
manpages can be merged into a single one since they operate on the same
abstraction in the interface.

I am cross referrencing with NetBSD which recently added documentation for the
usbdi interface:
  - https://man-k.org/man/netbsd/9/usbdi?r=1=usb_rem_task

Feedback? OK's?

Regards,
Adam
Index: Makefile
===
RCS file: /cvs/src/share/man/man9/Makefile,v
retrieving revision 1.280
diff -u -p -r1.280 Makefile
--- Makefile5 Sep 2016 07:22:29 -   1.280
+++ Makefile11 Sep 2016 17:51:33 -
@@ -33,7 +33,7 @@ MAN=  aml_evalnode.9 atomic_add_int.9 ato
spl.9 srp_enter.9 srpl_rc_init.9 startuphook_establish.9 \
socreate.9 sosplice.9 style.9 syscall.9 sysctl_int.9 \
task_add.9 tc_init.9 time_second.9 timeout.9 tsleep.9 tvtohz.9 \
-   uiomove.9 uvm.9 usbd_close_pipe.9 usbd_open_pipe.9 usbd_ref_wait.9 \
+   uiomove.9 uvm.9 usbd_open_pipe.9 usbd_ref_wait.9 \
usbd_transfer.9 vfs.9 vfs_busy.9 \
vfs_cache.9 vaccess.9 vclean.9 vcount.9 vdevgone.9 vfinddev.9 vflush.9 \
vflushbuf.9 vget.9 vgone.9 vhold.9 vinvalbuf.9 vnode.9 vnsubr.9 \
Index: usbd_close_pipe.9
===
RCS file: usbd_close_pipe.9
diff -N usbd_close_pipe.9
--- usbd_close_pipe.9   4 May 2015 10:12:34 -   1.1
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,48 +0,0 @@
-.\" $OpenBSD: usbd_close_pipe.9,v 1.1 2015/05/04 10:12:34 mpi Exp $
-.\"
-.\" Copyright (c) 2015 Sean Levy 
-.\"
-.\" Permission to use, copy, modify, and distribute this software for any
-.\" purpose with or without fee is hereby granted, provided that the above
-.\" copyright notice and this permission notice appear in all copies.
-.\"
-.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
-.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
-.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-.\"
-.Dd $Mdocdate: May 4 2015 $
-.Dt USBD_CLOSE_PIPE 9
-.Os
-.Sh NAME
-.Nm usbd_close_pipe , usbd_abort_pipe
-.Nd delete or abort transfers on a USB pipe
-.Sh SYNOPSIS
-.In dev/usb/usb.h
-.In dev/usb/usbdi.h
-.Ft usbd_status
-.Fn usbd_close_pipe "struct usbd_pipe *pipe"
-.Ft usbd_status
-.Fn usbd_abort_pipe "struct usbd_pipe *pipe"
-.Sh DESCRIPTION
-The
-.Fn usbd_abort_pipe
-function aborts any transfers queued on
-.Fa pipe .
-.Pp
-The
-.Fn usbd_close_pipe
-function aborts any transfers queued on
-.Fa pipe
-then deletes it.
-.Sh CONTEXT
-.Fn usbd_abort_pipe
-and
-.Fn usbd_close_pipe
-can be called during autoconf or from process context.
-.Sh SEE ALSO
-.Xr usb 4 ,
-.Xr usbd_open_pipe 9
Index: usbd_open_pipe.9
===
RCS file: /cvs/src/share/man/man9/usbd_open_pipe.9,v
retrieving revision 1.4
diff -u -p -r1.4 usbd_open_pipe.9
--- usbd_open_pipe.927 Jun 2016 23:38:01 -  1.4
+++ usbd_open_pipe.911 Sep 2016 17:51:33 -
@@ -18,8 +18,11 @@
 .Dt USBD_OPEN_PIPE 9
 .Os
 .Sh NAME
-.Nm usbd_open_pipe , usbd_open_pipe_intr
-.Nd create USB pipe
+.Nm usbd_open_pipe ,
+.Nm usbd_open_pipe_intr ,
+.Nm usbd_close_pipe ,
+.Nm usbd_abort_pipe
+.Nd manage USB transfer pipes
 .Sh SYNOPSIS
 .In dev/usb/usb.h
 .In dev/usb/usbdi.h
@@ -27,16 +30,21 @@
 .Fn usbd_open_pipe "struct usbd_interface *iface" "uint8_t address" "uint8_t 
flags" "struct usbd_pipe **pipe"
 .Ft usbd_status
 .Fn usbd_open_pipe_intr "struct usbd_interface *iface" "uint8_t address" 
"uint8_t flags" "struct usbd_pipe **pipe" "void *priv" "void *buffer" "uint32_t 
len" "usbd_callback cb" "int ival"
+.Ft usbd_status
+.Fn usbd_close_pipe "struct usbd_pipe *pipe"
+.Ft usbd_status
+.Fn usbd_abort_pipe "struct usbd_pipe *pipe"
 .Sh DESCRIPTION
 The
-.Fn usbd_open_pipe
-and
+.Fn usbd_open_pipe ,
 .Fn usbd_open_pipe_intr
-functions create pipes.
+and
+.Fn usbd_close_pipe
+functions create and destroy pipes.
 A pipe is a logical connection between the host and an endpoint on a
 USB device.
 USB drivers use pipes to manage transfers to or from a USB
-endpoint.
+endpoint. It is common to have more than one pipe per device.
 .Pp
 The
 .Fn usbd_open_pipe
@@ -121,12 +129,22 @@ transfers maintained by the pipe, unlike
 continue to be processed every
 .Fa ival
 milliseconds.
+.Pp
+.Fn usbd_close_pipe
+function aborts any transfers queued on
+.Fa pipe .
+.Pp
+.Fn usbd_abort_pipe
+function aborts any transfers queued on
+.Fa pipe
+then delets it.
 .Sh CONTEXT
-.Fn usbd_open_pipe
+.Fn usbd_open_pipe ,
+.Fn usbd_open_pipe_intr ,
+.Fn 

mg - fix modeline segfault

[Adam Wolk]

Hi tech@

attaching a fix for the following crash caused by a null pointer dereference
while the modeline is trying to work on a unusable display

#0  0x0bf6a4e04433 in modeline (wp=0xbf948d9d400, modelinecolor=2) at 
display.c:800
800 vscreen[n]->v_color = modelinecolor;/* Mode line color. 
 */
(gdb) bt
#0  0x0bf6a4e04433 in modeline (wp=0xbf948d9d400, modelinecolor=2) at 
display.c:800
#1  0x0bf6a4e04ecf in update (modelinecolor=2) at display.c:501
#2  0x0bf6a4e0ee28 in main (argc=Variable "argc" is not available.
) at main.c:199

quite easy to reproduce:
 1. start a tmux session
 2. split the screen in half (^B ")
 3. start mg in one screen
 4. resize the mg screen to 2 lines (smallest allow by tmux)
 5. by now tmux should be showing unusable display
 6. type something or do a modeline command
segfault.

The interesting thing is that mg works without a crash if it's started from
a 2 line display regardless of what you do. So I am having doubts how sane
that check for 'unusable' display is.

I also assume there might be more places that die when trying to work
with an unusable display (I didn't find/hit them yet).

Thinking about it made me try another diff. Which removes the 'window is 
unusable'
check completely. So far I havent seen a single crash with it and I can resize
the window down to 2 lines and back.

I guess I'm asking for an OK for the second diff (or a reason why we should not)
versus OK'ing the first one :)

Regards,
awolk
Index: display.c
===
RCS file: /cvs/src/usr.bin/mg/display.c,v
retrieving revision 1.47
diff -u -p -r1.47 display.c
--- display.c   3 Apr 2015 22:10:29 -   1.47
+++ display.c   6 Sep 2016 21:15:51 -
@@ -797,6 +797,8 @@ modeline(struct mgwin *wp, int modelinec
int len;
 
n = wp->w_toprow + wp->w_ntrows;/* Location. */
+   if (!vscreen[n])
+   return;
vscreen[n]->v_color = modelinecolor;/* Mode line color.  */
vscreen[n]->v_flag |= (VFCHG | VFHBAD); /* Recompute, display.   */
vtmove(n, 0);   /* Seek to right line.   */
Index: window.c
===
RCS file: /cvs/src/usr.bin/mg/window.c,v
retrieving revision 1.36
diff -u -p -r1.36 window.c
--- window.c18 Nov 2015 18:21:06 -  1.36
+++ window.c6 Sep 2016 21:29:48 -
@@ -89,12 +89,6 @@ do_redraw(int f, int n, int force)
while (wp->w_wndp != NULL)
wp = wp->w_wndp;
 
-   /* check if too small */
-   if (nrow < wp->w_toprow + 3) {
-   dobeep();
-   ewprintf("Display unusable");
-   return (FALSE);
-   }
wp->w_ntrows = nrow - wp->w_toprow - 2;
sgarbf = TRUE;
update(CMODE);

Re: mg - Check pointer before calling showbuffer()

[Adam Wolk]

On Tue, Sep 06, 2016 at 05:10:39PM +, Mark Lumsden wrote:
> Source Joachim Nilsson:
> 
> Found by Coverity Scan.  The popbuf() function iterated over a list to
> find a wp pointer, then sent it to showbuffer() which immediately went
> ahead and dereferenced it.  This patch simply adds a NULL pointer check
> before calling showbuffer(), if NULL then just return NULL to callee.
> 
> The missing NULL check is actually referenced in a comment a few lines
> earlier in the code. ok?
> 
> -lum
> 

I tested the diff and that's OK awolk@ with a slight suggestion to also
grab the for loop with { } since you are already adding it for the
dangling else.

> Index: buffer.c
> ===
> RCS file: /cvs/src/usr.bin/mg/buffer.c,v
> retrieving revision 1.101
> diff -u -p -u -p -r1.101 buffer.c
> --- buffer.c  31 Aug 2016 12:22:28 -  1.101
> +++ buffer.c  6 Sep 2016 17:04:22 -
> @@ -713,12 +713,16 @@ popbuf(struct buffer *bp, int flags)
>  
>   while (wp != NULL && wp == curwp)
>   wp = wp->w_wndp;
> - } else
> + } else {
>   for (wp = wheadp; wp != NULL; wp = wp->w_wndp)
>   if (wp->w_bufp == bp) {
>   wp->w_rflag |= WFFULL | WFFRAME;
>   return (wp);
>   }
> + }
> + if (!wp)
> + return (NULL);
> +
>   if (showbuffer(bp, wp, WFFULL) != TRUE)
>   return (NULL);
>   return (wp);
> 

pledge: telnet should not verify if hostname is a fully qualified domain

[Adam Wolk]

Hi tech@,

I have been noticing coredumps from telnet on my laptop for some time
now and finally found an evening to investigate it.

The typical use case:

$ telnet localhost 22
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
SSH-2.0-OpenSSH_7.2
^]
telnet> quit
Connection closed.
Abort trap (core dumped) 
$ 

Plus the following in dmesg:
telnet(67078): syscall 97 "dns"

The bug was reproducible by me both by calling quit or close in the
telnet> prompt but no one else I asked was able to reproduce it.

Rebuilding the code with debug symbols and grabbing the backtrace
revealed this fine piece of code:

/* If this is not the full name, try to get it via DNS */
if (strchr(hbuf, '.') == 0) {
struct hostent *he = gethostbyname(hbuf);
if (he != 0)
strncpy(hbuf, he->h_name, sizeof hbuf-1);
hbuf[sizeof hbuf-1] = '\0';
}

Full backtrace: 
https://gist.github.com/mulander/392bce616de89830f64aaf72b9cab56d

Which was added in 12-March-98 by art@ while adding encryption support
from kth-krb (kerberos only) plus doing some tweaks for better
binary/8-bit support
(http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/telnet/commands.c#rev1.10).

The reason for entering that code path is me having a not fully
qualified name for my host. Setting up a proper name (napalm.local
instead of napalm) makes telnet happy again. Regardless I don't see a
reason why telnet should be doing this check. Here is the rationale:

- It's not performed and required on initial run (either by running
telnet + telnet> open host port or by running telnet host port
directly)
- It breaks the pledge assumption of not needing DNS after the
  connection is established

I would like to just drop that part of code. Any OK's, comments?

Index: commands.c
===
RCS file: /cvs/src/usr.bin/telnet/commands.c,v
retrieving revision 1.83
diff -u -p -r1.83 commands.c
--- commands.c  16 Mar 2016 15:41:11 -  1.83
+++ commands.c  3 May 2016 00:24:51 -
@@ -1445,14 +1445,6 @@ env_init(void)
 
gethostname(hbuf, sizeof hbuf);
 
-   /* If this is not the full name, try to get it via DNS */
-   if (strchr(hbuf, '.') == 0) {
-   struct hostent *he = gethostbyname(hbuf);
-   if (he != 0)
-   strncpy(hbuf, he->h_name, sizeof hbuf-1);
-   hbuf[sizeof hbuf-1] = '\0';
-   }
-
if (asprintf (, "%s%s", hbuf, cp2) == -1)
err(1, "asprintf");
 

Re: Firefox, malloc(3) and threads

[Adam Wolk]

On Fri, 22 Jan 2016 22:46:39 +0100 (CET)
Mark Kettenis  wrote:

> Firefox makes a lot of concurrent malloc(3) calls.  The locking to
> make malloc(3) thread-safe is a bit...suboptimal.  This diff makes
> things better by using a mutex instead of spinlock.  If you're running
> Firefox you want to try it; it makes video watchable on some machines.
> If you're not running Firefox you want to try it; to make sure it
> doesn't break things.
> 
> Enjoy,
> 
> Mark
>  '

Applied to a Jan 15h snapshot sources. Youtube is not fully 'watchable'
on firefox but feels significantly better. I can also now watch full
screen youtube videos on chromium 1920x1080 with no stutter (lenovo
g50-70).

Generally gnome 3 feels a bit snappier especially on first load,
bringing up the menu searching for 'terminal' leads to a faster
rendering of the results. This might be just 'imagined' by me.

On a more measurable front. I ran the octane benchmark against firefox
post and before the patch. It resulted in a slight improvement from
12486 to 12826 score [1].

cpu0: Intel(R) Core(TM) i7-4510U CPU @ 2.00GHz, 1895.93 MHz
cpu1: Intel(R) Core(TM) i7-4510U CPU @ 2.00GHz, 1895.62 MHz
cpu2: Intel(R) Core(TM) i7-4510U CPU @ 2.00GHz, 1895.62 MHz
cpu3: Intel(R) Core(TM) i7-4510U CPU @ 2.00GHz, 1895.62 MHz
inteldrm0 at pci0 dev 2 function 0 "Intel HD Graphics" rev 0x0b
running Intel Haswell Mobile for the gfx card. 

Regards,
Adam

[1] - https://twitter.com/mulander/status/691327370985345024

Re: [patch] PkgCreate.pm make it more clear why a shared library is invalid

[Adam Wolk]

On Thu, 12 Nov 2015 16:15:35 +0100
Marc Espie <es...@nerim.net> wrote:

> On Wed, Nov 11, 2015 at 05:13:45PM +0100, Adam Wolk wrote:
> > Hi tech@,
> > 
> > I have been working recently on packaging a shared library for the
> > first time and hit a stumbling block yesterday.
> > 
> > $ make package
> > `/usr/ports/pobj/libwebsockets-1.5/fake-amd64/.fake_done' is up to
> > date. ===>  Building package for libwebsockets-1.5  
> > Create /usr/ports/packages/amd64/all/libwebsockets-1.5.tgz
> > reading plist|
> > Error: Invalid shared library @lib lib/libwebsockets.so.5  
> 
> I'm pretty sure the naming scheme is out of place in the error
> message. But yeah, the message is not perfect.

Maybe then just adding a name would help here? It is a bit difficult to
handle since the code checks both that the file matches the naming
scheme & that it's located in at least one sub-folder.

Leaving it up to you. I know what to do next time I spot that. Thought
it would be easier for someone else to understand the error when hit.

Regards,
Adam

Index: OpenBSD/PkgCreate.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm,v
retrieving revision 1.118
diff -u -p -r1.118 PkgCreate.pm
--- OpenBSD/PkgCreate.pm6 Nov 2015 08:53:12 -   1.118
+++ OpenBSD/PkgCreate.pm11 Nov 2015 16:07:33 -
@@ -674,7 +674,7 @@ sub check_version
$state->error("Incorrectly versioned shared library: 
#1", $unsubst);
}
} else {
-   $state->error("Invalid shared library #1", $unsubst);
+   $state->error("Invalid shared library name #1", $unsubst);
}
$state->{has_libraries} = 1;
 }

[patch] PkgCreate.pm make it more clear why a shared library is invalid

[Adam Wolk]

Hi tech@,

I have been working recently on packaging a shared library for the
first time and hit a stumbling block yesterday.

$ make package
`/usr/ports/pobj/libwebsockets-1.5/fake-amd64/.fake_done' is up to date.
===>  Building package for libwebsockets-1.5
Create /usr/ports/packages/amd64/all/libwebsockets-1.5.tgz
reading plist|
Error: Invalid shared library @lib lib/libwebsockets.so.5

This error message made me think that I either generated the .so
incorrectly or that it's format is not a valid shared library. I went
to bed and tackled the issue again today.

Quick look at PkgCreate revealed that the error is generated by
PkgCreate in check_version:

 
https://github.com/lattera/openbsd/blob/master/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm#L618

The error is a result of $self->parse returning undef as it's result.
Parse resolves to:

 
https://github.com/lattera/openbsd/blob/master/usr.sbin/pkg_add/OpenBSD/PackingElement.pm#L659

and the result is just based on a regexp match & extract:
 $filename =~ m/^(.*?)\/?lib([^\/]+)\.so\.(\d+)\.(\d+)$/o

Now in my case after seeing the above it is obvious that the packaging
fails due to the lib not following lib$NAME.so.$major.$minor naming
scheme.

I would like to suggest a small change to the reported error to make
the cause of the problem more apparent.

Current : Error: Invalid shared library @lib lib/libwebsockets.so.5
Proposed: Error: Shared library name doesn't match
lib/lib$name.so.$major.$minor naming scheme @lib lib/libwebsockets.so.5

I did confirm that not existing files are handeled later on:

$ make package
`/usr/ports/pobj/libwebsockets-1.5/fake-amd64/.fake_done' is up to date.
===>  Building package for libwebsockets-1.5
Create /usr/ports/packages/amd64/all/libwebsockets-1.5.tgz
checksumming|**
| 91%
Error: 
/usr/ports/pobj/libwebsockets-1.5/fake-amd64/usr/local/lib/libwebsockets.so.5.0
does not exist Fatal error: can't continue


of course the error message is just a suggestion, maybe someone can
come up with a better/less verbose one.

Here's the output after the patch:
# make package
`/usr/ports/pobj/libwebsockets-1.5/fake-amd64/.fake_done' is up to date.
===>  Building package for libwebsockets-1.5
Create /usr/ports/packages/amd64/all/libwebsockets-1.5.tgz
reading plist|
Error: Shared library name doesn't match lib/lib$name.so.$major.$minor
naming scheme @lib lib/libwebsockets.so.5

checking dependencies...
checksumming
Fatal error: can't continue
 at /usr/libdata/perl5/OpenBSD/PkgCreate.pm line 1543.
*** Error 1 in . (/usr/ports/infrastructure/mk/bsd.port.mk:1959
'/usr/ports/packages/amd64/all/libwebsockets-1.5.tgz') *** Error 1 in .
(/usr/ports/infrastructure/mk/bsd.port.mk:2511 '_internal-package') ***
Error 1 in /usr/ports/mystuff/devel/libwebsockets
(/usr/ports/infrastructure/mk/bsd.port.mk:2491 'package')


Small offtopic question before the patch:
I noticed that the regexp in PackingElement.pm uses the /o flag.
Actually a lot of regexp in pkg_add subtree use them. The perlre(1)
docs have this to say about this flag:

   Substitution-specific modifiers described in

   "s/PATTERN/REPLACEMENT/msixpodualgcer" in perlop are:
 o  - pretend to optimize your code, but actually introduce
   bugs

Quick search on perlmonks yields:
http://www.perlmonks.org/?node_id=256053

Guess it's mostly harmless just wanted to know if there are plans to
kill them off?

Error message patch:

Index: OpenBSD/PkgCreate.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm,v
retrieving revision 1.118
diff -u -p -r1.118 PkgCreate.pm
--- OpenBSD/PkgCreate.pm6 Nov 2015 08:53:12 -   1.118
+++ OpenBSD/PkgCreate.pm11 Nov 2015 16:07:33 -
@@ -674,7 +674,7 @@ sub check_version
$state->error("Incorrectly versioned shared library: 
#1", $unsubst);
}
} else {
-   $state->error("Invalid shared library #1", $unsubst);
+   $state->error("Shared library name doesn't match 
lib/lib\$name.so.\$major.\$minor naming scheme #1", $unsubst);
}
$state->{has_libraries} = 1;
 }

file(1) no longer tells if a file is stripped (www/faq/ports/guide.html patch)

[Adam Wolk]

Hi tech@

http://www.openbsd.org/faq/ports/guide.html still suggests that file(1)
can be used to determine whether a file was stripped or not:

> Install the application with make fake. Libraries should never be
> stripped. Executables are stripped by default; this is governed by
> ${INSTALL_STRIP}. ${INSTALL_PROGRAM} honors this automatically and is
> preferable to unconditional stripping (e.g., by an install-strip
> target or by running strip from the Makefile). You can use file(1) to
> determine if a binary is stripped or not.

This is no longer true after the recent file(1) rewrite.

$ ls -alh camcell*
-rwxr-xr-x  1 mulander  mulander   127K Nov 10 20:43 camcell
-rwxr-xr-x  1 mulander  mulander  47.3K Nov 10 21:04 camcell_stripped
$ file camcell*
camcell:  ELF 64-bit LSB shared object, x86-64, version 1
camcell_stripped: ELF 64-bit LSB shared object, x86-64, version 1

I first wanted to write a patch for the docs to point to objdump(1)
with --syms flag but saw it stating:

>   --syms
>   Print the symbol table entries of the file.  This is similar
>   to the information provided by the nm program.

So I tried nm(1) but unfortunately even though a stripped binary on
Linux reports with nm(1):

mulander@inferno:~$ nm camcell_openbsd_stripped 
nm: camcell_openbsd_stripped: no symbols

On OpenBSD it still provides a lot of debugging symbols from shared
libs (as expected).

Hence I rewrote the docs to use objdump(1) with the --syms flag which
reports if the provided input binary was stripped of symbols like
initially intended.

Regards,
Adam Wolk

Index: guide.html
===
RCS file: /cvs/www/faq/ports/guide.html,v
retrieving revision 1.38
diff -u -p -r1.38 guide.html
--- guide.html  30 Jul 2015 10:56:41 -  1.38
+++ guide.html  10 Nov 2015 20:11:44 -
@@ -464,7 +464,8 @@ this is governed by ${INSTALL_STRIP}
 ${INSTALL_PROGRAM} honors this automatically and is preferable to
 unconditional stripping (e.g., by an install-strip target or by
 running strip from the Makefile).
-You can use file(1) to determine if a binary is stripped or not.
+You can use objdump(1) --syms to determine if a binary is stripped or not.
+Stripped files have no symbols in the SYMBOL TABLE.
 
 
 Check port for security holes again. This is

Re: Unlock the reaper

[Adam Wolk]

On Wed, 8 Jul 2015 22:20:49 +0100
Stuart Henderson st...@openbsd.org wrote:

 On 2015/07/08 20:00, Max Fillinger wrote:
  On Wed, Jul 08, 2015 at 03:53:46PM +0200, Mark Kettenis wrote:
   I'm looking for testers for this diff.  This should be safe to
   run on amd64, i386 and sparc64.  But has been reported to lock up
   i386 machines.  I can't reproduce this on any of my own systems.
   So I'm looking for help.  I'm looking for people that are able to
   build a kernel with this diff and the MP_LOCKDEBUG option enabled
   (uncommented) in their GENERIC.MP kernel, run it on an MP machine
   and put some load on it to see if it locks up and/or panics.
   
   Being able to move forward with this would make OpenBSD run
   significantly better on MP systems.
   
   Thanks,
   
   Mark
  
  I just finished compiling the kernel for amd64; I might test i386
  later. What kind of load would be required to give useful feedback?
  Would building the userland or some of the bigger ports be a useful
  test?
 

I have been running with the patch for ~2h with a decent load on amd64
applied to a Jul 8 snapshot. No crashes, no panics and dmesg doesn't
show anything unusual.

 Building base with the reaper unlock diff on i386 doesn't seem to
 trigger problems, or at least I haven't run into them in a few
 attempts.
 
 I do see problems when building ports on a dpb cluster, quite quickly
 in some cases - I just did a run and one node locked after 261s,
 another after 756s (dpb master stayed up FWIW).
 

Stuart, do you think the issue could be memory related and more easily
triggered when the system is forced to swap?

My amd64 box has 8 gigs of ram though the i386 device I used before was
always a *lot* easier to memory starve to say the least. The patches
apply to the uvm which is responsible handling virtual memory 
swapping.

That could be a good reason for a hard to reproduce bug on a memory
starved i386 device wouldn't it?

I can slap a new snaphost on my old i386 box and try memory starving it
tomorrow. Will appreciate any debunking of this train thought of course
or pointers on what type of load to put on the system (cpu, memory,
io?).

 If you're trying to reproduce, make sure you set ddb.console=1 and
 check that you can break into ddb under normal conditions. If you
 manage to trigger a hang, see if you can break into ddb and get
 the usual things (backtrace, ps, sh reg, etc).
 
 I've been unable to get into ddb after a hang, including on this
 most recent run with MP_LOCKDEBUG.
 
 Nothing particular special was being built during the last hang;
 from dpb term-report, the last entry before i386-2- appeared
 (indicating that the host is no longer contactable) showed these
 
 archivers/libzip
 audio/libogg
 archivers/lzo2
 
 Looking at build logs (which are streamed over ssh and logged
 on the dpb master) lzo2 and libzip were compiling (cc from base)
 and libogg was doing pkg_create/gzip when contact was lost.
 So I don't think it's going to be triggered by any particular
 ports, there is nothing out of the ordinary about these, and
 no funny autoconf checks were occurring at the time.
 
 The main other build-related active process would be sshd,
 and since pkg_create was running it would also most likely
 have been writing to nfs at the time.

Re: [Patch] New item to the Migrating to OpenBSD guide

[Adam Wolk]

On Sun, 28 Jun 2015 19:55:58 +0200
Denis Fondras open...@ledeuns.net wrote:

  This patch is regarding the fact that there are no binary updates,
  which is a given thing
  
 
 What you missed : https://stable.mtier.org/

What do you mean? The author mentioned mtier.org both in his original
blog post and the patch sent to this mailing list.

Regarding the patch itself:

+a href=../stable-stable/a. Binary updates may be obtained
+from a href=https://stable.mtier.org;a third party/a for the i386
+and amd64 architectures./li

If it's going to be merged then it's probably worth to mention that
some OpenBSD developers work for mtier directly. Each time mtier is
mentioned someone is deemed to chime in with but I don't trust them
even though the same people commit code to the base OS...

Regards,
Adam

Re: [Patch] httpd - don't leak fcgi file descriptors

[Adam Wolk]

On Sun, 31 May 2015 19:25:22 -0400
Todd Mortimer t...@opennet.ca wrote:

Hi tech@,

 Hi Joerg,
 
 Thanks for getting back to me. 
 
 I cloned the server and upgraded it to the 31 May snapshot, did the
 sysmerge and upgraded the packages to the snapshot versions.
 
 The behaviour is still there. It actually appears to be somewhat
 more pronounced, and php-fpm hits max_children more quickly than
 it does under 5.7-stable. The same patch prevents the php-fpm
 processes from going idle on netio, and I have reproduced it against
 -current below.
 
 It also seems that httpd on -current has more parallel connections
 open to php-fpm at once compared to the same setup on 5.7-stable.
 

I have been following this thread since the initial report.
I'm also running owncloud (+ roundcube for mail) on an OpenBSD
-current amd64 server (snapshot from May 20).

I have been hitting the exact same issue but initially I accounted
it for just not tweaking the settings.

My server is really low on traffic (only two users  rarely concurrent).

I can confirm that after getting to max_children limit the server starts
returning error 500 on each request (both on roundcube  owncloud) until
php-fpm or httpd is restarted.

I did increase pm.max_children setting from 5 - 10 I still hit the
max_children limit on roughly the same interval.

I will try to find the time to upgrade the server to a newer snapshot
and test the patch provided by Todd.


# grep max_children /var/log/php-fpm.log  
[29-Mar-2015 17:25:31] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[29-Mar-2015 18:57:16] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[29-Mar-2015 19:22:12] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[30-Mar-2015 03:22:25] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[31-Mar-2015 21:47:08] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[31-Mar-2015 22:47:09] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[02-Apr-2015 11:39:22] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[02-Apr-2015 13:39:26] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[03-Apr-2015 15:44:39] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[05-Apr-2015 17:07:15] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[05-Apr-2015 17:12:54] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[08-Apr-2015 14:00:57] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[21-May-2015 13:32:37] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[25-May-2015 17:15:54] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[25-May-2015 17:42:39] WARNING: [pool www] server reached pm.max_children 
setting (5), consider raising it
[30-May-2015 15:27:55] WARNING: [pool www] server reached pm.max_children 
setting (10), consider raising it
# 

After increasing the limit I also hit:

[30-May-2015 15:27:51] WARNING: [pool www] seems busy (you may need to increase 
pm.start_servers, or pm.min/max_spare_servers), spawning 8 children, there are 
0 idle, and 6 total children
[30-May-2015 15:27:52] WARNING: [pool www] seems busy (you may need to increase 
pm.start_servers, or pm.min/max_spare_servers), spawning 16 children, there are 
0 idle, and 7 total children
[30-May-2015 15:27:53] WARNING: [pool www] seems busy (you may need to increase 
pm.start_servers, or pm.min/max_spare_servers), spawning 32 children, there are 
0 idle, and 8 total children
[30-May-2015 15:27:54] WARNING: [pool www] seems busy (you may need to increase 
pm.start_servers, or pm.min/max_spare_servers), spawning 32 children, there are 
0 idle, and 9 total children




 I agree that my patch is more of a workaround, and it would be
 better to track down how it is that the client is being passed to
 server_fcgi with an open socket. I was going this way when I started
 looking at the source, but then I saw that clt-clt_srvevb and
 clt-clt_srvbev get the same treatment (free if not null, then
 reassign) at the same spot in server_fcgi(), and I figured if it
 was good enough for clt_srvevb and clt_srvbev, why not for clt_fd?
 
 I would be happy to look into a proper solution if that would be
 better.
 
 Thanks!
 Todd
 
 On May 31, 2015, at 2:23, Joerg Jung m...@umaxx.net wrote:
 
  Hi,
  
  Am 20.05.2015 um 02:06 schrieb Todd Mortimer t...@opennet.ca:
  
  The attached patch fixes a problem I’ve been having with httpd +
  php_fpm + owncloud on 5.7. The patch is against 5.7-release.
  
  Can you try with recent snapshot, and see if issue
  still occurs, please?
  Development happens in -current.
  
  After several days running 

Re: sys/ucontext.h - dead code walking?

[Adam Wolk]

On Sun, Apr 19, 2015, at 12:23 AM, Philip Guenther wrote:
 On Sat, Apr 18, 2015 at 2:56 PM, Adam Wolk adam.w...@koparo.com wrote:
  On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote:
   From: Adam Wolk adam.w...@koparo.com
   Date: Sat, 18 Apr 2015 23:23:40 +0200
 ...
   Which lead me to a hunt on how other parts of base/ports handle this.
   I grepped /usr/src and found something interesting.
  
   /gnu/gcc/gcc/config/pa/hpux-unwind.h
 
  This is HP-UX specific code.
 
  Yes, but there are also other code paths like:
  ./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h
 
  It's in the base system, even if it's correct for other platforms then I 
  don't see a reason
  for code that will never compile on OpenBSD to be included in OpenBSD base 
  - unless
  removing it from the build system is more effort than maintaining it's 
  presence.
 
 There's always a question with 3rd party code, such as everything
 under gnu/, of whether local changes should be minimized or expansive.
 Once the changes become too expansive, it'll effectively be a fork
 which requires more local resources to be spent on it going forward:
 look how much effort has gone into libressl.
 
 It seems in this case that the benefits of removing that code are
 insubstantial compared to the time that would be required (would need
 to verify that all the archs still build unchanged).  Properly done,
 there would be *no* effect on the binaries, and would have only
 limited improvements on code comprehensibility: this isn't like other
 programs where we can delete piles of #ifdefs that cluster the main
 code, and really there's very little development being done in the gcc
 code itself...so why bother?
 
 
 Philip Guenther

Understood. Thank you for the explanation.

Regards,
Adam

sys/ucontext.h - dead code walking?

[Adam Wolk]

Hi tech@,

I'm working on a port for lang/dart and got stuck on ucontext.h compile
errors.
The first one was quite easy, changing sys/ucontext.h to signal.h since
ucontext_t is
defined there

sys/signal.h:
typedef struct sigcontext ucontext_t;

This allowed me to move forward and stop on the next bit:
In file included from runtime/vm/thread_interrupter.h:9:0,
 from runtime/vm/profiler.h:13,
 from runtime/vm/dart.ca c:22:
runtime/vm/signal_handler.h:49:44: error: 'mcontext_t' does not name a
type
   static uintptr_t GetProgramCounter(const mcontext_t mcontext);
^
runtime/vm/signal_handler.h:50:42: error: 'mcontext_t' does not name a
type


Which lead me to a hunt on how other parts of base/ports handle this.
I grepped /usr/src and found something interesting.

/gnu/gcc/gcc/config/pa/hpux-unwind.h

which contains a:
#include sys/ucontext.h

Now taking this example C program:
$ cat dead.c 
#include sys/ucontext.h

int
main(int argc, char *argv[])
{
  return 0;
}
$ 

and trying to compile it:
$ make dead
cc -O2 -pipe-o dead dead.c 
dead.c:1:26: error: sys/ucontext.h: No such file or directory
*** Error 1 in /home/mulander/code/c (sys.mk:85 'dead')

We can see that sys/ucontext.h is not present. Hence the hpux-unwind.h
header file must be dead code.

Grepping src I found some more occurrences, all in base gcc (minus some
changelog/comment entries).
Should header files including sys/ucontext.h be removed along with their
paired .c files?

./gnu/gcc/fixincludes/ChangeLog:* tests/base/sys/ucontext.h: New
file.
./gnu/gcc/fixincludes/fixincl.x:  |sys/ucontext.h|;
./gnu/gcc/fixincludes/inclhack.def:/* The /usr/include/sys/ucontext.h on
ia64-*linux-gnu systems defines
./gnu/gcc/fixincludes/inclhack.def:files = sys/ucontext.h;
./gnu/gcc/fixincludes/tests/base/sys/ucontext.h:   
fixinc/tests/inc/sys/ucontext.h
./gnu/gcc/gcc/config/alpha/linux-unwind.h:#include sys/ucontext.h
./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h
./gnu/gcc/gcc/config/i386/linux-unwind.h:/* There's no sys/ucontext.h
for glibc 2.0, so no
./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h
./gnu/gcc/gcc/config/ia64/linux-unwind.h:#include sys/ucontext.h
./gnu/gcc/gcc/config/mips/linux-unwind.h: * struct ucontext from
sys/ucontext.h so this private copy is used.  */
./gnu/gcc/gcc/config/pa/hpux-unwind.h:#include sys/ucontext.h
./gnu/gcc/gcc/config/pa/linux-unwind.h:#include sys/ucontext.h
./gnu/gcc/gcc/config/rs6000/host-darwin.c:#include sys/ucontext.h
./gnu/gcc/gcc/config/sh/linux-unwind.h:#include sys/ucontext.h
./gnu/usr.bin/binutils/gdb/s390-nat.c:#include sys/ucontext.h
./gnu/usr.bin/binutils/gdb/sparc-nat.c:   fp_status' in
sys/ucontext.h, which is automatically included by
./gnu/usr.bin/binutils/gdb/user-regs.c:   sys/ucontext.h includes
sys/procfs.h includes sys/user.h, which
./gnu/usr.bin/binutils/gdb/osf-share/cma_tcb_defs.h:#   include
sys/ucontext.h
./gnu/usr.bin/gcc/gcc/ChangeLog:sys/ucontext.h inclusion in
ifndef USE_GNULIBC_1.
./gnu/usr.bin/gcc/gcc/ChangeLog.7:  including signal.h and
sys/ucontext.h, not needed.
./gnu/usr.bin/gcc/gcc/ChangeLog.7:  to avoid clash with Irix header
file sys/ucontext.h.  Rename gp_regs
./gnu/usr.bin/gcc/gcc/config/alpha/linux.h:#include sys/ucontext.h
./gnu/usr.bin/gcc/gcc/config/i386/linux.h:/* There's no sys/ucontext.h
for some (all?) libc1, so no
./gnu/usr.bin/gcc/gcc/config/i386/linux.h:#include sys/ucontext.h
./gnu/usr.bin/gcc/gcc/config/i386/linux64.h:#include sys/ucontext.h
./gnu/usr.bin/gcc/gcc/config/ia64/linux.h:#include sys/ucontext.h

PS.
I would greatly appreciate If anyone pointed me at a file that still
defines
mcontext_t or an acceptable workaround :)

Regards,
-- 
  Adam Wolk
  adam.w...@koparo.com

Re: sys/ucontext.h - dead code walking?

[Adam Wolk]

On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote:
  From: Adam Wolk adam.w...@koparo.com
  Date: Sat, 18 Apr 2015 23:23:40 +0200
  
  Hi tech@,
  
  I'm working on a port for lang/dart and got stuck on ucontext.h compile
  errors.
  The first one was quite easy, changing sys/ucontext.h to signal.h since
  ucontext_t is
  defined there
  
  sys/signal.h:
  typedef struct sigcontext ucontext_t;
 
 It is questionable whether ucontext_t should be defined there.  The
 sys/ucontext.h header has been removed from POSIX, but POSIX still
 refers to ucontext_t in its signal handler description.
 
 In the end the reason sys/ucontext.h has been removed from POSIX is
 because it is impossible to use its functionality in a portable
 fashion.  It is inherently architecture dependent, and effectively OS
 dependent as well.
 
  This allowed me to move forward and stop on the next bit:
  In file included from runtime/vm/thread_interrupter.h:9:0,
   from runtime/vm/profiler.h:13,
   from runtime/vm/dart.ca c:22:
  runtime/vm/signal_handler.h:49:44: error: 'mcontext_t' does not name a
  type
 static uintptr_t GetProgramCounter(const mcontext_t mcontext);
  ^
  runtime/vm/signal_handler.h:50:42: error: 'mcontext_t' does not name a
  type
 
 If this bit of code is indeed essential,you'll have to write an
 OpenBSD-specific version of it.  My advise would be to stick to using
 struct sigcontext.  Change GetProgramCounter to take const struct
 sigcontext sc as an argument, and make it return sc.sc_pc; That
 would make it work on alpha/i386/sparc/sparc64/vax and we can add the
 appropriate define on other architectures.  For amd64 that would be
 
   #define sc_pc sc_rip
 
 If you need more help, please post the relevant code or provide an url
 with (preferabley browsable) source code.
 

The browsable code can be seen on github:
 -
 
https://github.com/dart-lang/bleeding_edge/blob/master/dart/runtime/vm/signal_handler.h

It seems that the android path defines:
 typedef struct sigcontext mcontext_t;

which matches your recommendation and has a high chance of the whole
port 
going forward. I'll try adding it in the OpenBSD build path for the
port.

Thank you for the hint, you probably unblocked my progress on the port

  Which lead me to a hunt on how other parts of base/ports handle this.
  I grepped /usr/src and found something interesting.
  
  /gnu/gcc/gcc/config/pa/hpux-unwind.h
 
 This is HP-UX specific code.

Yes, but there are also other code paths like:
./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h

It's in the base system, even if it's correct for other platforms then I
don't see a reason
for code that will never compile on OpenBSD to be included in OpenBSD
base - unless
removing it from the build system is more effort than maintaining it's
presence.

I'm not saying it's bad - just wanted to point out that I stumbled upon
it.

Regards,
Adam

Re: inteldrm/radeondrm running -current

[Adam Wolk]

On Wed, Apr 15, 2015, at 11:56 PM, Mark Kettenis wrote:
 Hi folks,
 
 Earlier today, I committed a diff that includes a check that the drm
 ioctls return a proper error code.  If you see something like:
 
   drmioctl: cmd 0xXX errno -YY
 
 in your dmesg or on your console, please let me know.
 
 Thanks,
 
 Mark
 

Hi Mark,

The only intel drm messages I receive on my current snapshot (Apr 14)
are (1 - see below). I see an Apr 15 snapshot on NYC*BUG though I doubt
there's a way
to test if your patch is included there and probably better to wait a
couple
of days before updating?

I'm running:
inteldrm0 at vga1
drm0 at inteldrm0
error: [drm:pid0:i915_write32] *ERROR* Unknown unclaimed register before
writing to 10
error: [drm:pid0:intel_dp_set_link_train] *ERROR* Timed out waiting for
DP idle patterns
error: [drm:pid0:i915_write32] *ERROR* Unknown unclaimed register before
writing to 64040
error: [drm:pid0:intel_dp_set_link_train] *ERROR* Timed out waiting for
DP idle patterns
inteldrm0: 1366x768

the shop claims the intel card is: 'Intel HD Graphics 4400'

 ATI Radeon HD 8500M rev 0x00 at pci3 dev 0 function 0 not configured

1)

error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:intel_dp_set_link_train] *ERROR* Timed out waiting
for DP idle patterns
error: [drm:pid14978:i915_write32] *ERROR* Unknown unclaimed register
before writing to 64040

Regards,
Adam

Re: [PATCH] bsd.port.mk - make relation between GH_TAGNAME GH_COMMIT more apparent (prevent actual bug regression)

[Adam Wolk]

On Sun, Apr 5, 2015, at 01:31 PM, Stuart Henderson wrote:
 On 2015-04-04, Landry Breuil lan...@rhaalovely.net wrote:
  On Sat, Apr 04, 2015 at 11:07:11PM +0200, Adam Wolk wrote:
  Hi tech@
  
  I'm the maintainer of www/otter-browser and I got caught while packaging
  otter-browser 0.9.04. Upstream asked us to point at a different commit
  then the tagged revision so we did:
  
  GH_TAGNAME =   v0.9.04
  # This is the actual tagged revision
  # GH_COMMIT =  869d29d19719b3057e137a79d4a10025d2c920f6
  # but we were asked by upstream to release from the following commit
  # as it's considered an important fix affecting the majority of users
  GH_COMMIT =23d7ee6f9cd636e750687a01975b177c1c9c2e53
  
  This port was reviewed with an ok by two people and underwent further
  changes later on.
  
  I didn't notice that the port actually packaged GH_TAGNAME contents
  instead of GH_COMMIT.
 
  GH_COMMIT is meaningless in terms of package version, which expects a
  correctly structured version, hence GH_TAGNAME being usually used in
  combination with GH_PROJECT.
 
 Pretty much all ports with GH_TAGNAME are also setting GH_COMMIT, but
 GH_COMMIT doesn't do anything there. I think we were hoping it would
 fetch a specific commitid in case the tag was slided, but it doesn't
 look like github supports that.
 

I can confirm this. That's how I got burned with otter. Ports 'tell you'
that they
are downloading from that 'tag' pluus the GH_COMMIT you set making you
think it's actually downloading the proper changeset.

In reality, github does redirects behind the scene and points at the
tagged
revision.

 I think we should remove the existing ones and make it an error to
 specify both GH_TAGNAME and GH_COMMIT. Thoughts? If people think this
 is a good idea I'll do the ports mop-up.
 
 Index: bsd.port.mk
 ===
 RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
 retrieving revision 1.1288
 diff -u -p -r1.1288 bsd.port.mk
 --- bsd.port.mk 4 Jan 2015 05:47:07 -   1.1288
 +++ bsd.port.mk 5 Apr 2015 11:30:41 -
 @@ -1168,6 +1168,9 @@ MASTER_SITE_OVERRIDE ?= No
  .endif
  
  .if !empty(GH_ACCOUNT)  !empty(GH_PROJECT)
 +.  if !empty(GH_COMMIT)  !empty(GH_TAGNAME)
 +ERRORS += Fatal: specifying both GH_TAGNAME and GH_COMMIT is invalid
 +.  endif
  .  if ${GH_TAGNAME} == master
  ERRORS += Fatal: using master as GH_TAGNAME is invalid
  .  endif
 
 

I think it is a good idea. I would also suggest changing the docs to no
longer
suggest that it is good practice to set both.

Index: bsd.port.mk.5
===
RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
retrieving revision 1.415
diff -u -p -r1.415 bsd.port.mk.5
--- bsd.port.mk.5   3 Apr 2015 10:19:22 -   1.415
+++ bsd.port.mk.5   5 Apr 2015 12:26:41 -
@@ -1701,8 +1701,7 @@ Yields a suitable default for
 Account name of the GitHub user hosting the project.
 .It Ev GH_COMMIT
 SHA1 commit id to fetch.
-It is good practice to always specify
-the commit id, even if ${GH_TAGNAME} was specified.
+It is an error to specify ${GH_COMMIT} when ${GH_TAGNAME} is specified.
 .It Ev GH_PROJECT
 Name of the project on GitHub.
 .It Ev GH_TAGNAME

Re: [PATCH] bsd.port.mk - make relation between GH_TAGNAME GH_COMMIT more apparent (prevent actual bug regression)

[Adam Wolk]

On Sat, Apr 4, 2015, at 11:27 PM, Landry Breuil wrote:
 On Sat, Apr 04, 2015 at 11:07:11PM +0200, Adam Wolk wrote:
  Hi tech@
  
  I'm the maintainer of www/otter-browser and I got caught while packaging
  otter-browser 0.9.04. Upstream asked us to point at a different commit
  then the tagged revision so we did:
  
  GH_TAGNAME =   v0.9.04
  # This is the actual tagged revision
  # GH_COMMIT =  869d29d19719b3057e137a79d4a10025d2c920f6
  # but we were asked by upstream to release from the following commit
  # as it's considered an important fix affecting the majority of users
  GH_COMMIT =23d7ee6f9cd636e750687a01975b177c1c9c2e53
  
  This port was reviewed with an ok by two people and underwent further
  changes later on.
  
  I didn't notice that the port actually packaged GH_TAGNAME contents
  instead of GH_COMMIT.
 
 GH_COMMIT is meaningless in terms of package version, which expects a
 correctly structured version, hence GH_TAGNAME being usually used in
 combination with GH_PROJECT.
 
 Look, you even set it yourself for otter-browser:
 
 DISTNAME =  ${GH_PROJECT}-${GH_TAGNAME:C/^v//}
 
 (and PKGNAME is derived from DISTNAME)
 Here, since you go forward in git history, you have the choice of
 bumping REVISION, or using .MMDD suffixes, or using the special
 'pre' / 'rc' / 'beta' keywords in the version, see packages-specs(7).
 
 S i'm not sure the documentation is at fault here :)
 
 Landry
 

Yep, my fault. I should have tested this earlier.

ser/archive/v0.9.05/1ea5df13f908495df4ad9d634d997f6fd4c9.tar.gz 
 
Trying 192.30.252.128...
Requesting
https://github.com/OtterBrowser/otter-browser/archive/v0.9.05/1ea5df13f908495df4a
d9d634d997f6fd4c9.tar.gz
Redirected to
https://codeload.github.com/OtterBrowser/otter-browser/tar.gz/v0.9.05
Trying 192.30.252.144...
Requesting
https://codeload.github.com/OtterBrowser/otter-browser/tar.gz/v0.9.05
100%
|**| 
2248 KB00:05
2302624 bytes received in 5.65 seconds (398.11 KB/s)


So github redirects to the tag even though the ports system just shows a
download

===  Checking files for otter-browser-0.9.05
 Fetch 
 https://github.com/OtterBrowser/otter-browser/archive/v0.9.05/1ea5df13f908495df4ad9d634d997f6fd4c9.tar.gz

I should pay more attention to what's going on during port building.
Feel free to ignore the
patch  thanks for feedback ;)

Regards,
Adam

[PATCH] bsd.port.mk - make relation between GH_TAGNAME GH_COMMIT more apparent (prevent actual bug regression)

[Adam Wolk]

Hi tech@

I'm the maintainer of www/otter-browser and I got caught while packaging
otter-browser 0.9.04. Upstream asked us to point at a different commit
then the tagged revision so we did:

GH_TAGNAME =   v0.9.04
# This is the actual tagged revision
# GH_COMMIT =  869d29d19719b3057e137a79d4a10025d2c920f6
# but we were asked by upstream to release from the following commit
# as it's considered an important fix affecting the majority of users
GH_COMMIT =23d7ee6f9cd636e750687a01975b177c1c9c2e53

This port was reviewed with an ok by two people and underwent further
changes later on.

I didn't notice that the port actually packaged GH_TAGNAME contents
instead of GH_COMMIT.

Current documentation for both tags are as follows:
 GH_COMMIT SHA1 commit id to fetch.  It is good practice to
 always
   specify the commit id, even if ${GH_TAGNAME} was
   specified.

 GH_TAGNAMEName of the tag to download.  Setting ${GH_TAGNAME}
 to
   master is invalid and will throw an error. 
   ${WRKDIST} is
   auto-generated based on the ${GH_TAGNAME} if
   specified,
   otherwise ${GH_COMMIT} will be used to generate
   ${WRKDIST}.

I would like to suggest a small alteration to GH_COMMIT to point out
that
GH_TAGNAME takes precedence even if they point at different changeset.
The ports system doesn't warn about that situation and I almost got
caught
by it twice since upstream again asks us to package a couple of
revisions
ahead of the tagged version.

Regards,
Adam

Index: bsd.port.mk.5
===
RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
retrieving revision 1.415
diff -u -p -r1.415 bsd.port.mk.5
--- bsd.port.mk.5   3 Apr 2015 10:19:22 -   1.415
+++ bsd.port.mk.5   4 Apr 2015 20:58:59 -
@@ -1703,6 +1703,7 @@ Account name of the GitHub user hosting 
 SHA1 commit id to fetch.
 It is good practice to always specify
 the commit id, even if ${GH_TAGNAME} was specified.
+${GH_TAGNAME} takes precedence even if ${GH_COMMIT} points at a
different changeset.
 .It Ev GH_PROJECT
 Name of the project on GitHub.
 .It Ev GH_TAGNAME

Re: Unbreak adventure(6)

[Adam Wolk]

On Wed, Dec 31, 2014, at 04:16 PM, Theo Buehler wrote:
 The adventure game is currently broken.  When it's started without
 any arguments, it spits a pile of garbage to stdout before eventually
 dumping its core.
 
Confirmed true for i386 running a snapshot from 27-Dec-2014.
With your patch (obtained from CVS) the game starts up properly
and I'm able to quit without breaking the terminal.

 The game data of adventure(6) is obfuscated at compile time with a
 scheme relying on deterministic random() and deobfuscated at runtime.
 This is done ``to prevent casual snooping of the executable'' (cf.
 status.c).  Thus the program must use the deterministic random generator
 for that elaborate scheme.
 
 Randomness during game play comes exclusively from the ran() function in
 wizard.c -- which currently suffers from modulo bias -- better use
 arc4random_uniform() there.
 
 Index: init.c
 ===
 RCS file: /cvs/src/games/adventure/init.c,v
 retrieving revision 1.12
 diff -u -p -r1.12 init.c
 --- init.c  8 Dec 2014 21:56:27 -   1.12
 +++ init.c  31 Dec 2014 15:11:34 -
 @@ -56,6 +56,11 @@ int setbit[16] = {1, 2, 4, 010, 020,
  void
  init(void)  /* everything for 1st time run */
  {
 +   /*
 +* We need deterministic randomness for the obfuscation schemes
 +* in io.c and setup.c.
 +*/
 +   srandom_deterministic(1);
   rdata();/* read data from orig. file */
   linkdata();
   poof();
 Index: setup.c
 ===
 RCS file: /cvs/src/games/adventure/setup.c,v
 retrieving revision 1.11
 diff -u -p -r1.11 setup.c
 --- setup.c 8 Dec 2014 21:56:27 -   1.11
 +++ setup.c 31 Dec 2014 15:11:34 -
 @@ -78,6 +78,8 @@ main(int argc, char *argv[])
   count = 0;
   linestart = YES;
  
 +   srandom_deterministic(1);
 +
   while ((c = getc(infile)) != EOF) {
   if (count++ % LINE == 0)
   printf(\n\t);
 Index: wizard.c
 ===
 RCS file: /cvs/src/games/adventure/wizard.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 wizard.c
 --- wizard.c16 Nov 2014 04:49:48 -  1.16
 +++ wizard.c31 Dec 2014 15:11:34 -
 @@ -141,8 +141,5 @@ ciao(void)
  int
  ran(int range)
  {
 -   longi;
 -
 -   i = random() % range;
 -   return (i);
 +   return (arc4random_uniform(range));
  }
 

Re: next LibreSSL-portable release coming soon

[Adam Wolk]

Hi Brent,

I did a compile and run the tests on Archlinux 64-bit.
 - no compiler warnings
 - all tests passed (results below)

I also took a look at the pages generated in man for tls_init.3,
openssl.3  ssl.3 - they all look fine (no visible glitches).

Please let me know if I can do more tests (ie. guiding me to a specific
part of library/executable).

[mulander@koparo portable]$ uname -a
Linux koparo 3.17.4-1-ARCH #1 SMP PREEMPT Fri Nov 21 21:14:42 CET 2014
x86_64 GNU/Linux

[mulander@koparo portable]$ gcc --version
gcc (GCC) 4.9.2
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is
NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.

[mulander@koparo portable]$ ./apps/openssl version
LibreSSL 2.1

Tested on commit: commit cfbc62e686f020f3db6b2aa4e86cc19b662c3597


Testsuite summary for libressl 2.1.2

# TOTAL: 44
# PASS:  44
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0


Regards,
-- 
  Adam Wolk
  adam.w...@koparo.com

On Mon, Dec 8, 2014, at 03:54 PM, Brent Cook wrote:
 We spent the weekend buttoning up features and closing issues with
 LibreSSL-portable.
 All features and fixes for the next release are now landed in the github
 mirror:
 
 https://github.com/libressl-portable/portable
 
 Please test if you can. Thanks!
 
  - Brent
 

Re: Is there a repo for the latest LibreSSL portable?

[Adam Wolk]

Hi,

On Sun, Aug 10, 2014, at 12:38 PM, Nicholas Wilson wrote:
 Maybe this is a silly question - but where is the code for the portable
 version checked in? I think I understand the development model from
 working
 with OpenSSH dev, but surely the portable compat files must be kept in
 version control somewhere though, as well as in the tarball releases. I'd
 like to contribute to LibreSSL but do I have to install and develop on
 OpenBSD just to run the latest trunk code?
 

According to http://www.libressl.org/:
 We have a github repository clone as libressl-portable[1] on github for the 
 curious. This is a copy of the working respositories which are not 
 maintained on github.

[1] https://github.com/libressl-portable/

I guess you can work on the portable github mirror and submit patches to
the list if you don't want to work with cvs directly. Worth to also note
the readme on the github repo:

 Development is done in the upstream OpenBSD codebase. 
 A github clone of the official repositories is kept at: 
 https://github.com/libressl-portable
 We update this repository from the OpenBSD respositories
 semi-frequently, so changes may not show up in GitHub immediately. 
 The GitHub repository should be used for informational purposes only.


Regards,
-- 
  Adam Wolk
  adam.w...@koparo.com