PATCH: fix check in smtpd/table_socketmap.c

2014-07-10 Thread Doug Hogan
socket() returns -1 on error.


Index: usr.sbin/smtpd/table_socketmap.c
===
RCS file: /cvs/src/usr.sbin/smtpd/table_socketmap.c,v
retrieving revision 1.4
diff -u -p -d -r1.4 table_socketmap.c
--- usr.sbin/smtpd/table_socketmap.c8 Jul 2014 13:49:09 -   1.4
+++ usr.sbin/smtpd/table_socketmap.c11 Jul 2014 04:29:33 -
@@ -125,7 +125,7 @@ table_socketmap_connect(const char *s)
return 1;
 
 err:
-   if (sock) {
+   if (sock != -1) {
close(sock);
sock = -1;
}



PATCH: add more malloc.conf details to malloc.3

2014-07-10 Thread Doug Hogan
I don't think the current man page has enough detail for what the
malloc.conf settings do.


Index: lib/libc/stdlib/malloc.3
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.78
diff -u -p -d -r1.78 malloc.3
--- lib/libc/stdlib/malloc.31 May 2014 18:41:59 -   1.78
+++ lib/libc/stdlib/malloc.311 Jul 2014 04:30:37 -
@@ -220,10 +220,16 @@ Malloc will first look for a symbolic li
 .Pa /etc/malloc.conf
 and next check the environment for a variable called
 .Ev MALLOC_OPTIONS
+(if not
+.Xr issetugid 2 )
 and finally for the global variable
 .Va malloc_options
 and scan them for flags in that order.
 Flags are single letters, uppercase means on, lowercase means off.
+
+The default flags are 'AP', small chunks are always junked, and the first
+part of the pages is junked after free.  The default junk behavior does not
+correspond to 'j' or 'J'.
 .Bl -tag -width indent
 .It Cm A
 .Dq Abort .
@@ -270,7 +276,7 @@ Fill some junk into the area allocated.
 Currently junk is bytes of 0xd0 when allocating; this is pronounced
 .Dq Duh .
 \:-)
-Freed chunks are filled with 0xdf.
+Freed chunks are filled with 0xdf.  This performs more junking than by default.
 .It Cm j
 .Dq Don't Junk .
 By default, small chunks are always junked, and the first part of pages
@@ -297,6 +303,7 @@ This can substantially aid in compacting
 .\Consult the source for this one.
 .It Cm S
 Enable all options suitable for security auditing.
+This currently enables flags 'UJG' and sets the free cache page size to 0.
 .It Cm U
 .Dq Free unmap .
 Enable use after free protection for larger allocations.



PATCH: missing NUL terminate after readlink() in csh

2014-07-11 Thread Doug Hogan
Csh has a section of code where it NUL terminates after a strlcpy().
Strlcpy() may read past what readlink() wrote since readlink() does
not append a NUL.


Index: bin/csh/dir.c
===
RCS file: /cvs/src/bin/csh/dir.c,v
retrieving revision 1.14
diff -u -p -d -r1.14 dir.c
--- bin/csh/dir.c   27 Oct 2009 23:59:21 -  1.14
+++ bin/csh/dir.c   11 Jul 2014 06:04:23 -
@@ -760,8 +760,8 @@ dcanon(Char *cp, Char *p)
!adrof(STRignore_symlinks) 
(cc = readlink(short2str(cp), tlink,
   sizeof tlink-1)) = 0) {
+   tlink[cc] = '\0';
(void) Strlcpy(link, str2short(tlink), sizeof 
link/sizeof(Char));
-   link[cc] = '\0';
 
/*
 * restore the '/'.
Index: bin/pax/ftree.c
===
RCS file: /cvs/src/bin/pax/ftree.c,v
retrieving revision 1.31
diff -u -p -d -r1.31 ftree.c
--- bin/pax/ftree.c 24 May 2014 18:51:00 -  1.31
+++ bin/pax/ftree.c 11 Jul 2014 06:04:23 -
@@ -477,7 +477,7 @@ next_file(ARCHD *arcn)
}
/*
 * set link name length, watch out readlink does not
-* always NUL terminate the link path
+* NUL terminate the link path
 */
arcn-ln_name[cnt] = '\0';
arcn-ln_nlen = cnt;



Re: PATCH: add more malloc.conf details to malloc.3

2014-07-11 Thread Doug Hogan
On Fri, Jul 11, 2014 at 01:03:54AM -0600, Anthony J. Bentley wrote:
 In addition to what jmc said, if you document these flags they need to
 be marked up with the Cm macro instead of ASCII single quotes.

My mistake.  Fixed version:


Index: lib/libc/stdlib/malloc.3
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.78
diff -u -p -d -r1.78 malloc.3
--- lib/libc/stdlib/malloc.31 May 2014 18:41:59 -   1.78
+++ lib/libc/stdlib/malloc.311 Jul 2014 07:19:27 -
@@ -220,10 +220,23 @@ Malloc will first look for a symbolic li
 .Pa /etc/malloc.conf
 and next check the environment for a variable called
 .Ev MALLOC_OPTIONS
+(if not
+.Xr issetugid 2 )
 and finally for the global variable
 .Va malloc_options
 and scan them for flags in that order.
 Flags are single letters, uppercase means on, lowercase means off.
+.Pp
+The default flags are
+.Cm A
+and
+.Cm P ,
+small chunks are always junked, and the first
+part of the pages is junked after free.
+The default junk behavior does not correspond to
+.Cm j
+or
+.Cm J .
 .Bl -tag -width indent
 .It Cm A
 .Dq Abort .
@@ -271,6 +284,7 @@ Currently junk is bytes of 0xd0 when all
 .Dq Duh .
 \:-)
 Freed chunks are filled with 0xdf.
+This performs more junking than by default.
 .It Cm j
 .Dq Don't Junk .
 By default, small chunks are always junked, and the first part of pages
@@ -297,6 +311,11 @@ This can substantially aid in compacting
 .\Consult the source for this one.
 .It Cm S
 Enable all options suitable for security auditing.
+This currently enables flags
+.Cm U ,
+.Cm J ,
+.Cm G
+and sets the free cache page size to 0.
 .It Cm U
 .Dq Free unmap .
 Enable use after free protection for larger allocations.



PATCH: fix various fdopen error handling fd leaks

2014-07-11 Thread Doug Hogan
Another patch will handle some of the fdopen error handling leaks
that are combined with missing unlink calls when using mkstemp.


Index: games/atc/log.c
===
RCS file: /cvs/src/games/atc/log.c,v
retrieving revision 1.17
diff -u -p -d -r1.17 log.c
--- games/atc/log.c 27 Oct 2009 23:59:23 -  1.17
+++ games/atc/log.c 11 Jul 2014 07:41:36 -
@@ -109,6 +109,7 @@ open_score_file(void)
score_fp = fdopen(score_fd, r+);
if (score_fp == NULL) {
perror(_PATH_SCORE);
+   close(score_fd);
return (-1);
}
umask(old_mode);
Index: sbin/isakmpd/ike_auth.c
===
RCS file: /cvs/src/sbin/isakmpd/ike_auth.c,v
retrieving revision 1.110
diff -u -p -d -r1.110 ike_auth.c
--- sbin/isakmpd/ike_auth.c 16 Apr 2007 13:01:39 -  1.110
+++ sbin/isakmpd/ike_auth.c 11 Jul 2014 07:41:36 -
@@ -299,12 +299,14 @@ ignorekeynote:
 
if (check_file_secrecy_fd(fd, keyfile, fsize)) {
free(privkeyfile);
+   close(fd);
return 0;
}
 
if ((keyfp = fdopen(fd, r)) == NULL) {
log_print(ike_auth_get_key: fdopen failed);
free(privkeyfile);
+   close(fd);
return 0;
}
 #if SSLEAY_VERSION_NUMBER = 0x00904100L
Index: usr.bin/finger/net.c
===
RCS file: /cvs/src/usr.bin/finger/net.c,v
retrieving revision 1.12
diff -u -p -d -r1.12 net.c
--- usr.bin/finger/net.c27 Oct 2009 23:59:38 -  1.12
+++ usr.bin/finger/net.c11 Jul 2014 07:41:36 -
@@ -141,5 +141,8 @@ netfinger(name)
}
if (lastc != '\n')
putchar('\n');
-   (void)fclose(fp);
+   if (fp == NULL)
+   (void)close(s);
+   else
+   (void)fclose(fp);
 }
Index: usr.bin/mandoc/mandocdb.c
===
RCS file: /cvs/src/usr.bin/mandoc/mandocdb.c,v
retrieving revision 1.111
diff -u -p -d -r1.111 mandocdb.c
--- usr.bin/mandoc/mandocdb.c   21 Jun 2014 16:17:56 -  1.111
+++ usr.bin/mandoc/mandocdb.c   11 Jul 2014 07:41:37 -
@@ -1320,6 +1320,8 @@ parse_cat(struct mpage *mpage, int fd)
fopen(mpage-mlinks-file, r) :
fdopen(fd, r);
if (NULL == stream) {
+   if (-1 != fd)
+   close(fd);
if (warnings)
say(mpage-mlinks-file, fopen);
return;
Index: usr.bin/ssh/ssh-keygen.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.c,v
retrieving revision 1.249
diff -u -p -d -r1.249 ssh-keygen.c
--- usr.bin/ssh/ssh-keygen.c3 Jul 2014 03:47:27 -   1.249
+++ usr.bin/ssh/ssh-keygen.c11 Jul 2014 07:41:37 -
@@ -953,12 +953,14 @@ do_gen_all_hostkeys(struct passwd *pw)
f = fdopen(fd, w);
if (f == NULL) {
printf(fdopen %s failed\n, identity_file);
+   close(fd);
key_free(public);
first = 0;
continue;
}
if (!key_write(public, f)) {
fprintf(stderr, write key failed\n);
+   fclose(f);
key_free(public);
first = 0;
continue;
Index: usr.bin/uudecode/uudecode.c
===
RCS file: /cvs/src/usr.bin/uudecode/uudecode.c,v
retrieving revision 1.19
diff -u -p -d -r1.19 uudecode.c
--- usr.bin/uudecode/uudecode.c 20 May 2014 01:25:23 -  1.19
+++ usr.bin/uudecode/uudecode.c 11 Jul 2014 07:41:37 -
@@ -290,6 +290,8 @@ decode2(void)
if ((fd = open(outfile, flags, mode))  0 ||
(outfp = fdopen(fd, w)) == NULL) {
warn(%s: %s, infile, outfile);
+   if (fd != -1)
+   close(fd);
return (1);
}
}
Index: usr.sbin/lpr/lpd/printjob.c
===
RCS file: /cvs/src/usr.sbin/lpr/lpd/printjob.c,v
retrieving revision 1.52
diff -u -p -d -r1.52 printjob.c
--- usr.sbin/lpr/lpd/printjob.c 7 Feb 2014 23:06:21 -   1.52
+++ usr.sbin/lpr/lpd/printjob.c 11 Jul 2014 07:41:38 -
@@ -804,8 +804,12 @@ sendit(char *file)
 
/* open control file */
fd = safe_open(file, O_RDONLY|O_NOFOLLOW, 0);
-   if (fd  0 || (cfp = fdopen(fd, r)) == NULL)
+   if (fd  0 || (cfp = fdopen(fd, r)) == NULL) {
+   if (fd != 

PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan

Index: bin/csh/dol.c
===
RCS file: /cvs/src/bin/csh/dol.c,v
retrieving revision 1.17
diff -u -p -d -r1.17 dol.c
--- bin/csh/dol.c   12 Aug 2010 02:00:27 -  1.17
+++ bin/csh/dol.c   11 Jul 2014 09:12:11 -
@@ -829,7 +829,8 @@ heredoc(Char *term)
 
 if (mkstemp(tmp)  0)
stderror(ERR_SYSTEM, tmp, strerror(errno));
-(void) unlink(tmp);/* 0 0 inode! */
+else
+   (void) unlink(tmp); /* 0 0 inode! */
 Dv[0] = term;
 Dv[1] = NULL;
 gflag = 0;
Index: sbin/disklabel/disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.195
diff -u -p -d -r1.195 disklabel.c
--- sbin/disklabel/disklabel.c  5 May 2014 16:33:34 -   1.195
+++ sbin/disklabel/disklabel.c  11 Jul 2014 09:13:43 -
@@ -815,9 +815,12 @@ edit(struct disklabel *lp, int f)
FILE *fp;
u_int64_t total_sectors, starting_sector, ending_sector;
 
-   if ((fd = mkstemp(tmpfil)) == -1 || (fp = fdopen(fd, w)) == NULL) {
-   if (fd != -1)
+   if ((fd = mkstemp(tmpfil)) == -1 ||
+   (fp = fdopen(fd, w)) == NULL) {
+   if (fd != -1) {
+   unlink(tmpfil);
close(fd);
+   }
warn(%s, tmpfil);
return (1);
}
Index: sbin/scsi/scsi.c
===
RCS file: /cvs/src/sbin/scsi/scsi.c,v
retrieving revision 1.28
diff -u -p -d -r1.28 scsi.c
--- sbin/scsi/scsi.c12 Nov 2013 04:59:02 -  1.28
+++ sbin/scsi/scsi.c11 Jul 2014 09:13:44 -
@@ -571,8 +571,11 @@ edit_init(void)
strlcpy(edit_name, /var/tmp/sc, sizeof edit_name);
if ((fd = mkstemp(edit_name)) == -1)
err(1, mkstemp);
-   if ( (edit_file = fdopen(fd, w+)) == 0)
+   if ( (edit_file = fdopen(fd, w+)) == 0) {
+   unlink(edit_name);
+   close(fd);
err(1, fdopen);
+   }
edit_opened = 1;
 
atexit(edit_done);
Index: usr.bin/gzsig/sign.c
===
RCS file: /cvs/src/usr.bin/gzsig/sign.c,v
retrieving revision 1.13
diff -u -p -d -r1.13 sign.c
--- usr.bin/gzsig/sign.c10 Mar 2013 10:36:57 -  1.13
+++ usr.bin/gzsig/sign.c11 Jul 2014 09:14:10 -
@@ -281,6 +281,7 @@ sign(int argc, char *argv[])
if ((fout = fdopen(fd, w)) == NULL) {
fprintf(stderr, Error opening %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
close(fd);
continue;
@@ -288,6 +289,7 @@ sign(int argc, char *argv[])
if (copy_permissions(fileno(fin), fd)  0) {
fprintf(stderr, Error initializing %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
fclose(fout);
continue;
Index: usr.bin/htpasswd/htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.10
diff -u -p -d -r1.10 htpasswd.c
--- usr.bin/htpasswd/htpasswd.c 24 Mar 2014 20:33:01 -  1.10
+++ usr.bin/htpasswd/htpasswd.c 11 Jul 2014 09:14:10 -
@@ -164,8 +164,10 @@ main(int argc, char** argv)
if ((fd = mkstemp(tmpl)) == -1)
err(1, mkstemp);
 
-   if ((out = fdopen(fd, w+)) == NULL)
+   if ((out = fdopen(fd, w+)) == NULL) {
+   unlink(tmpl);
err(1, cannot open tempfile);
+   }
 
while ((linelen = getline(line, linesize, in))
!= -1) {
Index: usr.bin/m4/eval.c
===
RCS file: /cvs/src/usr.bin/m4/eval.c,v
retrieving revision 1.72
diff -u -p -d -r1.72 eval.c
--- usr.bin/m4/eval.c   28 Apr 2014 12:34:11 -  1.72
+++ usr.bin/m4/eval.c   11 Jul 2014 09:14:11 -
@@ -818,8 +818,11 @@ dodiv(int n)
char fname[] = _PATH_DIVNAME;
 
if ((fd = mkstemp(fname))  0 || 
-   (outfile[n] = fdopen(fd, w+)) == NULL)
+   (outfile[n] = fdopen(fd, w+)) == NULL) {
+   if (fd != -1)
+   unlink(fname);
err(1, %s: cannot divert, fname);
+   }
if (unlink(fname) == -1)
err(1, %s: cannot unlink, fname);
}
Index: usr.bin/rwall/rwall.c

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan
On Fri, Jul 11, 2014 at 12:19:22PM +0200, Philip Guenther wrote:
 This should call warn() before unlink() or close() to guarantee that the
 correct errno value is reported.

Philip,

I see what you are saying.  I was following the man page example in
mkstemp(3) which calls warn() after unlink/close.  I'll update the
patch.

I see a number of places in the tree which call warn like the man page
example.  I'll submit a patch to fix those too.

Thanks!



Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan
On Fri, Jul 11, 2014 at 12:19:22PM +0200, Philip Guenther wrote:
 This should call warn() before unlink() or close() to guarantee that the
 correct errno value is reported.
...
 This and several other need to save errno and use errc(), ala:

Updated patch.  Updated mktemp.3 this time.


Index: bin/csh/dol.c
===
RCS file: /cvs/src/bin/csh/dol.c,v
retrieving revision 1.17
diff -u -p -d -r1.17 dol.c
--- bin/csh/dol.c   12 Aug 2010 02:00:27 -  1.17
+++ bin/csh/dol.c   11 Jul 2014 16:20:04 -
@@ -829,7 +829,8 @@ heredoc(Char *term)
 
 if (mkstemp(tmp)  0)
stderror(ERR_SYSTEM, tmp, strerror(errno));
-(void) unlink(tmp);/* 0 0 inode! */
+else
+   (void) unlink(tmp); /* 0 0 inode! */
 Dv[0] = term;
 Dv[1] = NULL;
 gflag = 0;
Index: lib/libc/stdio/mktemp.3
===
RCS file: /cvs/src/lib/libc/stdio/mktemp.3,v
retrieving revision 1.51
diff -u -p -d -r1.51 mktemp.3
--- lib/libc/stdio/mktemp.3 5 Jun 2013 03:39:23 -   1.51
+++ lib/libc/stdio/mktemp.3 11 Jul 2014 16:20:18 -
@@ -147,11 +147,11 @@ int fd;
 strlcpy(sfn, /tmp/ed.XX, sizeof(sfn));
 if ((fd = mkstemp(sfn)) == -1 ||
 (sfp = fdopen(fd, w+)) == NULL) {
+   warn(%s, sfn);
if (fd != -1) {
unlink(sfn);
close(fd);
}
-   warn(%s, sfn);
return (NULL);
 }
 return (sfp);
Index: sbin/disklabel/disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.195
diff -u -p -d -r1.195 disklabel.c
--- sbin/disklabel/disklabel.c  5 May 2014 16:33:34 -   1.195
+++ sbin/disklabel/disklabel.c  11 Jul 2014 16:20:22 -
@@ -815,10 +815,13 @@ edit(struct disklabel *lp, int f)
FILE *fp;
u_int64_t total_sectors, starting_sector, ending_sector;
 
-   if ((fd = mkstemp(tmpfil)) == -1 || (fp = fdopen(fd, w)) == NULL) {
-   if (fd != -1)
-   close(fd);
+   if ((fd = mkstemp(tmpfil)) == -1 ||
+   (fp = fdopen(fd, w)) == NULL) {
warn(%s, tmpfil);
+   if (fd != -1) {
+   unlink(tmpfil);
+   close(fd);
+   }
return (1);
}
display(fp, lp, 0, 1);
Index: sbin/scsi/scsi.c
===
RCS file: /cvs/src/sbin/scsi/scsi.c,v
retrieving revision 1.28
diff -u -p -d -r1.28 scsi.c
--- sbin/scsi/scsi.c12 Nov 2013 04:59:02 -  1.28
+++ sbin/scsi/scsi.c11 Jul 2014 16:20:22 -
@@ -571,8 +571,12 @@ edit_init(void)
strlcpy(edit_name, /var/tmp/sc, sizeof edit_name);
if ((fd = mkstemp(edit_name)) == -1)
err(1, mkstemp);
-   if ( (edit_file = fdopen(fd, w+)) == 0)
-   err(1, fdopen);
+   if ( (edit_file = fdopen(fd, w+)) == 0) {
+   int saved_errno = errno;
+   unlink(edit_name);
+   close(fd);
+   errc(1, saved_errno, fdopen);
+   }
edit_opened = 1;
 
atexit(edit_done);
Index: usr.bin/gzsig/sign.c
===
RCS file: /cvs/src/usr.bin/gzsig/sign.c,v
retrieving revision 1.13
diff -u -p -d -r1.13 sign.c
--- usr.bin/gzsig/sign.c10 Mar 2013 10:36:57 -  1.13
+++ usr.bin/gzsig/sign.c11 Jul 2014 16:20:25 -
@@ -281,6 +281,7 @@ sign(int argc, char *argv[])
if ((fout = fdopen(fd, w)) == NULL) {
fprintf(stderr, Error opening %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
close(fd);
continue;
@@ -288,6 +289,7 @@ sign(int argc, char *argv[])
if (copy_permissions(fileno(fin), fd)  0) {
fprintf(stderr, Error initializing %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
fclose(fout);
continue;
Index: usr.bin/htpasswd/htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.10
diff -u -p -d -r1.10 htpasswd.c
--- usr.bin/htpasswd/htpasswd.c 24 Mar 2014 20:33:01 -  1.10
+++ usr.bin/htpasswd/htpasswd.c 11 Jul 2014 16:20:25 -
@@ -164,8 +164,11 @@ main(int argc, char** argv)
if ((fd = mkstemp(tmpl)) == -1)
err(1, mkstemp);
 
-   if ((out = fdopen(fd, w+)) == NULL)
-   err(1, cannot open tempfile);
+   if 

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan
On Fri, Jul 11, 2014 at 07:29:06PM +0200, Marc Espie wrote:
 I don't like that part. The logic is a bit wrong. Especially since 
 unlink(fname) is always called for fd != -1, so I feel there should be one
 single call.

Ok

Index: usr.bin/m4/eval.c
===
RCS file: /cvs/src/usr.bin/m4/eval.c,v
retrieving revision 1.72
diff -u -p -d -r1.72 eval.c
--- usr.bin/m4/eval.c   28 Apr 2014 12:34:11 -  1.72
+++ usr.bin/m4/eval.c   11 Jul 2014 18:09:31 -
@@ -817,11 +817,10 @@ dodiv(int n)
if (outfile[n] == NULL) {
char fname[] = _PATH_DIVNAME;
 
-   if ((fd = mkstemp(fname))  0 || 
-   (outfile[n] = fdopen(fd, w+)) == NULL)
-   err(1, %s: cannot divert, fname);
-   if (unlink(fname) == -1)
-   err(1, %s: cannot unlink, fname);
+   if ((fd = mkstemp(fname))  0 ||
+   unlink(fname) == -1 ||
+   (outfile[n] = fdopen(fd, w+)) == NULL)
+   err(1, %s: cannot divert, fname);
}
active = outfile[n];
 }



PATCH: avoid clobbering errno before err/warn

2014-07-11 Thread Doug Hogan
This patch is generated by coccinelle, but I reviewed it.  I changed the
lpr patch to use warnc() so it has less code executing inside
PRIV_START.

I targeted if statements where it modifies errno before warn or err is
called.  It checked a list of functions that are typically used in error
handling and may set errno: close, fclose, unlink, rmdir, fflush and
kill.


Index: bin/systrace/intercept.c
===
RCS file: /cvs/src/bin/systrace/intercept.c,v
retrieving revision 1.61
diff -u -p -d -r1.61 intercept.c
--- bin/systrace/intercept.c24 Apr 2014 01:57:06 -  1.61
+++ bin/systrace/intercept.c12 Jul 2014 04:27:22 -
@@ -356,22 +356,26 @@ intercept_run(int bg, int *fdp, uid_t ui

/* Setup done, restore signal handling state */
if (signal(SIGUSR1, ohandler) == SIG_ERR) {
+   int saved_errno = errno;
kill(pid, SIGKILL);
-   err(1, signal);
+   errc(1, saved_errno, signal);
}
if (sigprocmask(SIG_SETMASK, oset, NULL) == -1) {
+   int saved_errno = errno;
kill(pid, SIGKILL);
-   err(1, sigprocmask);
+   errc(1, saved_errno, sigprocmask);
}
 
if (bg) {
if (daemon(1, 1) == -1) {
+   int saved_errno = errno;
kill(pid, SIGKILL);
-   err(1, daemon);
+   errc(1, saved_errno, daemon);
}
if ((*fdp = intercept_open()) == -1) {
+   int saved_errno = errno;
kill(pid, SIGKILL);
-   err(1, intercept_open);
+   errc(1, saved_errno, intercept_open);
}
}
 
Index: regress/lib/libc/stdio_threading/fgetln/fgetln_test.c
===
RCS file: /cvs/src/regress/lib/libc/stdio_threading/fgetln/fgetln_test.c,v
retrieving revision 1.1
diff -u -p -d -r1.1 fgetln_test.c
--- regress/lib/libc/stdio_threading/fgetln/fgetln_test.c   19 Nov 2009 
08:06:06 -  1.1
+++ regress/lib/libc/stdio_threading/fgetln/fgetln_test.c   12 Jul 2014 
04:29:16 -
@@ -50,11 +50,12 @@ main(void)
strlcpy(sfn, /tmp/barnacles., sizeof(sfn));
if ((fd = mkstemp(sfn)) == -1 ||
(sfp = fdopen(fd, w+)) == NULL) {
+   int saved_errno = errno;
if (fd != -1) {
unlink(sfn);
close(fd);
}
-   err(1, could not open temporary file);
+   errc(1, saved_errno, could not open temporary file);
}
 
for (i = 0; i  4096 * THREAD_COUNT; i++)
Index: regress/lib/libc/stdio_threading/fgets/fgets_test.c
===
RCS file: /cvs/src/regress/lib/libc/stdio_threading/fgets/fgets_test.c,v
retrieving revision 1.1
diff -u -p -d -r1.1 fgets_test.c
--- regress/lib/libc/stdio_threading/fgets/fgets_test.c 19 Nov 2009 08:06:06 
-  1.1
+++ regress/lib/libc/stdio_threading/fgets/fgets_test.c 12 Jul 2014 04:29:16 
-
@@ -49,11 +49,12 @@ main(void)
strlcpy(sfn, /tmp/barnacles., sizeof(sfn));
if ((fd = mkstemp(sfn)) == -1 ||
(sfp = fdopen(fd, w+)) == NULL) {
+   int saved_errno = errno;
if (fd != -1) {
unlink(sfn);
close(fd);
}
-   err(1, could not open temporary file);
+   errc(1, saved_errno, could not open temporary file);
}
 
for (i = 0; i  4096 * THREAD_COUNT; i++)
Index: regress/lib/libc/stdio_threading/fputs/fputs_test.c
===
RCS file: /cvs/src/regress/lib/libc/stdio_threading/fputs/fputs_test.c,v
retrieving revision 1.1
diff -u -p -d -r1.1 fputs_test.c
--- regress/lib/libc/stdio_threading/fputs/fputs_test.c 19 Nov 2009 08:06:06 
-  1.1
+++ regress/lib/libc/stdio_threading/fputs/fputs_test.c 12 Jul 2014 04:29:16 
-
@@ -46,11 +46,12 @@ main(void)
strlcpy(sfn, /tmp/barnacles., sizeof(sfn));
if ((fd = mkstemp(sfn)) == -1 ||
(sfp = fdopen(fd, w+)) == NULL) {
+   int saved_errno = errno;
if (fd != -1) {
unlink(sfn);
close(fd);
}
-   err(1, could not open temporary file);
+   errc(1, saved_errno, could not open temporary file);
}
 
run_threads(fputs_thread, sfp);
Index: regress/lib/libc/stdio_threading/fread/fread_test.c
===
RCS file: /cvs/src/regress/lib/libc/stdio_threading/fread/fread_test.c,v
retrieving revision 1.1
diff -u -p -d -r1.1 fread_test.c
--- 

Re: identcpu for 1-GByte pages

2014-07-14 Thread Doug Hogan
On Wed, Jul 02, 2014 at 10:36:25PM -0700, Matthew Dempsky wrote:
 According to the Intel 64 and IA-32 Architectures Software
 Developer's Manual, CPUID.8001H:EDX.Page1GB [bit 26] indicates
 whether 1-GByte pages are supported with IA-32e paging.
 
 I think the diff below adds support for identifying this feature in
 dmesg, but my X201s is seemingly to old to support it.

Works for me with the last snapshot:

OpenBSD 5.5-current (GENERIC.MP) #272: Sun Jul 13 20:46:20 MDT 2014
t...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 17042100224 (16252MB)
avail mem = 16579661824 (15811MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xec120 (82 entries)
bios0: vendor American Megatrends Inc. version 1.1a date 01/03/2014
bios0: Supermicro X10SAE
acpi0 at bios0: rev 2
acpi0: sleep states S0 S1 S3 S4 S5
acpi0: tables DSDT FACP APIC FPDT LPIT SSDT SSDT SSDT SSDT MCFG HPET SSDT SSDT 
ASF! DMAR EINJ ERST HEST BERT
acpi0: wakeup devices PS2K(S3) PS2M(S3) PXSX(S4) RP01(S4) PXSX(S4) RP02(S4) 
PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) BR30(S4) 
RP06(S4) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E3-1246 v3 @ 3.50GHz, 3500.44 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 100MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4, IBE



Re: PATCH: further kernel malloc - mallocarray

2014-07-16 Thread Doug Hogan
On Tue, Jul 15, 2014 at 11:34:01PM -0700, patrick keshishian wrote:
 For obvious cases such as this, is it worth converting?

Maybe not.  I left it since it is an array.

 might be safer to change this (in a separate diff) to:
 
   dc-dc_bs = mallocarray(ri-ri_rows,
   ri-ri_cols * sizeof(struct wsdisplay_charcell),
   ...

That was a mistake.  You're right.

 Is the bit of `sizeof(mfi_bbu_indicators)' a bug in
 the original source code?
 should it not be `nitems(mfi_bbu_indicators)'?

I forgot to mention that.  I think it's a bug since
mfi_bbu_indicators is defined as:

static const char *mfi_bbu_indicators[] = {
...
};



PATCH: fix open_memstream's regression test

2014-07-17 Thread Doug Hogan
Ran into this while running the regression suite for another patch.

The regression test for open_memstream causes a SIGBUS because it closes
the stream too early.

Replaced fclose with fflush since they both update pbuf and psize.
Added a similar check for fclose at the end.

Index: regress/lib/libc/open_memstream/open_memstreamtest.c
===
RCS file: /cvs/src/regress/lib/libc/open_memstream/open_memstreamtest.c,v
retrieving revision 1.3
diff -u -p -d -r1.3 open_memstreamtest.c
--- regress/lib/libc/open_memstream/open_memstreamtest.c28 Mar 2013 
09:35:58 -  1.3
+++ regress/lib/libc/open_memstream/open_memstreamtest.c18 Jul 2014 
02:17:58 -
@@ -127,8 +127,8 @@ main(void)
failures++;
}
 
-   if (fclose(fp) == EOF) {
-   warnx(fclose failed. (17));
+   if (fflush(fp) == EOF) {
+   warnx(fflush failed. (17));
failures++;
}
 
@@ -156,6 +156,22 @@ main(void)
 
if (ftell(fp) != 7) {
warnx(failed seeking backward. (22));
+   failures++;
+   }
+
+   if (fseek(fp, 5, SEEK_CUR) != 0) {
+   warnx(failed to fseek. (23));
+   failures++;
+   }
+
+   if (fclose(fp) == EOF) {
+   warnx(fclose failed. (24));
+   failures++;
+   }
+
+   if (size != 12) {
+   warnx(failed, size %zu should be %u.  (25),
+   size, 12);
failures++;
}
 



Re: lynx: disable old protocols

2014-07-19 Thread Doug Hogan
On Sat, Jul 19, 2014 at 12:28:17PM +0100, Stuart Henderson wrote:
 Personally I remember a few nearby mirror URLs, but I do think this could
 be improved - we could add a sample pkg.conf file to /etc/examples with
 a list of mirrors updated from mirrors.dat. Unless there are objections to
 that idea, I'll look at modifying the scripts for this.

This addresses the list of package mirrors.  What about the list of anoncvs
mirrors?  That's the other task I would sometimes use lynx for.  If I'm
installing on a machine in a different location, I'd like to use a closer
mirror than the ones I memorized.

You could have mirrors.dat as a one time update since the packages are
signed.  The main issues would be if a mirror wants to get added/deleted
in between releases or if some mirror is compromised and stops sending
out new packages.

It's different with anoncvs since we're relying on the ssh pubkeys and
the updates are not signed.  Would it make sense to have a package that
contains a list of the anoncvs mirrors + ssh fingerprints?  The list would
be signed and updated in the same manner as other packages.  It may make
sense to throw mirrors.dat in there so it is signed and updated as well.
The first mirrors.dat update is bootstrapped from the installation and
then updated as a package.



PATCH: regress/libexec/ld.so/randomdata fix

2014-07-19 Thread Doug Hogan
The regression tests under randomdata do not work.  They must build
libaa before running.  libaa doesn't have a target for regress so
even though it's a SUBDIR in ../Makefile, it doesn't do anything.


Index: regress/libexec/ld.so/randomdata/libaa/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/randomdata/libaa/Makefile,v
retrieving revision 1.1.1.1
diff -u -p -d -r1.1.1.1 Makefile
--- regress/libexec/ld.so/randomdata/libaa/Makefile 16 Aug 2012 17:21:05 
-  1.1.1.1
+++ regress/libexec/ld.so/randomdata/libaa/Makefile 19 Jul 2014 08:27:34 
-
@@ -3,4 +3,6 @@
 LIB= aa
 SRCS+= aa.c
 
+regress: all
+
 .include bsd.lib.mk



PATCH: regress/libexec/ld.so/initfirst fix

2014-07-19 Thread Doug Hogan
Initfirst needs the same fix as randomdata.


Index: regress/libexec/ld.so/initfirst/test2/libaa/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libaa/Makefile,v
retrieving revision 1.1
diff -u -p -d -r1.1 Makefile
--- regress/libexec/ld.so/initfirst/test2/libaa/Makefile29 Nov 2011 
04:36:15 -  1.1
+++ regress/libexec/ld.so/initfirst/test2/libaa/Makefile19 Jul 2014 
16:50:13 -
@@ -2,4 +2,7 @@ LIB=aa
 SRCS= aa.C
 LDADD=-L../libab
 LDADD+=-lab
+
+regress: all
+
 .include bsd.lib.mk
Index: regress/libexec/ld.so/initfirst/test2/libab/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libab/Makefile,v
retrieving revision 1.1
diff -u -p -d -r1.1 Makefile
--- regress/libexec/ld.so/initfirst/test2/libab/Makefile29 Nov 2011 
04:36:15 -  1.1
+++ regress/libexec/ld.so/initfirst/test2/libab/Makefile19 Jul 2014 
16:50:13 -
@@ -3,4 +3,7 @@ SRCS= ab.C
 LDADD=-Wl,-z,initfirst
 LDADD+=-L../libac
 LDADD+=-lac
+
+regress: all
+
 .include bsd.lib.mk
Index: regress/libexec/ld.so/initfirst/test2/libac/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libac/Makefile,v
retrieving revision 1.1
diff -u -p -d -r1.1 Makefile
--- regress/libexec/ld.so/initfirst/test2/libac/Makefile29 Nov 2011 
04:36:15 -  1.1
+++ regress/libexec/ld.so/initfirst/test2/libac/Makefile19 Jul 2014 
16:50:13 -
@@ -2,4 +2,7 @@ LIB=ac
 SRCS= ac.C
 LDADD=-L../libad
 LDADD+=-lad
+
+regress: all
+
 .include bsd.lib.mk
Index: regress/libexec/ld.so/initfirst/test2/libad/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libad/Makefile,v
retrieving revision 1.1
diff -u -p -d -r1.1 Makefile
--- regress/libexec/ld.so/initfirst/test2/libad/Makefile29 Nov 2011 
04:36:15 -  1.1
+++ regress/libexec/ld.so/initfirst/test2/libad/Makefile19 Jul 2014 
16:50:13 -
@@ -3,4 +3,7 @@ SRCS= ad.C
 LDADD=-Wl,-z,initfirst
 LDADD+=-L../libae
 LDADD+=-lae
+
+regress: all
+
 .include bsd.lib.mk
Index: regress/libexec/ld.so/initfirst/test2/libae/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libae/Makefile,v
retrieving revision 1.1
diff -u -p -d -r1.1 Makefile
--- regress/libexec/ld.so/initfirst/test2/libae/Makefile29 Nov 2011 
04:36:15 -  1.1
+++ regress/libexec/ld.so/initfirst/test2/libae/Makefile19 Jul 2014 
16:50:13 -
@@ -1,3 +1,6 @@
 LIB=ae
 SRCS= ae.C
+
+regress: all
+
 .include bsd.lib.mk



PATCH: regress/sys/kern/rcvtimeo

2014-07-19 Thread Doug Hogan

Index: regress/sys/kern/rcvtimeo/rcvtimeo.c
===
RCS file: /cvs/src/regress/sys/kern/rcvtimeo/rcvtimeo.c,v
retrieving revision 1.4
diff -u -p -r1.4 rcvtimeo.c
--- regress/sys/kern/rcvtimeo/rcvtimeo.c2 Sep 2003 23:52:17 -   
1.4
+++ regress/sys/kern/rcvtimeo/rcvtimeo.c19 Jul 2014 17:16:09 -
@@ -11,6 +11,7 @@
 #include unistd.h
 #include stdio.h
 #include stdlib.h
+#include string.h
 #include errno.h
 #include err.h
 
@@ -40,6 +41,7 @@ main(int argc, char *argv[])
if ((s = socket(AF_INET, SOCK_DGRAM, 0))  0)
err(1, socket);
 
+   memset(sin, 0, sizeof(sin));
sin.sin_len = sizeof(sin);
sin.sin_family = AF_INET;
sin.sin_port = htons(3);/* XXX assuming nothing is there */



PATCH: fix regress skip slow

2014-07-19 Thread Doug Hogan
REGRESS_SKIP_SLOW didn't work as intended.  Default is no yet it checks
for !empty.

Added t-exhaust as slow because it takes  90 minutes on my recent CPU.


Index: regress/lib/libc/regex/Makefile
===
RCS file: /cvs/src/regress/lib/libc/regex/Makefile,v
retrieving revision 1.8
diff -u -p -d -r1.8 Makefile
--- regress/lib/libc/regex/Makefile 5 Nov 2011 15:07:12 -   1.8
+++ regress/lib/libc/regex/Makefile 19 Jul 2014 17:38:29 -
@@ -11,6 +11,7 @@ CFLAGS+= -I${.CURDIR}/../../../../lib/li
 TESTS= ${.CURDIR}/tests
 
 REGRESS_TARGETS=do-reg do-reg-long do-reg-backref do-t_exhaust
+REGRESS_SLOW_TARGETS=do-t_exhaust
 
 do-reg: ${PROG}
./re  ${TESTS}
Index: share/mk/bsd.regress.mk
===
RCS file: /cvs/src/share/mk/bsd.regress.mk,v
retrieving revision 1.12
diff -u -p -d -r1.12 bsd.regress.mk
--- share/mk/bsd.regress.mk 1 Aug 2013 20:43:07 -   1.12
+++ share/mk/bsd.regress.mk 19 Jul 2014 17:38:31 -
@@ -55,7 +55,7 @@ REGRESS_SKIP_TARGETS=run-regress-${PROG}
 .  endif
 .endif
 
-.if defined(REGRESS_SLOW_TARGETS)  !empty(REGRESS_SKIP_SLOW)
+.if defined(REGRESS_SLOW_TARGETS)  ${REGRESS_SKIP_SLOW} != no
 REGRESS_SKIP_TARGETS+=${REGRESS_SLOW_TARGETS}
 .endif
 



Re: PATCH: regress/libexec/ld.so/randomdata fix

2014-07-19 Thread Doug Hogan
On Sat, Jul 19, 2014 at 06:03:18PM +, Miod Vallat wrote:
 They do; the magic to build libaa comes from
 regress/libexec/ld.so/randomdata/Makefile.inc .
 
 There are no regress tests to run in the libaa directory itself, but it
 needs to be in SUBDIR to get the obj directory and symlink created.

The problem is that even though libaa is in SUBDIR, it doesn't do
anything when make is called in libaa.  It runs 'make regress' in libaa and
libaa doesn't have a regress target so it's a nop.

I have a log from a partial run of 'make regression-tests' (I hit a bug
in kqueue which hangs indefinitely) and it showed up as failing.  I went
into libexec/ld.so/randomdata and ran 'make depend regress' and it doesn't
work without that patch.

It would get built if something called 'make' since libaa would have a
matching target then.  I checked the top level /usr/src/Makefile and the
parent of randomdata and they do not call the default target.

I just installed an i386 snapshot on a laptop (no local changes yet) and
tried it against the latest -current.  I still see this behavior when
I go into the directory and run 'make depend regress'.  It's working
for you?



Re: PATCH: regress/libexec/ld.so/randomdata fix

2014-07-19 Thread Doug Hogan
On Sat, Jul 19, 2014 at 06:59:14PM +, Miod Vallat wrote:
 Ah, I see; you're running `make regress' ; simply running `make' should
 work correctly (it implies building, and the running the regress target,
 when Makefile includes bsd.regress.mk).

That would fix it for the local case inside of /usr/src/regress.  Wouldn't
it still be an issue at the top level /usr/src/Makefile when you run
'make regression-tests' which calls make depend  make regress instead
of make?

I looked at the comments in /usr/src/regress/Makefile and it says to run
'make depend' followed by 'make regress'.  I went looking in the tree for
other cases where there's a lib dependency.
/usr/src/regress/misc/sse2/libbar has 'regress: all'.

Let me know which way you would prefer it fixed and I'll submit a new
patch.



Re: PATCH: fix regress skip slow

2014-07-19 Thread Doug Hogan
On Sat, Jul 19, 2014 at 06:55:02PM +, Miod Vallat wrote:
 What flags were you using? As its name implies, this test causes a lot
 of memory to be allocated, so malloc flags such as F and G will take a
 toll...

I was using G since I had S enabled.

  I think that was the cause of the kqueue hang I was seeing.  I launched
  the regression tests and came back hours later and they were still
  running.  All CPU cores were at 100% idle and it said it was still
  running kqueue.  I just ran it on this i386 snapshot without any malloc.conf
  settings and it ran quickly.
 
 ... but they should not cause the system to be idle nevertheless. So
 this is worth investigating.

Ok, I'll take a closer look then.  There's only a few failing tests left.



PATCH: sys/kern/subr_extent.c

2014-07-19 Thread Doug Hogan
regress/sys/kern/extent is failing because of the userland definitions
in sys/kern/subr_extent.c.

Even though subr_extent.c includes sys/pool.h, the struct pool definition
is guarded for _KERNEL and _LIBKVM.  I added a simple struct pool
definition that meets the needs of the dummy pool_* functions.  The
field pr_size is the same type as the real struct pool's.

Changed the 2 argument free()s to match the kernel free().


Index: sys/kern/subr_extent.c
===
RCS file: /cvs/src/sys/kern/subr_extent.c,v
retrieving revision 1.51
diff -u -p -d -r1.51 subr_extent.c
--- sys/kern/subr_extent.c  12 Jul 2014 18:43:32 -  1.51
+++ sys/kern/subr_extent.c  20 Jul 2014 00:51:11 -
@@ -58,14 +58,18 @@
 #include string.h
 
 #definemalloc(s, t, flags) malloc(s)
-#definefree(p, t)  free(p)
+#definefree(p, t, s)   free(p)
 #definetsleep(chan, pri, str, timo)(EWOULDBLOCK)
 #definewakeup(chan)((void)0)
 #definepool_get(pool, flags)   malloc((pool)-pr_size, 0, 0)
 #definepool_init(a, b, c, d, e, f, g)  (a)-pr_size = (b)
 #definepool_setipl(pool, ipl)  /* nothing */
-#definepool_put(pool, rp)  free((rp), 0)
+#definepool_put(pool, rp)  free((rp), 0, 0)
 #definepanic   printf
+
+struct pool {
+   unsigned int pr_size;
+};
 #endif
 
 #if defined(DIAGNOSTIC) || defined(DDB)



Re: PATCH: avoid clobbering errno before err/warn

2014-07-19 Thread Doug Hogan
On Sat, Jul 19, 2014 at 06:40:54PM -0700, Philip Guenther wrote:
 Nice job!  Committed (except for the microcode/bwi file where your diff
 revealed another existing bug that we can fix at the same time...)

Thanks!  Here's the patch with the existing bug fixed.  I checked other
places for err/warn in this file but I think I'm already touching them
all.


Index: sys/dev/microcode/bwi/build/build.c
===
RCS file: /cvs/src/sys/dev/microcode/bwi/build/build.c,v
retrieving revision 1.3
diff -u -p -d -r1.3 build.c
--- sys/dev/microcode/bwi/build/build.c 12 Jul 2014 19:01:49 -  1.3
+++ sys/dev/microcode/bwi/build/build.c 20 Jul 2014 01:56:52 -
@@ -20,6 +20,7 @@
 #include sys/stat.h
 
 #include err.h
+#include errno.h
 #include fcntl.h
 #include stdio.h
 #include stdlib.h
@@ -104,12 +105,14 @@ main(int argc, char *argv[])
 
/* write header */
if (write(fdout, nfiles, sizeof(nfiles))  1) {
+   int saved_errno = errno;
close(fdout);
-   err(1, write header 1 to output file failed\n);
+   errc(1, saved_errno, write header 1 to output file failed);
}
if (write(fdout, h, headersize - sizeof(nfiles))  1) {
+   int saved_errno = errno;
close(fdout);
-   err(1, write header 2 to output file failed\n);
+   errc(1, saved_errno, write header 2 to output file failed);
}
 
/* network to host byte order */
@@ -122,25 +125,29 @@ main(int argc, char *argv[])
/* write each file */
for (i = 0; i  nfiles; i++) {
if ((fdin = open(h[i].filename, O_RDONLY)) == -1) {
+   int saved_errno = errno;
close(fdout);
-   err(1, open input file failed\n);
+   errc(1, saved_errno, open input file failed);
}
if ((p = malloc(h[i].filesize)) == NULL) {
+   int saved_errno = errno;
close(fdout);
close(fdin);
-   err(1, malloc);
+   errc(1, saved_errno, malloc);
}
if (read(fdin, p, h[i].filesize)  1) {
+   int saved_errno = errno;
free(p);
close(fdout);
close(fdin);
-   err(1, read input file failed\n);
+   errc(1, saved_errno, read input file failed);
}
if (write(fdout, p, h[i].filesize)  1) {
+   int saved_errno = errno;
free(p);
close(fdout);
close(fdin);
-   err(1, write to output file failed\n);
+   errc(1, saved_errno, write to output file failed);
}
free(p);
close(fdin);



PATCH: reallocarray in a few places in sys/

2014-07-21 Thread Doug Hogan
Use reallocarray() in a few places in sys.


Index: sys/arch/hppa/stand/mkboot/mkboot.c
===
RCS file: /cvs/src/sys/arch/hppa/stand/mkboot/mkboot.c,v
retrieving revision 1.18
diff -u -p -d -r1.18 mkboot.c
--- sys/arch/hppa/stand/mkboot/mkboot.c 22 Jan 2014 09:18:04 -  1.18
+++ sys/arch/hppa/stand/mkboot/mkboot.c 22 Jul 2014 01:11:23 -
@@ -204,10 +204,11 @@ putfile(char *from_file, int to)
if (n != sizeof (elf_header))
err(1, %s: reading ELF header, from_file);
header_count = ntohs(elf_header.e_phnum);
-   memory_needed = header_count * sizeof (*elf_segments);
-   elf_segments = malloc(memory_needed);
+   elf_segments = reallocarray(NULL, header_count,
+   sizeof(*elf_segments));
if (elf_segments == NULL)
err(1, malloc);
+   memory_needed = header_count * sizeof(*elf_segments);
(void) lseek(from, ntohl(elf_header.e_phoff), SEEK_SET);
n = read(from, elf_segments, memory_needed);
if (n != memory_needed)
Index: sys/arch/hppa64/stand/mkboot/mkboot.c
===
RCS file: /cvs/src/sys/arch/hppa64/stand/mkboot/mkboot.c,v
retrieving revision 1.4
diff -u -p -d -r1.4 mkboot.c
--- sys/arch/hppa64/stand/mkboot/mkboot.c   22 Jan 2014 09:26:55 -  
1.4
+++ sys/arch/hppa64/stand/mkboot/mkboot.c   22 Jul 2014 01:11:24 -
@@ -204,10 +204,11 @@ putfile(char *from_file, int to)
if (n != sizeof (elf_header))
err(1, %s: reading ELF header, from_file);
header_count = betoh32(elf_header.e_phnum);
-   memory_needed = header_count * sizeof (*elf_segments);
-   elf_segments = malloc(memory_needed);
+   elf_segments = reallocarray(NULL, header_count,
+   sizeof(*elf_segments));
if (elf_segments == NULL)
err(1, malloc);
+   memory_needed = header_count * sizeof(*elf_segments);
(void) lseek(from, betoh32(elf_header.e_phoff), SEEK_SET);
n = read(from, elf_segments, memory_needed);
if (n != memory_needed)
Index: sys/dev/microcode/bwi/extract/extract.c
===
RCS file: /cvs/src/sys/dev/microcode/bwi/extract/extract.c,v
retrieving revision 1.3
diff -u -p -d -r1.3 extract.c
--- sys/dev/microcode/bwi/extract/extract.c 12 Jul 2014 19:01:49 -  
1.3
+++ sys/dev/microcode/bwi/extract/extract.c 22 Jul 2014 01:14:43 -
@@ -53,7 +53,7 @@ main(int argc, char *argv[])
nfiles = ntohl(nfiles);
 
/* allocate space for header struct */
-   if ((h = malloc(nfiles * sizeof(*h))) == NULL)
+   if ((h = reallocarray(NULL, nfiles, sizeof(*h))) == NULL)
err(1, malloc);
for (i = 0; i  nfiles; i++) {
if ((h[i] = malloc(sizeof(struct header))) == NULL)



PATCH: Remove useless if !NULL check before BIO_free() in LibreSSL

2014-07-21 Thread Doug Hogan
BIO_free() returns immediately when the sole input is NULL.


Index: lib/libssl/src/apps/apps.c
===
RCS file: /cvs/src/lib/libssl/src/apps/apps.c,v
retrieving revision 1.68
diff -u -p -d -r1.68 apps.c
--- lib/libssl/src/apps/apps.c  19 Jul 2014 03:40:26 -  1.68
+++ lib/libssl/src/apps/apps.c  22 Jul 2014 05:14:46 -
@@ -669,8 +669,7 @@ end:
BIO_printf(err, unable to load certificate\n);
ERR_print_errors(err);
}
-   if (cert != NULL)
-   BIO_free(cert);
+   BIO_free(cert);
return (x);
 }
 
@@ -745,8 +744,7 @@ load_key(BIO *err, const char *file, int
goto end;
}
 end:
-   if (key != NULL)
-   BIO_free(key);
+   BIO_free(key);
if (pkey == NULL) {
BIO_printf(err, unable to load %s\n, key_descrip);
ERR_print_errors(err);
@@ -833,8 +831,7 @@ load_pubkey(BIO *err, const char *file, 
}
 
 end:
-   if (key != NULL)
-   BIO_free(key);
+   BIO_free(key);
if (pkey == NULL)
BIO_printf(err, unable to load %s\n, key_descrip);
return (pkey);
Index: lib/libssl/src/apps/dh.c
===
RCS file: /cvs/src/lib/libssl/src/apps/dh.c,v
retrieving revision 1.25
diff -u -p -d -r1.25 dh.c
--- lib/libssl/src/apps/dh.c14 Jul 2014 00:35:10 -  1.25
+++ lib/libssl/src/apps/dh.c22 Jul 2014 05:14:46 -
@@ -297,8 +297,7 @@ bad:
ret = 0;
 
 end:
-   if (in != NULL)
-   BIO_free(in);
+   BIO_free(in);
if (out != NULL)
BIO_free_all(out);
if (dh != NULL)
Index: lib/libssl/src/apps/dhparam.c
===
RCS file: /cvs/src/lib/libssl/src/apps/dhparam.c,v
retrieving revision 1.33
diff -u -p -d -r1.33 dhparam.c
--- lib/libssl/src/apps/dhparam.c   14 Jul 2014 00:35:10 -  1.33
+++ lib/libssl/src/apps/dhparam.c   22 Jul 2014 05:14:47 -
@@ -441,8 +441,7 @@ bad:
ret = 0;
 
 end:
-   if (in != NULL)
-   BIO_free(in);
+   BIO_free(in);
if (out != NULL)
BIO_free_all(out);
if (dh != NULL)
Index: lib/libssl/src/apps/dsa.c
===
RCS file: /cvs/src/lib/libssl/src/apps/dsa.c,v
retrieving revision 1.28
diff -u -p -d -r1.28 dsa.c
--- lib/libssl/src/apps/dsa.c   14 Jul 2014 00:35:10 -  1.28
+++ lib/libssl/src/apps/dsa.c   22 Jul 2014 05:14:55 -
@@ -320,8 +320,7 @@ bad:
} else
ret = 0;
 end:
-   if (in != NULL)
-   BIO_free(in);
+   BIO_free(in);
if (out != NULL)
BIO_free_all(out);
if (dsa != NULL)
Index: lib/libssl/src/apps/dsaparam.c
===
RCS file: /cvs/src/lib/libssl/src/apps/dsaparam.c,v
retrieving revision 1.34
diff -u -p -d -r1.34 dsaparam.c
--- lib/libssl/src/apps/dsaparam.c  14 Jul 2014 00:35:10 -  1.34
+++ lib/libssl/src/apps/dsaparam.c  22 Jul 2014 05:15:02 -
@@ -382,8 +382,7 @@ bad:
ret = 0;
 
 end:
-   if (in != NULL)
-   BIO_free(in);
+   BIO_free(in);
if (out != NULL)
BIO_free_all(out);
if (dsa != NULL)
Index: lib/libssl/src/apps/ec.c
===
RCS file: /cvs/src/lib/libssl/src/apps/ec.c,v
retrieving revision 1.16
diff -u -p -d -r1.16 ec.c
--- lib/libssl/src/apps/ec.c14 Jul 2014 00:35:10 -  1.16
+++ lib/libssl/src/apps/ec.c22 Jul 2014 05:15:02 -
@@ -328,8 +328,7 @@ bad:
} else
ret = 0;
 end:
-   if (in)
-   BIO_free(in);
+   BIO_free(in);
if (out)
BIO_free_all(out);
if (eckey)
Index: lib/libssl/src/apps/ecparam.c
===
RCS file: /cvs/src/lib/libssl/src/apps/ecparam.c,v
retrieving revision 1.23
diff -u -p -d -r1.23 ecparam.c
--- lib/libssl/src/apps/ecparam.c   14 Jul 2014 00:35:10 -  1.23
+++ lib/libssl/src/apps/ecparam.c   22 Jul 2014 05:15:03 -
@@ -578,8 +578,7 @@ end:
if (ec_cofactor)
BN_free(ec_cofactor);
free(buffer);
-   if (in != NULL)
-   BIO_free(in);
+   BIO_free(in);
if (out != NULL)
BIO_free_all(out);
if (group != NULL)
Index: lib/libssl/src/apps/enc.c
===
RCS file: /cvs/src/lib/libssl/src/apps/enc.c,v
retrieving revision 1.38
diff -u -p -d -r1.38 enc.c
--- lib/libssl/src/apps/enc.c   14 Jul 2014 00:35:10 -  1.38
+++ lib/libssl/src/apps/enc.c   22 Jul 2014 05:15:04 -
@@ -600,17 +600,13 @@ end:

PATCH: Avoid useless if != NULL check on BUF_MEM_free() in LibreSSL

2014-07-22 Thread Doug Hogan
BUF_MEM_free() only has one parameter and it returns immediately
if it is NULL.


Index: src/crypto/asn1/a_d2i_fp.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_d2i_fp.c,v
retrieving revision 1.11
diff -u -p -d -r1.11 a_d2i_fp.c
--- src/crypto/asn1/a_d2i_fp.c  13 Jul 2014 11:10:20 -  1.11
+++ src/crypto/asn1/a_d2i_fp.c  22 Jul 2014 06:12:57 -
@@ -99,8 +99,7 @@ ASN1_d2i_bio(void *(*xnew)(void), d2i_of
ret = d2i(x, p, len);
 
 err:
-   if (b != NULL)
-   BUF_MEM_free(b);
+   BUF_MEM_free(b);
return (ret);
 }
 
@@ -122,8 +121,7 @@ ASN1_item_d2i_bio(const ASN1_ITEM *it, B
ret = ASN1_item_d2i(x, p, len, it);
 
 err:
-   if (b != NULL)
-   BUF_MEM_free(b);
+   BUF_MEM_free(b);
return (ret);
 }
 
@@ -265,7 +263,6 @@ asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
return off;
 
 err:
-   if (b != NULL)
-   BUF_MEM_free(b);
+   BUF_MEM_free(b);
return -1;
 }
Index: src/crypto/conf/conf_def.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/conf/conf_def.c,v
retrieving revision 1.28
diff -u -p -d -r1.28 conf_def.c
--- src/crypto/conf/conf_def.c  11 Jul 2014 15:38:03 -  1.28
+++ src/crypto/conf/conf_def.c  22 Jul 2014 06:13:02 -
@@ -412,14 +412,12 @@ again:
v = NULL;
}
}
-   if (buff != NULL)
-   BUF_MEM_free(buff);
+   BUF_MEM_free(buff);
free(section);
return (1);
 
 err:
-   if (buff != NULL)
-   BUF_MEM_free(buff);
+   BUF_MEM_free(buff);
free(section);
if (line != NULL)
*line = eline;
@@ -614,8 +612,7 @@ str_copy(CONF *conf, char *section, char
return (1);
 
 err:
-   if (buf != NULL)
-   BUF_MEM_free(buf);
+   BUF_MEM_free(buf);
return (0);
 }
 
Index: src/crypto/txt_db/txt_db.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/txt_db/txt_db.c,v
retrieving revision 1.18
diff -u -p -d -r1.18 txt_db.c
--- src/crypto/txt_db/txt_db.c  11 Jul 2014 08:44:49 -  1.18
+++ src/crypto/txt_db/txt_db.c  22 Jul 2014 06:13:06 -
@@ -287,8 +287,7 @@ TXT_DB_write(BIO *out, TXT_DB *db)
ret = tot;
 
 err:
-   if (buf != NULL)
-   BUF_MEM_free(buf);
+   BUF_MEM_free(buf);
return (ret);
 }
 
Index: src/crypto/x509/by_dir.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/x509/by_dir.c,v
retrieving revision 1.32
diff -u -p -d -r1.32 by_dir.c
--- src/crypto/x509/by_dir.c11 Jul 2014 08:44:49 -  1.32
+++ src/crypto/x509/by_dir.c22 Jul 2014 06:13:08 -
@@ -200,8 +200,7 @@ free_dir(X509_LOOKUP *lu)
a = (BY_DIR *)lu-method_data;
if (a-dirs != NULL)
sk_BY_DIR_ENTRY_pop_free(a-dirs, by_dir_entry_free);
-   if (a-buffer != NULL)
-   BUF_MEM_free(a-buffer);
+   BUF_MEM_free(a-buffer);
free(a);
 }
 
@@ -426,7 +425,6 @@ get_cert_by_subject(X509_LOOKUP *xl, int
}
}
 finish:
-   if (b != NULL)
-   BUF_MEM_free(b);
+   BUF_MEM_free(b);
return (ok);
 }
Index: src/crypto/x509/x509_obj.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/x509/x509_obj.c,v
retrieving revision 1.16
diff -u -p -d -r1.16 x509_obj.c
--- src/crypto/x509/x509_obj.c  11 Jul 2014 08:44:49 -  1.16
+++ src/crypto/x509/x509_obj.c  22 Jul 2014 06:13:08 -
@@ -173,7 +173,6 @@ X509_NAME_oneline(X509_NAME *a, char *bu
 
 err:
X509err(X509_F_X509_NAME_ONELINE, ERR_R_MALLOC_FAILURE);
-   if (b != NULL)
-   BUF_MEM_free(b);
+   BUF_MEM_free(b);
return (NULL);
 }
Index: src/ssl/d1_clnt.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/d1_clnt.c,v
retrieving revision 1.31
diff -u -p -d -r1.31 d1_clnt.c
--- src/ssl/d1_clnt.c   12 Jul 2014 22:33:39 -  1.31
+++ src/ssl/d1_clnt.c   22 Jul 2014 06:13:30 -
@@ -747,8 +747,7 @@ end:
s-in_handshake, NULL);
 #endif
 
-   if (buf != NULL)
-   BUF_MEM_free(buf);
+   BUF_MEM_free(buf);
if (cb != NULL)
cb(s, SSL_CB_CONNECT_EXIT, ret);
return (ret);
Index: src/ssl/s23_clnt.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/s23_clnt.c,v
retrieving revision 1.31
diff -u -p -d -r1.31 s23_clnt.c
--- src/ssl/s23_clnt.c  11 Jul 2014 08:17:36 -  1.31
+++ src/ssl/s23_clnt.c  22 Jul 2014 06:13:33 -
@@ -278,8 +278,7 @@ ssl23_connect(SSL *s)
}
 end:
s-in_handshake--;
-   if (buf != NULL)
-   BUF_MEM_free(buf);
+   

Re: PATCH: overflow behavior in malloc(9)

2014-07-22 Thread Doug Hogan
On Tue, Jul 22, 2014 at 02:51:17AM -0400, Jean-Philippe Ouellet wrote:
 That is misleading in the M_CANFAIL case.
 
 I'm not terribly good at wording things, but I suggest something
 more like this instead:

Hmm I think it's only misleading in the M_CANFAIL case.  I think this
diff makes it a little more complex than it needs to be.  What do you
think about leaving the malloc option section as-is and instead
explain how mallocarray() operates before it calls malloc()?

Something along these lines: mallocarray(9) is a wrapper around
malloc(9) that checks for overflow.  If arithmetic overflow is detected,
it returns NULL when M_CANFAIL is enabled or else calls panic().
Otherwise, it has the same behavior as malloc.

Does that work?



Re: syslogd libevent handler

2014-09-03 Thread Doug Hogan
On Sun, Aug 31, 2014 at 10:46:50PM +0200, Alexander Bluhm wrote:
 Move the handlers for the poll events into separate functions.  They
 will become the libevent callbacks later.
...

 @@ -631,23 +606,65 @@ main(int argc, char *argv[])
  
   for (i = 0; i  nfunix; i++) {
   if ((pfd[PFD_UNIX_0 + i].revents  POLLIN) != 0) {
 - ssize_t rlen;
 -
 - len = sizeof(fromunix);
 - rlen = recvfrom(pfd[PFD_UNIX_0 + i].fd, line,
 - MAXLINE, 0, (struct sockaddr *)fromunix,
 - len);
 - if (rlen  0) {
 - line[rlen] = '\0';
 - printline(LocalHostName, line);
 - } else if (rlen == -1  errno != EINTR)
 - logerror(recvfrom unix);
 + udp_read_handler(pfd[PFD_UNIX_0 + i].fd);
...

Shouldn't this be a call to unix_read_handler() instead of
udp_read_handler()?



Re: [PATCH] src/usr.bin/ftp/fetch.c: free ressl_config

2014-09-12 Thread Doug Hogan
On Thu, Sep 11, 2014 at 11:05:02PM -0500, Kent R. Spillner wrote:
 While reviewing tedu@'s libressl config cleanup diffs I noticed we're
 not explicitly freeing ressl_config in ftp(1).
...
 Index: fetch.c
 ===
...
   if (ssl != NULL) {
   ressl_close(ssl);
 + ressl_config_free(ressl_config);

Hmm this doesn't look right to me.  ressl_config is not allocated the
same way as the other variables.  The other variables such as ssl,
sslhost, etc are local to that function and allocated there.
ressl_config is a global and only allocated in main.c.  It is allocated
once and configured using getopt/getsubopt().

If you free it, you would need to reallocate it each time or set it to
NULL so libressl will revert back and discard the user's changes.
I think either way is wrong.  The SSL configuration should be retained
even if a single url_get() fails (which isn't fatal).

The above code is from url_get() which is called in a loop inside
auto_fetch().  I think the above change would use the getsubopt()
configuration for the first HTTPS URL argument and then try to use freed
memory for the others.



Re: Marking CIRCLEQ_* macros as deprecated in queue(3)

2014-09-12 Thread Doug Hogan
On Fri, Sep 12, 2014 at 02:23:34PM -0600, Todd C. Miller wrote:
 No objection here.  I'd go a step farther and nuke all the FOO_END
 macros as well.  They add nothing and just make code harder to read.

In preparation for that, this replaces all queue_END macro calls.
Since we're not calling CIRCLEQ_* anywhere in the tree, there's no need
to keep the symmetry.

Ok?

Index: bin/md5/md5.c
===
RCS file: /cvs/src/bin/md5/md5.c,v
retrieving revision 1.76
diff -u -p -r1.76 md5.c
--- bin/md5/md5.c   19 Jun 2014 15:30:49 -  1.76
+++ bin/md5/md5.c   13 Sep 2014 01:44:57 -
@@ -263,7 +263,7 @@ main(int argc, char **argv)
strcmp(hf-name, hftmp-name) == 0)
break;
}
-   if (hftmp == TAILQ_END(hl))
+   if (hftmp == NULL)
hash_insert(hl, hf, base64);
}
break;
Index: lib/libevent/event-internal.h
===
RCS file: /cvs/src/lib/libevent/event-internal.h,v
retrieving revision 1.6
diff -u -p -r1.6 event-internal.h
--- lib/libevent/event-internal.h   21 Apr 2010 20:02:40 -  1.6
+++ lib/libevent/event-internal.h   13 Sep 2014 01:45:11 -
@@ -78,7 +78,7 @@ struct event_base {
 #defineTAILQ_NEXT(elm, field)  ((elm)-field.tqe_next)
 #define TAILQ_FOREACH(var, head, field)
\
for((var) = TAILQ_FIRST(head);  \
-   (var) != TAILQ_END(head);   \
+   (var) != NULL;  \
(var) = TAILQ_NEXT(var, field))
 #defineTAILQ_INSERT_BEFORE(listelm, elm, field) do {   
\
(elm)-field.tqe_prev = (listelm)-field.tqe_prev;  \
Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.638
diff -u -p -r1.638 parse.y
--- sbin/pfctl/parse.y  23 Aug 2014 00:11:03 -  1.638
+++ sbin/pfctl/parse.y  13 Sep 2014 01:45:15 -
@@ -1152,8 +1152,8 @@ tabledef  : TABLE '' STRING '' table_op
YYERROR;
}
free($3);
-   for (ti = SIMPLEQ_FIRST($5.init_nodes);
-   ti != SIMPLEQ_END($5.init_nodes); ti = nti) {
+   for (ti = SIMPLEQ_FIRST($5.init_nodes); ti != NULL;
+   ti = nti) {
if (ti-file)
free(ti-file);
for (h = ti-host; h != NULL; h = nh) {
Index: sbin/pfctl/pfctl_optimize.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_optimize.c,v
retrieving revision 1.33
diff -u -p -r1.33 pfctl_optimize.c
--- sbin/pfctl/pfctl_optimize.c 22 Nov 2013 04:12:48 -  1.33
+++ sbin/pfctl/pfctl_optimize.c 13 Sep 2014 01:45:16 -
@@ -844,7 +844,7 @@ block_feedback(struct pfctl *pf, struct 
break;
}
}
-   if (por2 == TAILQ_END(block-sb_rules))
+   if (por2 == NULL)
TAILQ_INSERT_TAIL(block-sb_rules, por1, por_entry);
}
 
Index: sys/arch/alpha/dev/bus_dma.c
===
RCS file: /cvs/src/sys/arch/alpha/dev/bus_dma.c,v
retrieving revision 1.33
diff -u -p -r1.33 bus_dma.c
--- sys/arch/alpha/dev/bus_dma.c12 Jul 2014 18:44:40 -  1.33
+++ sys/arch/alpha/dev/bus_dma.c13 Sep 2014 01:45:16 -
@@ -511,7 +511,7 @@ _bus_dmamem_alloc_range(t, size, alignme
segs[curseg].ds_len = PAGE_SIZE;
m = TAILQ_NEXT(m, pageq);
 
-   for (; m != TAILQ_END(mlist); m = TAILQ_NEXT(m, pageq)) {
+   for (; m != NULL; m = TAILQ_NEXT(m, pageq)) {
curaddr = VM_PAGE_TO_PHYS(m);
 #ifdef DIAGNOSTIC
if (curaddr  low || curaddr = high) {
Index: sys/arch/arm/arm/bus_dma.c
===
RCS file: /cvs/src/sys/arch/arm/arm/bus_dma.c,v
retrieving revision 1.26
diff -u -p -r1.26 bus_dma.c
--- sys/arch/arm/arm/bus_dma.c  12 Jul 2014 18:44:41 -  1.26
+++ sys/arch/arm/arm/bus_dma.c  13 Sep 2014 01:45:17 -
@@ -1033,7 +1033,7 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
 #endif /* DEBUG_DMA */
m = TAILQ_NEXT(m, pageq);
 
-   for (; m != TAILQ_END(mlist); m = TAILQ_NEXT(m, pageq)) {
+   for (; m != NULL; m = TAILQ_NEXT(m, pageq)) {
curaddr = VM_PAGE_TO_PHYS(m);
 #ifdef DIAGNOSTIC
   

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Doug Hogan
On Thu, Sep 18, 2014 at 09:28:41AM +0100, bytevolc...@safe-mail.net wrote:
 Revised diffbelow:

You are inheriting some bugs in here by reusing newfs code. :)

I agree with the comments from others.  Additional comments:

  #include util.h
 +#include paths.h
...
 + args.ta_root_mode = modeset ? mode : sb.st_mode;
 +
 + int newflags = mntflags;
...
 + if(template) newflags = ~MNT_RDONLY;
...
 + if(template) {

KNF

 +#include pathnames.h

This was too much copy/paste from newfs.  You aren't using any of
the definitions in here.

 + if (execv(cmd, argv) != 0)
 + err(1, %s, cmd);

execv() does not return when successful.  On error, it returns -1.

 + strlcpy(mountpoint, src, sizeof(mountpoint));

The strlcpy call should check for truncation.  There's no guarantee
that src is less than sizeof(mountpoint) since src = template = optarg.

 + tmp = getenv(TMPDIR);
 + if (tmp == NULL || *tmp == '\0')
 + tmp = _PATH_TMP;

It's being used with mkdtemp(), but I think you should still check
issetugid() before trusting getenv().

+   if (mntflags  MNT_RDONLY) {
+   mntflags |= MNT_UPDATE;
+   if (mount(MOUNT_TMPFS, canon_dir, mntflags, args)  0) 
{

This doesn't look right.  /usr/src/sys/tmpfs/tmpfs_vfsops.c doesn't
support MNT_UPDATE in tmpfs_mount().


I don't see a reason for canon_dev to exist.  It gets written to but isn't
used anywhere else.



Re: ressl: two way fds extention

2014-10-31 Thread Doug Hogan
On Sat, Nov 01, 2014 at 03:07:24AM +0100, Jan Klemkow wrote:
 Index: tls_client.c
 ===
 RCS file: /cvs/src/lib/libtls/tls_client.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 tls_client.c
 --- tls_client.c  31 Oct 2014 13:46:17 -  1.1
 +++ tls_client.c  1 Nov 2014 01:50:56 -
 @@ -123,6 +123,13 @@ err:
  int
  tls_connect_socket(struct tls *ctx, int socket, const char *hostname)
  {
 + return tls_connect_fds(ctx, socket, socket, hostname);
 +}

This changes the behavior of tls_connect_socket() and tls_connect().
Joel's diff set ctx-socket = socket before calling tls_connect_fds() so
it would behave the same.  When you call tls_close(ctx), it will close
ctx-socket in the existing code and Joel's diff.

I don't think you want to change the semantics like this.  I think
either tls_connect_fds() is the special case where you need to manually
close the sockets or tls_close() should close everything.  With the
above change, even people calling tls_connect() will need to save one of
the fd_(read|write) before calling tls_close() and then close the fd
afterward.



Re: Brainy: Kernel Memory Leak in sdmmc

2015-02-15 Thread Doug Hogan
On Sun, Feb 15, 2015 at 08:53:08PM +, Miod Vallat wrote:
 This ought to fix this problem:

This looks reasonable but needs a tweak if you want to keep the existing
behavior.  In the existing code, it has this section above the copyout():

rw_enter_write(sc-sc_lock);
if (request == SDIOCEXECMMC)
error = sdmmc_mmc_command(sc, cmd);
else
error = sdmmc_app_command(sc, cmd);
rw_exit(sc-sc_lock);
if (error  !cmd.c_error)
cmd.c_error = error;

and returns 0 even if error is non-zero.  With this diff, it will either
return the result of those functions or the result of copyout depending
on whether ucmd-c_data is non-zero.  Using the result of copyout is
intentional, but not the result from sdmmc_{mmc,app}_command().

Could you set error=0 after the above chunk or use a new variable?
Also, one of the new lines has space indents rather than a tab.

 Index: sdmmc.c
 ===
 RCS file: /cvs/src/sys/dev/sdmmc/sdmmc.c,v
 retrieving revision 1.36
 diff -u -p -r1.36 sdmmc.c
 --- sdmmc.c   1 Nov 2014 16:32:06 -   1.36
 +++ sdmmc.c   15 Feb 2015 20:52:08 -
 @@ -749,7 +749,7 @@ sdmmc_ioctl(struct device *self, u_long 
   struct sdmmc_command *ucmd;
   struct sdmmc_command cmd;
   void *data;
 - int error;
 + int error = 0;
  
   switch (request) {
  #ifdef SDMMC_DEBUG
 @@ -784,8 +784,9 @@ sdmmc_ioctl(struct device *self, u_long 
   M_WAITOK | M_CANFAIL);
   if (data == NULL)
   return ENOMEM;
 - if (copyin(ucmd-c_data, data, ucmd-c_datalen))
 - return EFAULT;
 + error = copyin(ucmd-c_data, data, ucmd-c_datalen);
 + if (error != 0)
 + goto exec_done;
  
   cmd.c_data = data;
   cmd.c_datalen = ucmd-c_datalen;
 @@ -804,10 +805,10 @@ sdmmc_ioctl(struct device *self, u_long 
   ucmd-c_flags = cmd.c_flags;
   ucmd-c_error = cmd.c_error;
  
 - if (ucmd-c_data  copyout(data, ucmd-c_data,
 - ucmd-c_datalen))
 - return EFAULT;
 + if (ucmd-c_data)
 +error = copyout(data, ucmd-c_data, ucmd-c_datalen);
  
 +exec_done:
   if (ucmd-c_data)
   free(data, M_TEMP, 0);
   break;
 @@ -815,7 +816,7 @@ sdmmc_ioctl(struct device *self, u_long 
   default:
   return ENOTTY;
   }
 - return 0;
 + return error;
  }
  #endif
  
 



New LibreSSL mailing lists

2015-06-03 Thread Doug Hogan
We have two new lists for LibreSSL:

libre...@openbsd.org - public list for technical discussion about
LibreSSL on any operating system.

libressl-secur...@openbsd.org - private list for reporting severe
vulnerabilities in OpenSSL or LibreSSL to the core LibreSSL team.


See http://www.openbsd.org/mail.html for more details.



LibreSSL errata

2015-06-11 Thread Doug Hogan
Patches are now available to fix a few issues in LibreSSL's libcrypto.

CVE-2015-1788 - Malformed ECParameters causes infinite loop
CVE-2015-1789 - Exploitable out-of-bounds read in X509_cmp_time
CVE-2015-1792 - CMS verify infinite loop with unknown hash function

Note that CMS was already disabled in LibreSSL.

Several other issues did not apply or were already fixed and one low
severity issue is under review.  For more information, see
https://www.openssl.org/news/secadv_20150611.txt

Thanks to the OpenSSL team for providing patches.

5.7 patch:
http://ftp.openbsd.org/pub/OpenBSD/patches/5.7/common/009_openssl.patch.sig
http://www.openbsd.org/errata57.html

5.6 patch:
http://ftp.openbsd.org/pub/OpenBSD/patches/5.6/common/026_openssl.patch.sig
http://www.openbsd.org/errata56.html



Re: Potential free uninitialized pointer in kern_ktrace.c

2015-08-01 Thread Doug Hogan
On Sat, Aug 01, 2015 at 07:31:58PM +0100, Mark Latimer wrote:
 reading through the compiler warnings I believe there is a potential issue
 in /usr/src/sys/kern/kern_ktrace.c At first glance it appears to free
 an uninitialized pointer memp.

I agree.


Index: sys/kern/kern_ktrace.c
===
RCS file: /cvs/src/sys/kern/kern_ktrace.c,v
retrieving revision 1.74
diff -u -p -r1.74 kern_ktrace.c
--- sys/kern/kern_ktrace.c  19 Jul 2015 04:45:25 -  1.74
+++ sys/kern/kern_ktrace.c  1 Aug 2015 18:55:44 -
@@ -348,7 +348,7 @@ ktruser(struct proc *p, const char *id, 
struct ktr_header kth;
struct ktr_user ktp;
int error;
-   void *memp;
+   void *memp = NULL;
 #defineSTK_PARAMS  128
long long stkbuf[STK_PARAMS / sizeof(long long)];
 



Re: pledge(2) hangman(6)

2015-10-21 Thread Doug Hogan
On Tue, Oct 20, 2015 at 09:04:51PM +0100, Ricardo Mestre wrote:
> Let's give some pledge(2) love to hangman(6)!
> 
> It seems to work fine for me with the patch mentioned below, nevertheless
> please be aware that I don't consider myself a developer, just a mere
> OpenBSD user with 'security uncle syndrome' :D
> 
> That being said please don't beat me to death for trying to turn this lovely
> OS a little bit more secure :)

I think you're close.  You should use "stdio" rather than "malloc"
though.  Out of the over 400 calls to pledge, there's only one call
that uses "malloc" directly and that's in regress. :)  "stdio" includes
"malloc" so that's why few include it directly.  My tree has some
uncommitted diffs so your count may differ.

Anyway, your diff looks good to me if you s/malloc/stdio/ in that line.

ok doug@ if anyone else wants to take a look.

> Index: src/games/hangman/main.c
> ===
> RCS file: /cvs/src/games/hangman/main.c,v
> retrieving revision 1.12
> diff -u -p -u -r1.12 main.c
> --- src/games/hangman/main.c7 Feb 2015 01:37:30 -   1.12
> +++ src/games/hangman/main.c20 Oct 2015 19:54:01 -
> @@ -43,6 +43,9 @@ main(int argc, char *argv[])
>  {
> int ch;
> 
> +   if (pledge("malloc tty rpath", NULL) == -1)
> +   err(1, "pledge");
> +
> while ((ch = getopt(argc, argv, "d:hk")) != -1) {
> switch (ch) {
> case 'd':
> 
> Best regards,
> Ricardo Mestre
> 



Re: pledge(2) and exec

2015-10-10 Thread Doug Hogan
On Sat, Oct 10, 2015 at 08:17:13AM +0200, Martijn van Duren wrote:
> I am however curious to this patch. By pledging ksh with exec it appears to
> me that once a pledged process is execve(2)d it looses it's already made
> pledges. (how else could applications spawned from the shell and still get
> their network interaction going?) This to me seems like something that might
> be undesirable (find remote code execution->insert exec of application->do
> some evil network activity)
> Is above observation correct or am I missing something?

It's too early for this conversation.  We're iteratively introducing
pledge requests and refining them as we go.  Right now, "exec" allows us
to make progress in other areas and it's no worse than the default.  We
still have a lot of diffs left to commit and write.  We'll come back to
this and other parts of pledge.



Not vulnerable to CVE-2015-1793

2015-07-09 Thread Doug Hogan
We have received several emails asking if we are impacted by the
latest CVE-2015-1793.  We are not impacted.  The code related to that
CVE was added after we forked 1.0.1g and we did not merge these changes
from upstream.  This CVE only concerns newer OpenSSL releases.



Re: fortune: remove OK_TO_WRITE_DISK ifdef + some teaks

2015-08-26 Thread Doug Hogan
On Wed, Aug 26, 2015 at 09:15:13AM +0200, Sebastien Marie wrote:
 The following patch remove #ifdef OK_TO_WRITE_DISK which isn't used by
 default.
 
 Additionnally, few disambiguisations suggested by cc(1) are added:
   - parentheses around  within ||
   - explicit braces to avoid ambiguous 'else'
 
 Comments ? OK ?

ok doug@

 Index: fortune.c
 ===
 RCS file: /cvs/src/games/fortune/fortune/fortune.c,v
 retrieving revision 1.42
 diff -u -p -r1.42 fortune.c
 --- fortune.c 6 Feb 2015 10:50:48 -   1.42
 +++ fortune.c 26 Aug 2015 07:10:31 -
 @@ -149,18 +149,14 @@ regex_t regex;
  int
  main(int ac, char *av[])
  {
 -#ifdef   OK_TO_WRITE_DISK
 - int fd;
 -#endif   /* OK_TO_WRITE_DISK */
 -
   getargs(ac, av);
  
   if (Match)
   exit(find_matches() != 0);
  
   init_prob();
 - if (Short_only  minlen_in_list(File_list)  SLEN ||
 - Long_only  maxlen_in_list(File_list) = SLEN)
 + if ((Short_only  minlen_in_list(File_list)  SLEN) ||
 + (Long_only  maxlen_in_list(File_list) = SLEN))
   exit(0);
  
   do {
 @@ -170,24 +166,6 @@ main(int ac, char *av[])
  
   display(Fortfile);
  
 -#ifdef   OK_TO_WRITE_DISK
 - if ((fd = creat(Fortfile-posfile, 0666))  0) {
 - perror(Fortfile-posfile);
 - exit(1);
 - }
 - /*
 -  * if we can, we exclusive lock, but since it isn't very
 -  * important, we just punt if we don't have easy locking
 -  * available.
 -  */
 - (void) flock(fd, LOCK_EX);
 - Fortfile-pos = htonl(Fortfile-pos);
 - write(fd, (char *) Fortfile-pos, sizeof Fortfile-pos);
 - Fortfile-pos = ntohl(Fortfile-pos);
 - if (!Fortfile-was_pos_file)
 - (void) chmod(Fortfile-path, 0666);
 - (void) flock(fd, LOCK_UN);
 -#endif   /* OK_TO_WRITE_DISK */
   if (Wait) {
   if (Fort_len == 0)
   (void) fortlen();
 @@ -212,7 +190,6 @@ rot13(char *p, size_t len)
  void
  display(FILEDESC *fp)
  {
 - char*p, ch;
   charline[BUFSIZ];
  
   open_fp(fp);
 @@ -337,13 +314,14 @@ form_file_list(char **files, int file_cn
   int i, percent;
   char*sp;
  
 - if (file_cnt == 0)
 + if (file_cnt == 0) {
   if (Find_files)
   return add_file(NO_PROB, FORTDIR, NULL, File_list,
   File_tail, NULL);
   else
   return add_file(NO_PROB, fortunes, FORTDIR,
   File_list, File_tail, NULL);
 + }
   for (i = 0; i  file_cnt; i++) {
   percent = NO_PROB;
   if (!isdigit(files[i][0]))
 @@ -504,9 +482,6 @@ over:
   fp-next = *head;
   *head = fp;
   }
 -#ifdef   OK_TO_WRITE_DISK
 - fp-was_pos_file = (access(fp-posfile, W_OK) = 0);
 -#endif   /* OK_TO_WRITE_DISK */
  
   return 1;
  }
 @@ -603,9 +578,6 @@ all_forts(FILEDESC *fp, char *offensive)
   obscene-datfile = datfile;
   obscene-posfile = posfile;
   obscene-read_tbl = 0;
 -#ifdef   OK_TO_WRITE_DISK
 - obscene-was_pos_file = (access(obscene-posfile, W_OK) = 0);
 -#endif   /* OK_TO_WRITE_DISK */
  }
  
  /*
 @@ -722,10 +694,6 @@ is_fortfile(char *file, char **datp, cha
   *datp = datfile;
   else
   free(datfile);
 -#ifdef   OK_TO_WRITE_DISK
 - if (posp != NULL)
 - *posp = copy(file, .pos);
 -#endif   /* OK_TO_WRITE_DISK */
   DPRINTF(2, (stderr, 1\n));
   return 1;
  }
 @@ -816,7 +784,7 @@ init_prob(void)
   exit(1);
   }
   percent = 100 - percent;
 - if (Equal_probs)
 + if (Equal_probs) {
   if (num_noprob != 0) {
   if (num_noprob  1) {
   frac = percent / num_noprob;
 @@ -830,7 +798,7 @@ init_prob(void)
   last-percent = percent;
   DPRINTF(1, (stderr, , residual = %d%%, percent));
   }
 - else {
 + } else {
   DPRINTF(1, (stderr,
   , %d%% distributed over remaining fortunes\n,
   percent));
 @@ -1003,25 +971,9 @@ open_dat(FILEDESC *fp)
  void
  get_pos(FILEDESC *fp)
  {
 -#ifdef   OK_TO_WRITE_DISK
 - int fd;
 -#endif /* OK_TO_WRITE_DISK */
 -
   assert(fp-read_tbl);
   if (fp-pos == POS_UNKNOWN) {
 -#ifdef   OK_TO_WRITE_DISK
 - if ((fd = open(fp-posfile, 0))  0 ||
 - read(fd, fp-pos, sizeof fp-pos) != sizeof fp-pos)
 - fp-pos = arc4random_uniform(fp-tbl.str_numstr);
 - else if (ntohl(fp-pos) = fp-tbl.str_numstr)
 - fp-pos %= fp-tbl.str_numstr;
 - else
 - fp-pos = ntohl(fp-pos);
 - if (fd = 0)
 - (void)