Re: top crash - pledge issue?

2015-10-28 Thread Ricardo Mestre

Hi,

Revision 1.81 from kern_pledge.c still doesn't solve the issue, miblen 
should be changed to 2 since machine.c from top(1) code is:


268 } else {
269 int cp_time_mib[] = {CTL_KERN, KERN_CPTIME};
270 long cp_time_tmp[CPUSTATES];
271
272 size = sizeof(cp_time_tmp);
273 if (sysctl(cp_time_mib, 2, cp_time_tmp, &size, NULL, 0) < 0)
274 warn("sysctl kern.cp_time failed");

By applying the patch below top(1) works again:

Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.81
diff -u -p -u -r1.81 kern_pledge.c
--- kern_pledge.c   28 Oct 2015 02:12:54 -  1.81
+++ kern_pledge.c   28 Oct 2015 07:44:20 -
@@ -908,7 +908,7 @@ pledge_sysctl_check(struct proc *p, int
if (miblen == 2 &&  /* kern.loadavg */
mib[0] == CTL_VM && mib[1] == VM_LOADAVG)
return (0);
-   if (miblen == 3 &&  /* kern.cptime */
+   if (miblen == 2 &&  /* kern.cptime */
mib[0] == CTL_KERN && mib[1] == KERN_CPTIME)
return (0);
if (miblen == 3 &&  /* kern.cptime2 */

Best regards,
Ricardo Mestre

On 27/10/2015 19:37, Ricardo Mestre wrote:

Hi people,

I can confirm this regress, just updated the kernel and top and had 
the same issue, but this diff seems to solve it, I just don't know if 
it's the right place to put it or not:


Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.80
diff -u -p -u -r1.80 kern_pledge.c
--- kern_pledge.c   26 Oct 2015 17:52:19 -  1.80
+++ kern_pledge.c   27 Oct 2015 19:32:09 -
@@ -911,6 +911,9 @@ pledge_sysctl_check(struct proc *p, int
if (miblen == 3 &&  /* 
kern.cptime2 */

mib[0] == CTL_KERN && mib[1] == KERN_CPTIME2)
return (0);
+   if (miblen == 2 &&  /* kern.cp_time */
+   mib[0] == CTL_KERN && mib[1] == KERN_CPTIME)
+   return (0);
}

if ((p->p_p->ps_pledge & PLEDGE_PS)) {

Best regards,
Ricardo Mestre

On 27/10/2015 19:00, Mike wrote:

OpenBSD 5.8-current (GENERIC) #1: Tue Oct 27 12:31:10 EDT 2015
m...@otest.24cl.home:/usr/src/sys/arch/amd64/compile/GENERIC


I didn't see anything in current.html that may affect this.

I downloaded the Oct 20 snapshot.  Then I updated the source to current
this morning.  After the build, top crashes immediately upon invocation.

# top
Abort (core dumped)


In messages.log, I see two lines:

Oct 27 14:52:22 otest /bsd: top(12603): sysctl 2: 1 40 -2129088583 -1
981777920 -255

Oct 27 14:52:22 otest /bsd: top(12603): syscall 202 "stdio"


which looks like it may be pledge output.

If I need to do something else to track this let me know.  I can make
the core dump available to download, if needed.

thx.






Re: pidfile()

2015-10-28 Thread Jason McIntyre
On Tue, Oct 27, 2015 at 02:28:51PM +0100, Jan Stary wrote:
> > usr.sbin/rbootd: rbootd.c 
> > usr.sbin/rarpd : rarpd.c 
> > usr.sbin/mrouted: main.c 
> > usr.bin/usbhidaction: usbhidaction.c 
> > usr.sbin/wsmoused: wsmoused.8 wsmoused.c wsmoused.h 
> > usr.sbin/ypserv/ypserv: Makefile ypserv.c 
> > usr.sbin/pppd  : main.c 
> 
> Below are the missing manpage bits.
> 

thanks. i've just fixed these.
jmc

> While there, should the commented
>   .\" .It Pa /tftpboot
> be removed from rarpd.8 ?
> 
> Also,
> $ find /var/run/ -name \*pid 
> /var/run/syslog.pid
> /var/run/unbound.pid
> /var/run/lpd.pid
> /var/run/sshd.pid
> /var/run/smtpd.pid
> /var/run/cron.pid
> 
> - should these stop using pidfiles too?
> 
>   Jan
> 
> 
> Index: usr.sbin/mrouted/mrouted.8
> ===
> RCS file: /cvs/src/usr.sbin/mrouted/mrouted.8,v
> retrieving revision 1.25
> diff -u -p -u -p -r1.25 mrouted.8
> --- usr.sbin/mrouted/mrouted.88 Sep 2014 01:27:55 -   1.25
> +++ usr.sbin/mrouted/mrouted.827 Oct 2015 13:23:34 -
> @@ -344,18 +344,11 @@ Dumps the internal routing tables to std
>  .Nm
>  was invoked with a non-zero debug level).
>  .El
> -.Pp
> -For convenience in sending signals,
> -.Nm
> -writes its process ID to
> -.Pa /var/run/mrouted.pid
> -upon startup.
>  .Sh FILES
>  .Bl -tag -width /var/tmp/mrouted.cache -compact
>  .It Pa /etc/mrouted.conf
>  .It Pa /var/tmp/mrouted.cache
>  .It Pa /var/tmp/mrouted.dump
> -.It Pa /var/run/mrouted.pid
>  .El
>  .Sh EXAMPLES
>  The routing tables look like this:
> Index: usr.sbin/pppd/pppd.8
> ===
> RCS file: /cvs/src/usr.sbin/pppd/pppd.8,v
> retrieving revision 1.45
> diff -u -p -u -p -r1.45 pppd.8
> --- usr.sbin/pppd/pppd.8  14 Sep 2015 20:06:59 -  1.45
> +++ usr.sbin/pppd/pppd.8  27 Oct 2015 13:23:34 -
> @@ -1241,14 +1241,6 @@ script.
>  .El
>  .Sh FILES
>  .Bl -tag -width Ds
> -.It Pa /var/run/ppp Ns Ar n Ns .pid
> -.\" (BSD or Linux),
> -.\" /etc/ppp/ppp Ns Ar n Ns .pid
> -.\" (others)
> -Process-ID for
> -.Nm
> -process on PPP interface unit
> -.Ar n .
>  .It Pa /etc/ppp/pap-secrets
>  Usernames, passwords and IP addresses for PAP authentication.
>  This file should be owned by root and not readable or writable by any other
> Index: usr.sbin/rarpd/rarpd.8
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.8,v
> retrieving revision 1.20
> diff -u -p -u -p -r1.20 rarpd.8
> --- usr.sbin/rarpd/rarpd.88 Sep 2014 01:27:55 -   1.20
> +++ usr.sbin/rarpd/rarpd.827 Oct 2015 13:23:34 -
> @@ -82,14 +82,11 @@ is the target IP address expressed in up
>  (only the first 8 characters of filenames are checked).
>  .El
>  .Sh FILES
> -.Bl -tag -width "/var/run/rarpd.pidXXX" -compact
> +.Bl -tag -width /etc/ethers -compact
>  .It Pa /etc/ethers
>  Ethernet host name database.
>  .It Pa /etc/hosts
>  Host name database.
> -.It Pa /var/run/rarpd.pid
> -Process ID of
> -.Nm .
>  .\" .It Pa /tftpboot
>  .El
>  .Sh SEE ALSO
> Index: usr.sbin/rbootd/rbootd.8
> ===
> RCS file: /cvs/src/usr.sbin/rbootd/rbootd.8,v
> retrieving revision 1.14
> diff -u -p -u -p -r1.14 rbootd.8
> --- usr.sbin/rbootd/rbootd.8  15 Nov 2014 14:41:03 -  1.14
> +++ usr.sbin/rbootd/rbootd.8  27 Oct 2015 13:23:34 -
> @@ -143,8 +143,6 @@ configuration file
>  debug output
>  .It Pa /usr/mdec/rbootd
>  directory containing boot files
> -.It Pa /var/run/rbootd.pid
> -process ID
>  .El
>  .Sh SEE ALSO
>  .Xr kill 1 ,
> Index: usr.sbin/ypserv/ypserv/ypserv.8
> ===
> RCS file: /cvs/src/usr.sbin/ypserv/ypserv/ypserv.8,v
> retrieving revision 1.27
> diff -u -p -u -p -r1.27 ypserv.8
> --- usr.sbin/ypserv/ypserv/ypserv.8   16 Jul 2013 11:13:34 -  1.27
> +++ usr.sbin/ypserv/ypserv/ypserv.8   27 Oct 2015 13:23:34 -
> @@ -98,9 +98,6 @@ map files and the
>  or
>  .Xr securenet 5
>  configuration file.
> -The process ID
> -can be found in the file
> -.Pa /var/run/ypserv.pid .
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> @@ -136,7 +133,6 @@ or
>  .Bl -tag -width /var/yp/ypserv.log -compact
>  .It Pa /var/yp/ypserv.log
>  .It Pa /var/yp/securenet
> -.It Pa /var/run/ypserv.pid
>  .El
>  .Sh SEE ALSO
>  .Xr securenet 5 ,
> 



Re: ld.so crash second attempt

2015-10-28 Thread Alf Schlichting
On Sun, 25 Oct 2015 11:01:27 +0100,
Peter Hajdu wrote:
> 
> [1  ]
> Hi,
> 
> I try to give it one more attempt with a bit more description about the
> bug.
> 
> After calling dlclose in _dl_notify_unload_shlib_ group reference counts
> are decreased by following the object's grpref-list.  Unfortunately the
> references are removed from the list during the graph traversal.
> 
> dlclose will run all destructors of the unused objects and tries to
> unload all objects reachable from the closed object's child and
> grpref-list.  Since the grpref-list references were removed, the unused
> destructed object stays in memory as garbage.  Next time when this
> object is loaded it is found in the memory and crashes during load.
> 
> This patch unloads all unused objects instead of following the closed
> object's child and grpref list.
> 
> Best regards,
> Peter Hajdu
> 

Hello,

I can confirm that this works with sdl2 on a maybe 4 week old snapshot
on amd64.
I used that old patch that was flying around to get sdl2 up and
running, now I replaced ld.so with a fresh checkout, applied the
patch and sdl2 still works fine, no other problems observed.

Thanks a lot,

Alf

> [2 ldso_fix.diff ]
> Index: dlfcn.c
> ===
> RCS file: /cvs/src/libexec/ld.so/dlfcn.c,v
> retrieving revision 1.91
> diff -u -p -r1.91 dlfcn.c
> --- dlfcn.c   19 Sep 2015 20:56:47 -  1.91
> +++ dlfcn.c   21 Oct 2015 13:52:46 -
> @@ -302,7 +302,7 @@ _dl_real_close(void *handle)
>   object->opencount--;
>   _dl_notify_unload_shlib(object);
>   _dl_run_all_dtors();
> - _dl_unload_shlib(object);
> + _dl_unload_unused();
>   _dl_cleanup_objects();
>   return (0);
>  }
> Index: library.c
> ===
> RCS file: /cvs/src/libexec/ld.so/library.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 library.c
> --- library.c 16 Jan 2015 16:18:07 -  1.71
> +++ library.c 21 Oct 2015 13:52:46 -
> @@ -74,6 +74,22 @@ _dl_unload_shlib(elf_object_t *object)
>   }
>  }
>  
> +void
> +_dl_unload_unused(void)
> +{
> + elf_object_t *obj, *next;
> +
> + for (obj = _dl_objects->next; obj != NULL; obj = next) {
> + next = obj->next;
> + if (OBJECT_REF_CNT(obj) != 0 || obj->status & STAT_UNLOADED)
> + continue;
> + obj->status |= STAT_UNLOADED;
> + _dl_load_list_free(obj->load_list);
> + _dl_munmap((void *)obj->load_base, obj->load_size);
> + _dl_remove_object(obj);
> + }
> +}
> +
>  elf_object_t *
>  _dl_tryload_shlib(const char *libname, int type, int flags)
>  {
> Index: library_mquery.c
> ===
> RCS file: /cvs/src/libexec/ld.so/library_mquery.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 library_mquery.c
> --- library_mquery.c  22 Jan 2015 05:48:17 -  1.49
> +++ library_mquery.c  21 Oct 2015 13:52:46 -
> @@ -79,6 +79,20 @@ _dl_unload_shlib(elf_object_t *object)
>   }
>  }
>  
> +void
> +_dl_unload_unused(void)
> +{
> + elf_object_t *obj, *next;
> +
> + for (obj = _dl_objects->next; obj != NULL; obj = next) {
> + next = obj->next;
> + if (OBJECT_REF_CNT(obj) != 0 || obj->status & STAT_UNLOADED)
> + continue;
> + obj->status |= STAT_UNLOADED;
> + _dl_load_list_free(obj->load_list);
> + _dl_remove_object(obj);
> + }
> +}
>  
>  elf_object_t *
>  _dl_tryload_shlib(const char *libname, int type, int flags)
> Index: resolve.h
> ===
> RCS file: /cvs/src/libexec/ld.so/resolve.h,v
> retrieving revision 1.73
> diff -u -p -r1.73 resolve.h
> --- resolve.h 19 Sep 2015 20:56:47 -  1.73
> +++ resolve.h 21 Oct 2015 13:52:46 -
> @@ -223,6 +223,7 @@ void _dl_unlink_dlopen(elf_object_t *dep
>  void _dl_notify_unload_shlib(elf_object_t *object);
>  void _dl_unload_shlib(elf_object_t *object);
>  void _dl_unload_dlopen(void);
> +void _dl_unload_unused(void);
>  
>  void _dl_run_all_dtors(void);
>  



Re: WAPBL implementation

2015-10-28 Thread Walter Neto
Adding WAPBL support for dumpfs(8)

next diffs:
- tunefs(8) showing log information and setting log size
- fsck_ffs(8) WAPBL support

ok jasper@


Index: sbin/dumpfs/dumpfs.c
===
RCS file: /Volumes/CSP/cvs/src/sbin/dumpfs/dumpfs.c,v
retrieving revision 1.32
diff -u -r1.32 dumpfs.c
--- sbin/dumpfs/dumpfs.c20 Jan 2015 18:22:21 -  1.32
+++ sbin/dumpfs/dumpfs.c28 Oct 2015 10:40:26 -
@@ -40,14 +40,18 @@
 
 #include  /* DEV_BSIZE MAXBSIZE isset */
 #include 
+#include 
+#include 
 
 #include 
+#include 
 #include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -68,12 +72,26 @@
 } cgun;
 #define acgcgun.cg
 
-intdumpfs(int, const char *);
-intdumpcg(const char *, int, int);
-intmarshal(const char *);
-intopen_disk(const char *);
-void   pbits(void *, int);
-__dead voidusage(void);
+union {
+   struct wapbl_wc_header wh;
+   struct wapbl_wc_null wn;
+   char pad[MAXBSIZE];
+} jbuf;
+#define awhjbuf.wh
+#define awnjbuf.wn
+
+int dojournal = 0;
+
+int dumpfs(int, const char *);
+int dumpcg(const char *, int, int);
+int marshal(const char *);
+int open_disk(const char *);
+voidpbits(void *, int);
+int print_journal(const char *, int);
+const char *wapbl_type_string(unsigned);
+voidprint_journal_header(const char *);
+off_t   print_journal_entries(const char *, size_t);
+__dead void usage(void);
 
 int
 main(int argc, char *argv[])
@@ -84,8 +102,11 @@
 
domarshal = eval = 0;
 
-   while ((ch = getopt(argc, argv, "m")) != -1) {
+   while ((ch = getopt(argc, argv, "jm")) != -1) {
switch (ch) {
+   case 'j':   /* WAPBL journal */
+   dojournal = 1;
+   break;
case 'm':
domarshal = 1;
break;
@@ -258,6 +279,15 @@
afs.fs_cgrotor, afs.fs_fmod, afs.fs_ronly, afs.fs_clean);
printf("avgfpdir %d\tavgfilesize %d\n",
afs.fs_avgfpdir, afs.fs_avgfilesize);
+   if (dojournal) {
+   printf("wapbl version 0x%x\tlocation %u\tflags 0x%x\n",
+   afs.fs_journal_version, afs.fs_journal_location,
+   afs.fs_journal_flags);
+   printf("wapbl loc0 %llu\tloc1 %llu",
+   afs.fs_journallocs[0], afs.fs_journallocs[1]);
+   printf("\tloc2 %llu\tloc3 %llu\n",
+   afs.fs_journallocs[2], afs.fs_journallocs[3]);
+   }
printf("flags\t");
if (afs.fs_magic == FS_UFS2_MAGIC ||
afs.fs_ffs1_flags & FS_FLAGS_UPDATED)
@@ -270,6 +300,8 @@
printf("unclean ");
if (fsflags & FS_DOSOFTDEP)
printf("soft-updates ");
+   if (fsflags & FS_DOWAPBL)
+   printf("wapbl ");
if (fsflags & FS_FLAGS_UPDATED)
printf("updated ");
 #if 0
@@ -281,6 +313,10 @@
printf("fsmnt\t%s\n", afs.fs_fsmnt);
printf("volname\t%s\tswuid\t%ju\n",
afs.fs_volname, (uintmax_t)afs.fs_swuid);
+   if (dojournal) {
+   printf("\n");
+   print_journal(name, fd);
+   }
printf("\ncs[].cs_(nbfree,ndir,nifree,nffree):\n\t");
afs.fs_csp = calloc(1, afs.fs_cssize);
for (i = 0, j = 0; i < afs.fs_cssize; i += afs.fs_bsize, j++) {
@@ -457,9 +493,182 @@
printf("\n");
 }
 
+int
+print_journal(const char *name, int fd)
+{
+   daddr_t off;
+   size_t count, blklen, bno, skip;
+   off_t boff, head, tail, len;
+   uint32_t generation;
+
+   if (afs.fs_journal_version != UFS_WAPBL_VERSION)
+   return 0;
+
+   generation = 0;
+   head = tail = 0;
+
+   switch (afs.fs_journal_location) {
+   case UFS_WAPBL_JOURNALLOC_END_PARTITION:
+   case UFS_WAPBL_JOURNALLOC_IN_FILESYSTEM:
+
+   off= afs.fs_journallocs[0];
+   count  = afs.fs_journallocs[1];
+   blklen = afs.fs_journallocs[2];
+
+   for (bno=0; bno= 2 * blklen &&
+ ((head >= tail && (boff < tail || boff >= head)) ||
+ (head < tail && (boff >= head && boff < tail
+   continue;
+
+   printf("journal block %lu offset %lld\n",
+   (unsigned long)bno, (long long) boff);
+
+   if (lseek(fd, (off_t)(off*blklen) + boff, SEEK_SET)
+   == (off_t)-1)
+   return (1);
+   if (read(fd, &jbuf, blklen) != (ssize_t)blklen) {
+   warnx("%s: error reading journal", name);
+   return 1;
+   }
+
+   switch (awh.wc_

Re: WAPBL implementation

2015-10-28 Thread Jasper Lievisse Adriaanse
On Wed, Oct 28, 2015 at 09:06:54AM -0200, Walter Neto wrote:
> Adding WAPBL support for dumpfs(8)
> 
> next diffs:
> - tunefs(8) showing log information and setting log size
> - fsck_ffs(8) WAPBL support
> 
> ok jasper@
Correction:
I only pointed out that we should have the option to somehow get information
about the log. I did not OK this or the original WAPBL diff.

-- 
jasper



Re: WAPBL implementation

2015-10-28 Thread Walter Neto
On Wed, Oct 28, 2015 at 12:50:24PM +0100, Jasper Lievisse Adriaanse wrote:
> On Wed, Oct 28, 2015 at 09:06:54AM -0200, Walter Neto wrote:
> > Adding WAPBL support for dumpfs(8)
> > 
> > next diffs:
> > - tunefs(8) showing log information and setting log size
> > - fsck_ffs(8) WAPBL support
> > 
> > ok jasper@
> Correction:
> I only pointed out that we should have the option to somehow get information
> about the log. I did not OK this or the original WAPBL diff.
> 

Sorry guys for the misunderstanding, I learned the language now.

Forgive my inexperience

> -- 
> jasper



Re: (patch) tcpdump cleanup stats format

2015-10-28 Thread Jérémie Courrèges-Anglas
Kevin Reay  writes:

> Correct printf format for received/dropped packet counts in cleanup().

Committed, thanks.

> ps_recv and ps_drop (struct pcap_stat) are both type u_int.
> Index: tcpdump.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 tcpdump.c
> --- tcpdump.c 14 Oct 2015 04:55:17 -  1.75
> +++ tcpdump.c 28 Oct 2015 02:43:30 -
> @@ -522,10 +522,10 @@ cleanup(int signo)
>   write(STDERR_FILENO, buf, strlen(buf));
>   } else {
>   (void)snprintf(buf, sizeof buf,
> - "%d packets received by filter\n", stat.ps_recv);
> + "%u packets received by filter\n", stat.ps_recv);
>   write(STDERR_FILENO, buf, strlen(buf));
>   (void)snprintf(buf, sizeof buf,
> - "%d packets dropped by kernel\n", stat.ps_drop);
> + "%u packets dropped by kernel\n", stat.ps_drop);
>   write(STDERR_FILENO, buf, strlen(buf));
>   }
>   }


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



boot from softraid, backspace in passphrase prompt

2015-10-28 Thread Uwe Stuehler
I want correct typing mistakes when booting from softraid crypto disks.
Can we handle at least the backspace key, plz^Hease? :)

diff --git sys/arch/amd64/stand/libsa/softraid.c 
sys/arch/amd64/stand/libsa/softraid.c
index 336865a..801cec4 100644
--- sys/arch/amd64/stand/libsa/softraid.c
+++ sys/arch/amd64/stand/libsa/softraid.c
@@ -557,6 +557,10 @@ sr_crypto_decrypt_keys(struct sr_boot_volume *bv)
c = cngetc();
if (c == '\r' || c == '\n')
break;
+   else if (c == '\b') {
+   i = i > 0 ? i - 2 : -1;
+   continue;
+   }
passphrase[i] = (c & 0xff);
}
passphrase[i] = 0;
diff --git sys/arch/i386/stand/libsa/softraid.c 
sys/arch/i386/stand/libsa/softraid.c
index 336865a..801cec4 100644
--- sys/arch/i386/stand/libsa/softraid.c
+++ sys/arch/i386/stand/libsa/softraid.c
@@ -557,6 +557,10 @@ sr_crypto_decrypt_keys(struct sr_boot_volume *bv)
c = cngetc();
if (c == '\r' || c == '\n')
break;
+   else if (c == '\b') {
+   i = i > 0 ? i - 2 : -1;
+   continue;
+   }
passphrase[i] = (c & 0xff);
}
passphrase[i] = 0;
diff --git sys/arch/sparc64/stand/ofwboot/softraid.c 
sys/arch/sparc64/stand/ofwboot/softraid.c
index 1dadfdb..7654194 100644
--- sys/arch/sparc64/stand/ofwboot/softraid.c
+++ sys/arch/sparc64/stand/ofwboot/softraid.c
@@ -572,6 +572,10 @@ sr_crypto_decrypt_keys(struct sr_boot_volume *bv)
c = getchar();
if (c == '\r' || c == '\n')
break;
+   else if (c == '\b') {
+   i = i > 0 ? i - 2 : -1;
+   continue;
+   }
passphrase[i] = (c & 0xff);
}
passphrase[i] = 0;



Re: boot from softraid, backspace in passphrase prompt

2015-10-28 Thread Theo de Raadt
Since people are typing blind, can you add support for ^U as well?

> I want correct typing mistakes when booting from softraid crypto disks.
> Can we handle at least the backspace key, plz^Hease? :)
> 
> diff --git sys/arch/amd64/stand/libsa/softraid.c 
> sys/arch/amd64/stand/libsa/softraid.c
> index 336865a..801cec4 100644
> --- sys/arch/amd64/stand/libsa/softraid.c
> +++ sys/arch/amd64/stand/libsa/softraid.c
> @@ -557,6 +557,10 @@ sr_crypto_decrypt_keys(struct sr_boot_volume *bv)
>   c = cngetc();
>   if (c == '\r' || c == '\n')
>   break;
> + else if (c == '\b') {
> + i = i > 0 ? i - 2 : -1;
> + continue;
> + }
>   passphrase[i] = (c & 0xff);
>   }
>   passphrase[i] = 0;
> diff --git sys/arch/i386/stand/libsa/softraid.c 
> sys/arch/i386/stand/libsa/softraid.c
> index 336865a..801cec4 100644
> --- sys/arch/i386/stand/libsa/softraid.c
> +++ sys/arch/i386/stand/libsa/softraid.c
> @@ -557,6 +557,10 @@ sr_crypto_decrypt_keys(struct sr_boot_volume *bv)
>   c = cngetc();
>   if (c == '\r' || c == '\n')
>   break;
> + else if (c == '\b') {
> + i = i > 0 ? i - 2 : -1;
> + continue;
> + }
>   passphrase[i] = (c & 0xff);
>   }
>   passphrase[i] = 0;
> diff --git sys/arch/sparc64/stand/ofwboot/softraid.c 
> sys/arch/sparc64/stand/ofwboot/softraid.c
> index 1dadfdb..7654194 100644
> --- sys/arch/sparc64/stand/ofwboot/softraid.c
> +++ sys/arch/sparc64/stand/ofwboot/softraid.c
> @@ -572,6 +572,10 @@ sr_crypto_decrypt_keys(struct sr_boot_volume *bv)
>   c = getchar();
>   if (c == '\r' || c == '\n')
>   break;
> + else if (c == '\b') {
> + i = i > 0 ? i - 2 : -1;
> + continue;
> + }
>   passphrase[i] = (c & 0xff);
>   }
>   passphrase[i] = 0;
> 



[PATCH] Add a simple roundrobin load balancing feature to rebound(8)

2015-10-28 Thread Dimitris Papastamos
Hi,

I thought it would be cool for rebound(8) to load balance on a number of
DNS servers.

While I was working on this, I did not manage to convince myself as to
whether this should be the default behaviour.

An alternative default would be to use the master server provided.  If
requests started timing out, it would switch to the secondary.  If the
master became available again, then it would switch back to it.

This diff may be useful regardless of what the default should be and
given that there is no config parser at the moment that can handle
options I thought I would send this out to get some feedback.

Cheers,
Dimitris

>From 86c7ad057137181b560341779c79bca339b30e95 Mon Sep 17 00:00:00 2001
From: Dimitris Papastamos 
Date: Wed, 28 Oct 2015 14:45:03 +
Subject: [PATCH] Add a simple roundrobin load balancing feature to rebound(8)

---
 usr.sbin/rebound/rebound.8 |   6 ++-
 usr.sbin/rebound/rebound.c | 105 +++--
 2 files changed, 77 insertions(+), 34 deletions(-)

diff --git a/usr.sbin/rebound/rebound.8 b/usr.sbin/rebound/rebound.8
index e25fc83..d32a5a9 100644
--- a/usr.sbin/rebound/rebound.8
+++ b/usr.sbin/rebound/rebound.8
@@ -28,8 +28,10 @@ The
 daemon proxies DNS requests.
 It listens on localhost and forwards queries to another server.
 .Pp
-At present, the config file consists of a single line containing the next
-hop DNS server.
+At present, the config file consists of lines containing the next hop
+DNS servers.  If more than one server is specified,
+.Nm
+will use them in a roundrobin fashion.
 .Sh SEE ALSO
 .Xr resolv.conf 5 ,
 .Xr unbound 8
diff --git a/usr.sbin/rebound/rebound.c b/usr.sbin/rebound/rebound.c
index 80405ab..7c9c7bd 100644
--- a/usr.sbin/rebound/rebound.c
+++ b/usr.sbin/rebound/rebound.c
@@ -37,6 +37,8 @@
 #include 
 #include 
 
+#define SA(x) ((struct sockaddr *)(x))
+
 uint16_t randomid(void);
 
 static struct timespec now;
@@ -93,6 +95,12 @@ static TAILQ_HEAD(, request) reqfifo;
 static RB_HEAD(reqtree, request) reqtree;
 RB_PROTOTYPE_STATIC(reqtree, request, reqnode, reqcmp)
 
+struct server {
+   struct sockaddr_storage addr;
+   int active;
+   TAILQ_ENTRY(server) entry;
+};
+static TAILQ_HEAD(, server) servers;
 
 static void
 logmsg(int prio, const char *msg, ...)
@@ -125,6 +133,27 @@ logerr(const char *msg, ...)
exit(1);
 }
 
+static void
+nextserver(struct sockaddr_storage *remoteaddr)
+{
+   struct server *s;
+
+   TAILQ_FOREACH(s, &servers, entry)
+   if (s->active)
+   break;
+   if (!s) {
+   s = TAILQ_FIRST(&servers);
+   } else {
+   s->active = 0;
+   if (TAILQ_NEXT(s, entry))
+   s = TAILQ_NEXT(s, entry);
+   else
+   s = TAILQ_FIRST(&servers);
+   }
+   s->active = 1;
+   memcpy(remoteaddr, &s->addr, sizeof(*remoteaddr));
+}
+
 static struct dnscache *
 cachelookup(struct dnspacket *dnsreq, size_t reqlen)
 {
@@ -177,8 +206,9 @@ reqcmp(struct request *r1, struct request *r2)
 RB_GENERATE_STATIC(reqtree, request, reqnode, reqcmp)
 
 static struct request *
-newrequest(int ud, struct sockaddr *remoteaddr)
+newrequest(int ud)
 {
+   struct sockaddr_storage remoteaddr;
struct sockaddr from;
socklen_t fromlen;
struct request *req;
@@ -231,10 +261,11 @@ newrequest(int ud, struct sockaddr *remoteaddr)
}
req->cacheent = hit;
 
-   req->s = socket(remoteaddr->sa_family, SOCK_DGRAM, 0);
+   nextserver(&remoteaddr);
+   req->s = socket(SA(&remoteaddr)->sa_family, SOCK_DGRAM, 0);
if (req->s == -1)
goto fail;
-   if (connect(req->s, remoteaddr, remoteaddr->sa_len) == -1) {
+   if (connect(req->s, SA(&remoteaddr), SA(&remoteaddr)->sa_len) == -1) {
logmsg(LOG_NOTICE, "failed to connect");
goto fail;
}
@@ -303,8 +334,9 @@ fail:
 }
 
 static struct request *
-newtcprequest(int ld, struct sockaddr *remoteaddr)
+newtcprequest(int ld)
 {
+   struct sockaddr_storage remoteaddr;
struct request *req;
 
if (!(req = calloc(1, sizeof(*req
@@ -321,11 +353,12 @@ newtcprequest(int ld, struct sockaddr *remoteaddr)
if (req->client == -1)
goto fail;
 
-   req->s = socket(remoteaddr->sa_family, SOCK_STREAM | SOCK_NONBLOCK, 0);
+   nextserver(&remoteaddr);
+   req->s = socket(SA(&remoteaddr)->sa_family, SOCK_STREAM | 
SOCK_NONBLOCK, 0);
if (req->s == -1)
goto fail;
req->phase = 1;
-   if (connect(req->s, remoteaddr, remoteaddr->sa_len) == -1) {
+   if (connect(req->s, SA(&remoteaddr), SA(&remoteaddr)->sa_len) == -1) {
if (errno != EINPROGRESS)
goto fail;
} else {
@@ -340,36 +373,45 @@ fail:
 }
 
 static int
-readconfig(FILE *conf, struct sockaddr_storage *remoteaddr)
+readconfig(FILE *conf)
 {
char buf[1024]

Re: [PATCH] Use rbtree for looking up the client in rebound(8)

2015-10-28 Thread Dimitris Papastamos
On Tue, Oct 27, 2015 at 05:20:31PM -0400, Ted Unangst wrote:
> Dimitris Papastamos wrote:
> > There was a comment in the code that indicated that it might be worth
> > investigating the use of trees.  I have not currently done any kind of
> > serious benchmarking on this but I am looking into it.
> 
> nice.
> 
> > +static int
> > +reqcmp(struct request *r1, struct request *r2)
> > +{
> > +   return r1->s - r2->s;
> > +}
> 
> Don't write comparison functions like this. Overflow makes bad things happen.
> The example in the man page is a good way to do it I think.

Ah I see, I've updated the patch.

Index: rebound.c
===
RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
retrieving revision 1.28
diff -u -p -r1.28 rebound.c
--- rebound.c   26 Oct 2015 12:24:48 -  1.28
+++ rebound.c   28 Oct 2015 08:09:01 -
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -83,11 +84,14 @@ struct request {
socklen_t fromlen;
struct timespec ts;
TAILQ_ENTRY(request) fifo;
+   RB_ENTRY(request) reqnode;
uint16_t clientid;
uint16_t reqid;
struct dnscache *cacheent;
 };
 static TAILQ_HEAD(, request) reqfifo;
+static RB_HEAD(reqtree, request) reqtree;
+RB_PROTOTYPE_STATIC(reqtree, request, reqnode, reqcmp)
 
 
 static void
@@ -144,6 +148,7 @@ freerequest(struct request *req)
struct dnscache *ent;
 
TAILQ_REMOVE(&reqfifo, req, fifo);
+   RB_REMOVE(reqtree, &reqtree, req);
if (req->client != -1)
close(req->client);
if (req->s != -1)
@@ -164,6 +169,13 @@ freecacheent(struct dnscache *ent)
free(ent);
 }
 
+static int
+reqcmp(struct request *r1, struct request *r2)
+{
+   return (r1->s < r2->s ? -1 : r1->s > r2->s);
+}
+RB_GENERATE_STATIC(reqtree, request, reqnode, reqcmp)
+
 static struct request *
 newrequest(int ud, struct sockaddr *remoteaddr)
 {
@@ -192,6 +204,7 @@ newrequest(int ud, struct sockaddr *remo
return NULL;
 
TAILQ_INSERT_TAIL(&reqfifo, req, fifo);
+   RB_INSERT(reqtree, &reqtree, req);
req->ts = now;
req->ts.tv_sec += 30;
 
@@ -298,6 +311,7 @@ newtcprequest(int ld, struct sockaddr *r
return NULL;
 
TAILQ_INSERT_TAIL(&reqfifo, req, fifo);
+   RB_INSERT(reqtree, &reqtree, req);
req->ts = now;
req->ts.tv_sec += 30;
 
@@ -358,7 +372,7 @@ launch(const char *confname, int ud, int
struct sockaddr_storage remoteaddr;
struct kevent chlist[2], kev[4];
struct timespec ts, *timeout = NULL;
-   struct request *req;
+   struct request reqkey, *req;
struct dnscache *ent;
struct passwd *pwd;
FILE *conf;
@@ -438,12 +452,8 @@ launch(const char *confname, int ud, int
kevent(kq, chlist, 1, NULL, 0, NULL);
}
} else if (kev[i].filter == EVFILT_WRITE) {
-   req = TAILQ_FIRST(&reqfifo);
-   while (req) {
-   if (req->s == kev[i].ident)
-   break;
-   req = TAILQ_NEXT(req, fifo);
-   }
+   reqkey.s = kev[i].ident;
+   req = RB_FIND(reqtree, &reqtree, &reqkey);
if (!req)
logerr("lost request");
req = tcpphasetwo(req);
@@ -455,13 +465,8 @@ launch(const char *confname, int ud, int
kevent(kq, chlist, 2, NULL, 0, NULL);
}
} else if (kev[i].filter == EVFILT_READ) {
-   /* use a tree here? */
-   req = TAILQ_FIRST(&reqfifo);
-   while (req) {
-   if (req->s == kev[i].ident)
-   break;
-   req = TAILQ_NEXT(req, fifo);
-   }
+   reqkey.s = kev[i].ident;
+   req = RB_FIND(reqtree, &reqtree, &reqkey);
if (!req)
logerr("lost request");
if (req->client == -1)
@@ -547,6 +552,7 @@ main(int argc, char **argv)
if (!debug)
daemon(0, 0);
 
+   RB_INIT(&reqtree);
TAILQ_INIT(&reqfifo);
TAILQ_INIT(&cache);
 



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

2015-10-28 Thread Michael McConville
Relayd, httpd, and ntpd define the functions get_data() and
get_string(). Both call calloc and then immediately memcpy. Calloc's
zeroing isn't optimized out. These functions are called in network data
paths in at least a couple places, so all this extra writing could have
a meaningful performance impact.

ok?


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



pledge(2) and spamd(8)

2015-10-28 Thread Ricardo Mestre

Hi

I was just trying to pledge(2) spamd(8), nevertheless came across 2 
priviliges kern_pledge.c is missing for this to work.


First spamd(8) needs to read sysctl kern.maxfiles in order to see if it 
can launch with that value or not, and second if the multicast options 
are passed as parameters then it also needs IP_MULTICAST_TTL since 
spamd(8) calls setsockopt(2) with that option set:


Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.89
diff -u -p -u -r1.89 kern_pledge.c
--- kern_pledge.c   28 Oct 2015 15:33:44 -  1.89
+++ kern_pledge.c   28 Oct 2015 16:13:31 -
@@ -889,6 +889,9 @@ pledge_sysctl_check(struct proc *p, int
if (miblen == 3 &&  /* kern.cptime2 */
mib[0] == CTL_KERN && mib[1] == KERN_CPTIME2)
return (0);
+   if (miblen == 2 &&  /* kern.maxfiles */
+   mib[0] == CTL_KERN && mib[1] == KERN_MAXFILES)
+   return (0);
}

if ((p->p_p->ps_pledge & PLEDGE_PS)) {
@@ -1210,6 +1213,7 @@ pledge_sockopt_check(struct proc *p, int
case IP_RECVDSTPORT:
return (0);
case IP_MULTICAST_IF:
+   case IP_MULTICAST_TTL:
case IP_ADD_MEMBERSHIP:
case IP_DROP_MEMBERSHIP:
if (p->p_p->ps_pledge & PLEDGE_MCAST)



With this patch then spamd(8) works with the patch below (I used a lot 
of options that I use on my servers like greylisting options, multicast, 
certificate, stuttering etc). Bear in mind that this is just an initial 
patch and the priviliges can be dropped further down the code and also 
that I'm just beginning and sharing this more as a question if I can go 
down this road than as a request:


Index: spamd.c
===
RCS file: /cvs/src/libexec/spamd/spamd.c,v
retrieving revision 1.130
diff -u -p -u -r1.130 spamd.c
--- spamd.c 10 Sep 2015 13:56:12 -  1.130
+++ spamd.c 28 Oct 2015 14:30:57 -
@@ -1211,6 +1211,9 @@ main(int argc, char *argv[])
char *tlskeyfile = NULL;
char *tlscertfile = NULL;

+   if (pledge("stdio rpath wpath inet dns ioctl id route mcast proc 
flock ps", NULL) == -1)

+   err(1, "pledge");
+
tzset();
openlog_r("spamd", LOG_PID | LOG_NDELAY, LOG_DAEMON, &sdata);

@@ -1227,6 +1230,10 @@ main(int argc, char *argv[])
if (gethostname(hostname, sizeof hostname) == -1)
err(1, "gethostname");
maxfiles = get_maxfiles();
+
+   if (pledge("stdio rpath wpath inet dns ioctl id route mcast proc 
flock", NULL) == -1)

+   err(1, "pledge");
+
if (maxcon > maxfiles)
maxcon = maxfiles;
if (maxblack > maxfiles)


Best regards,
Ricardo Mestre



Re: pledge(2) and spamd(8)

2015-10-28 Thread Theo de Raadt
> I was just trying to pledge(2) spamd(8), nevertheless came across 2 
> priviliges kern_pledge.c is missing for this to work.
>
> First spamd(8) needs to read sysctl kern.maxfiles in order to see if it 
> can launch with that value or not, and second if the multicast options 
> are passed as parameters then it also needs IP_MULTICAST_TTL since 
> spamd(8) calls setsockopt(2) with that option set:

I am not a fan of this approach.  Your diff is very close to

pledge("everything")

That is a very small stopgap against a problem.  Though you now have a
list of things being done in one process, and good argument for someone
to refactor this into privsep..

My gut reaction is to not allow these two operations.  I normally wait
until I see evidence other programs need such operations, while being
very strongly protected from pledge.  Not seeing that here.



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

2015-10-28 Thread Joerg Jung


> Am 28.10.2015 um 17:05 schrieb Michael McConville :
> 
> Relayd, httpd, and ntpd define the functions get_data() and
> get_string(). Both call calloc and then immediately memcpy. Calloc's
> zeroing isn't optimized out. These functions are called in network data
> paths in at least a couple places, so all this extra writing could have
> a meaningful performance impact.

I think this impact is negligible small and I believe
that you can not even measure it.
So this change makes not so much sense to me.

I wonder if using strndup() would make more 
sense for the first case here?

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



patch saves some cycles by extending pfr_walktree() a bit

2015-10-28 Thread Alexandr Nedvedicky
Hello,

This is yet another patch, which 'scratches surface', this time in pf_table.c.
As briefly discussed in Varazdin the plan is to clean up pf_table.c a bit, to
make implementation of reference handling and further MP stuff bit easier.

I've noticed sub-optimal implementation table entries at two or three places.
The code typically looks as follows:

xxx pfr_enqueue_addrs(kt, &addrq, NULL, 0);
yyy SLIST_FOREACH(p, &addrq, pfrke_workq)
yyy pfr_ktable_winfo_update(kt, p);

at xxx we traverse whole table using underlying pfr_walktree() function, which
links the whole table into single link list (a.k.a. work queue).  at yyy we
walk through whole list applying pfr_ktable_winfo_update() to every address
(kentry). This requires 2 x n steps. Patch below improves that to n by
extending pfr_walktree() so it is able to call desired function.

Also the patch is part of my effort to kill work queues in radix tables.

thanks and
regards
sasha

8<---8<---8<--8<

Index: pf_table.c
===
RCS file: /cvs/src/sys/net/pf_table.c,v
retrieving revision 1.115
diff -u -p -r1.115 pf_table.c
--- pf_table.c  7 Oct 2015 11:57:44 -   1.115
+++ pf_table.c  27 Oct 2015 21:40:18 -
@@ -107,7 +107,9 @@ struct pfr_walktree {
PFRW_GET_ADDRS,
PFRW_GET_ASTATS,
PFRW_POOL_GET,
-   PFRW_DYNADDR_UPDATE
+   PFRW_DYNADDR_UPDATE,
+   PFRW_WININFO_UPDATE,
+   PFRW_CLSTAT
}pfrw_op;
union {
struct pfr_addr *pfrw1_addr;
@@ -115,15 +117,22 @@ struct pfr_walktree {
struct pfr_kentryworkq  *pfrw1_workq;
struct pfr_kentry   *pfrw1_kentry;
struct pfi_dynaddr  *pfrw1_dyn;
+   struct {
+   time_t  tzero;
+   int negchange;
+   }pfrw1_clstat;
}pfrw_1;
int  pfrw_free;
int  pfrw_flags;
+   struct pfr_ktable   *pfrw_kt;
 };
 #define pfrw_addr  pfrw_1.pfrw1_addr
 #define pfrw_astatspfrw_1.pfrw1_astats
 #define pfrw_workq pfrw_1.pfrw1_workq
 #define pfrw_kentrypfrw_1.pfrw1_kentry
 #define pfrw_dyn   pfrw_1.pfrw1_dyn
+#definepfrw_tzero  pfrw_1.pfrw1_clstat.tzero
+#definepfrw_negchange  pfrw_1.pfrw1_clstat.negchange
 #define pfrw_cnt   pfrw_free
 
 #define senderr(e) do { rv = (e); goto _bad; } while (0)
@@ -156,6 +165,8 @@ void pfr_remove_kentries(struct 
pfr_k
struct pfr_kentryworkq *);
 voidpfr_clstats_kentries(struct pfr_kentryworkq *, time_t,
int);
+voidpfr_clstats_kentries_pfrw(struct pfr_ktable *, time_t,
+   int);
 voidpfr_reset_feedback(struct pfr_addr *, int, int);
 voidpfr_prepare_network(union sockaddr_union *, int, int);
 int pfr_route_kentry(struct pfr_ktable *,
@@ -181,6 +192,7 @@ int  pfr_ktable_compare(struct pfr_kta
struct pfr_ktable *);
 voidpfr_ktable_winfo_update(struct pfr_ktable *,
struct pfr_kentry *);
+voidpfr_ktable_winfo_update_sum(struct pfr_ktable *);
 struct pfr_ktable  *pfr_lookup_table(struct pfr_table *);
 voidpfr_clean_node_mask(struct pfr_ktable *,
struct pfr_kentryworkq *);
@@ -189,6 +201,8 @@ int  pfr_skip_table(struct pfr_table *
struct pfr_ktable *, int);
 struct pfr_kentry  *pfr_kentry_byidx(struct pfr_ktable *, int, int);
 int pfr_islinklocal(sa_family_t, struct pf_addr *);
+voidpfr_walk(struct pfr_ktable *, struct pfr_walktree *,
+   const char *);
 
 RB_PROTOTYPE(pfr_ktablehead, pfr_ktable, pfrkt_tree, pfr_ktable_compare);
 RB_GENERATE(pfr_ktablehead, pfr_ktable, pfrkt_tree, pfr_ktable_compare);
@@ -631,7 +645,6 @@ pfr_get_astats(struct pfr_table *tbl, st
 {
struct pfr_ktable   *kt;
struct pfr_walktree  w;
-   struct pfr_kentryworkq   workq;
int  rv;
time_t   tzero = time_second;
 
@@ -653,10 +666,8 @@ pfr_get_astats(struct pfr_table *tbl, st
rv = rn_walktree(kt->pfrkt_ip4, pfr_walktree, &w);
if (!rv)
rv = rn_walktree(kt->pfrkt_ip6, pfr_walktree, &w);
-   if (!rv && (flags & PFR_FLAG_CLSTATS)) {
-   pfr_enqueue_addrs(kt, &workq, NULL, 0);
-   pfr_clstats_kentries(&workq, tzero, 0);
-   }
+   if (!rv && (flags & PFR_FLAG_CLSTATS))
+   pfr_clstats

Patch 2/3 - make DIOCRDELADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexandr Nedvedicky
Hello,

this is the second patch in 3-patch series.

Patch changes DIOCRDELADDRS ioctl to DIOCRDELADDR, which accepts one IP address
only per ioctl(2) call. Patch updates kernel and pfctl(8) only. Other 
components,
which happen to use DIOCRDELADDRS will be updated by extra patch.

thanks and
regards
sasha

8<---8<---8<--8<

Index: sbin/pfctl/pfctl_radix.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_radix.c,v
retrieving revision 1.32
diff -u -p -r1.32 pfctl_radix.c
--- sbin/pfctl/pfctl_radix.c21 Jan 2015 21:50:33 -  1.32
+++ sbin/pfctl/pfctl_radix.c27 Oct 2015 22:56:54 -
@@ -207,6 +207,7 @@ pfr_del_addrs(struct pfr_table *tbl, str
 int *ndel, int flags)
 {
struct pfioc_table io;
+   int i, rv, del = 0;
 
if (tbl == NULL || size < 0 || (size && addr == NULL)) {
errno = EINVAL;
@@ -215,14 +216,18 @@ pfr_del_addrs(struct pfr_table *tbl, str
bzero(&io, sizeof io);
io.pfrio_flags = flags;
io.pfrio_table = *tbl;
-   io.pfrio_buffer = addr;
io.pfrio_esize = sizeof(*addr);
-   io.pfrio_size = size;
-   if (ioctl(dev, DIOCRDELADDRS, &io))
-   return (-1);
-   if (ndel != NULL)
-   *ndel = io.pfrio_ndel;
-   return (0);
+   io.pfrio_size = 1;
+   for (i = 0; (i < size) && (rv == 0); i++) {
+   io.pfrio_buffer = addr++;
+   rv = ioctl(dev, DIOCRDELADDR, &io);
+   del++;
+   }
+
+   if ((rv == 0) && (ndel != NULL))
+   *ndel = del;
+
+   return (rv);
 }
 
 int
Index: sys/net/pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.291
diff -u -p -r1.291 pf_ioctl.c
--- sys/net/pf_ioctl.c  13 Oct 2015 19:32:31 -  1.291
+++ sys/net/pf_ioctl.c  27 Oct 2015 22:57:20 -
@@ -835,7 +835,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case DIOCRCLRTSTATS:
case DIOCRCLRADDRS:
case DIOCRADDADDRS:
-   case DIOCRDELADDRS:
+   case DIOCRDELADDR:
case DIOCRSETADDRS:
case DIOCRGETASTATS:
case DIOCRCLRASTATS:
@@ -888,7 +888,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case DIOCRCLRTSTATS:
case DIOCRCLRADDRS:
case DIOCRADDADDRS:
-   case DIOCRDELADDRS:
+   case DIOCRDELADDR:
case DIOCRSETADDRS:
case DIOCRSETTFLAGS:
if (((struct pfioc_table *)addr)->pfrio_flags &
@@ -1829,16 +1829,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
 
-   case DIOCRDELADDRS: {
+   case DIOCRDELADDR: {
struct pfioc_table *io = (struct pfioc_table *)addr;
 
if (io->pfrio_esize != sizeof(struct pfr_addr)) {
error = ENODEV;
break;
}
-   error = pfr_del_addrs(&io->pfrio_table, io->pfrio_buffer,
-   io->pfrio_size, &io->pfrio_ndel, io->pfrio_flags |
-   PFR_FLAG_USERIOCTL);
+   error = pfr_del_addr(&io->pfrio_table, io->pfrio_buffer,
+   io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
break;
}
 
Index: sys/net/pf_table.c
===
RCS file: /cvs/src/sys/net/pf_table.c,v
retrieving revision 1.115
diff -u -p -r1.115 pf_table.c
--- sys/net/pf_table.c  7 Oct 2015 11:57:44 -   1.115
+++ sys/net/pf_table.c  27 Oct 2015 22:57:24 -
@@ -152,6 +152,8 @@ void pfr_destroy_kentries(struct 
pfr_
 voidpfr_destroy_kentry(struct pfr_kentry *);
 voidpfr_insert_kentries(struct pfr_ktable *,
struct pfr_kentryworkq *, time_t);
+voidpfr_remove_kentry(struct pfr_ktable *,
+   struct pfr_kentry *);
 voidpfr_remove_kentries(struct pfr_ktable *,
struct pfr_kentryworkq *);
 voidpfr_clstats_kentries(struct pfr_kentryworkq *, time_t,
@@ -343,14 +345,12 @@ _bad:
 }
 
 int
-pfr_del_addrs(struct pfr_table *tbl, struct pfr_addr *addr, int size,
-int *ndel, int flags)
+pfr_del_addr(struct pfr_table *tbl, struct pfr_addr *addr, int size, int flags)
 {
struct pfr_ktable   *kt;
-   struct pfr_kentryworkq   workq;
struct pfr_kentry   *p;
struct pfr_addr  ad;
-   int  i, rv, xdel = 0, log = 1;
+   int  rv;
 
ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL))
@@ -360,70 +3

Patch 3/3 - update userland to reflect DIOCRADDADDRS/DIOCRDELADDRS changes

2015-10-28 Thread Alexandr Nedvedicky
Hello,

this is the third patch in the first PF radix changes batch.  Patch requires
earlier patches to be in place, otherwise compilation will fail.

Patch updates various user land tools by new PF radix table changes:
s/DIOCRADDADDRS/DIOCRADDADDR
s/DIOCRDELADDRS/DIOCRDELADDR   
it's also no longer possible to pass more than 1 IP address per ioctl(2) call.
Patch updates those tools:
usr.sbin/authpf/authpf.c
usr.sbin/bgpd/pftable.c
usr.sbin/dhcpd/pfutils.c

thanks and
regards
sasha

8<---8<---8<--8<

Index: usr.sbin/authpf/authpf.c
===
RCS file: /cvs/src/usr.sbin/authpf/authpf.c,v
retrieving revision 1.123
diff -u -p -r1.123 authpf.c
--- usr.sbin/authpf/authpf.c21 Jan 2015 21:50:32 -  1.123
+++ usr.sbin/authpf/authpf.c27 Oct 2015 23:54:48 -
@@ -872,7 +872,7 @@ change_table(int add, const char *ipsrc)
return (-1);
}
 
-   if (ioctl(dev, add ? DIOCRADDADDRS : DIOCRDELADDRS, &io) &&
+   if (ioctl(dev, add ? DIOCRADDADDR : DIOCRDELADDR, &io) &&
errno != ESRCH) {
syslog(LOG_ERR, "cannot %s %s from table %s: %s",
add ? "add" : "remove", ipsrc, tablename,
Index: usr.sbin/bgpd/pftable.c
===
RCS file: /cvs/src/usr.sbin/bgpd/pftable.c,v
retrieving revision 1.8
diff -u -p -r1.8 pftable.c
--- usr.sbin/bgpd/pftable.c 21 Jan 2015 21:50:32 -  1.8
+++ usr.sbin/bgpd/pftable.c 27 Oct 2015 23:54:49 -
@@ -57,6 +57,8 @@ pftable_change(struct pf_table *pft)
 {
struct pfioc_table tio;
int ret;
+   int i;
+   struct pfr_addr *addr;
 
if (pft->naddrs == 0 || pft->what == 0)
return (0);
@@ -67,11 +69,15 @@ pftable_change(struct pf_table *pft)
bzero(&tio, sizeof(tio));
strlcpy(tio.pfrio_table.pfrt_name, pft->name,
sizeof(tio.pfrio_table.pfrt_name));
-   tio.pfrio_buffer = pft->worklist;
tio.pfrio_esize = sizeof(*pft->worklist);
-   tio.pfrio_size = pft->naddrs;
+   tio.pfrio_size = 1;
 
ret = ioctl(devpf, pft->what, &tio);
+   addr = pft->worklist;
+   for (i = 0; (i < pft->naddrs) && (ret == 0); i++) {
+   tio.pfrio_buffer = addr++;
+   ret = ioctl(devpf, pft->what, &tio);
+   }
 
/* bad prefixes shouldn't cause us to die */
if (ret == -1) {
@@ -193,7 +199,7 @@ pftable_add_work(const char *table, stru
}
 
/* Only one type of work on the list at a time */
-   what = del ? DIOCRDELADDRS : DIOCRADDADDRS;
+   what = del ? DIOCRDELADDR : DIOCRADDADDR;
if (pft->naddrs != 0 && pft->what != what)
fatal("attempt to mix pf table additions/deletions");
 
Index: usr.sbin/dhcpd/pfutils.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/pfutils.c,v
retrieving revision 1.13
diff -u -p -r1.13 pfutils.c
--- usr.sbin/dhcpd/pfutils.c5 Feb 2015 09:42:52 -   1.13
+++ usr.sbin/dhcpd/pfutils.c27 Oct 2015 23:54:51 -
@@ -154,7 +154,7 @@ pf_change_table(int fd, int op, struct i
addr.pfra_af = AF_INET;
addr.pfra_net = 32;
 
-   if (ioctl(fd, op ? DIOCRADDADDRS : DIOCRDELADDRS, &io) &&
+   if (ioctl(fd, op ? DIOCRADDADDR : DIOCRDELADDR, &io) &&
errno != ESRCH) {
warning( "DIOCR%sADDRS on table %s: %s",
op ? "ADD" : "DEL", table, strerror(errno));



Patch 1/3 - make DIOCRADDADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexandr Nedvedicky
Hello,

this is the first patch in series of three. All patches modify PF radix
table API such the ioctl() functions accept one IP address per call.
The idea has been proposed by Claudio at Varazdin.

I still have to untangle pfr_commit_ktable() and DIOCRSETADDRS ioctl.  Both
seem to be more complicated than DIOCRADDADDRS and DIOCRDELADDRS (subject of
next patch).

Patch changes DIOCRADDADDRS ioctl to DIOCRADDADDRS, which accepts one IP
address only per ioctl(2) call. Patch updates kernel and pfctl(8) only. Other
components, which happen to use DIOCRADDADDRS will be updated by extra patch.

thanks and
regards
sasha

P.S. I still need to update pf(4) manpage, I'll do it as soon as there we
will reach agreement on proposed patches.

8<---8<---8<--8<

Index: sbin/pfctl/pfctl_radix.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_radix.c,v
retrieving revision 1.32
diff -u -p -r1.32 pfctl_radix.c
--- sbin/pfctl/pfctl_radix.c21 Jan 2015 21:50:33 -  1.32
+++ sbin/pfctl/pfctl_radix.c27 Oct 2015 23:24:59 -
@@ -184,6 +184,7 @@ pfr_add_addrs(struct pfr_table *tbl, str
 int *nadd, int flags)
 {
struct pfioc_table io;
+   int i, rv, add = 0;
 
if (tbl == NULL || size < 0 || (size && addr == NULL)) {
errno = EINVAL;
@@ -192,14 +193,18 @@ pfr_add_addrs(struct pfr_table *tbl, str
bzero(&io, sizeof io);
io.pfrio_flags = flags;
io.pfrio_table = *tbl;
-   io.pfrio_buffer = addr;
io.pfrio_esize = sizeof(*addr);
-   io.pfrio_size = size;
-   if (ioctl(dev, DIOCRADDADDRS, &io))
-   return (-1);
-   if (nadd != NULL)
-   *nadd = io.pfrio_nadd;
-   return (0);
+   io.pfrio_size = 1;  /* TODO: check .pfrio_size is needed */
+   for (i = 0; (i < size) && (rv == 0); i++) {
+   io.pfrio_buffer = addr++;
+   rv = ioctl(dev, DIOCRADDADDR, &io);
+   add++;
+   }
+
+   if ((rv == 0) && (nadd != NULL))
+   *nadd = add;
+
+   return (rv);
 }
 
 int
Index: sys/net/pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.291
diff -u -p -r1.291 pf_ioctl.c
--- sys/net/pf_ioctl.c  13 Oct 2015 19:32:31 -  1.291
+++ sys/net/pf_ioctl.c  27 Oct 2015 23:25:23 -
@@ -834,7 +834,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case DIOCRGETTSTATS:
case DIOCRCLRTSTATS:
case DIOCRCLRADDRS:
-   case DIOCRADDADDRS:
+   case DIOCRADDADDR:
case DIOCRDELADDRS:
case DIOCRSETADDRS:
case DIOCRGETASTATS:
@@ -887,7 +887,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case DIOCRDELTABLES:
case DIOCRCLRTSTATS:
case DIOCRCLRADDRS:
-   case DIOCRADDADDRS:
+   case DIOCRADDADDR:
case DIOCRDELADDRS:
case DIOCRSETADDRS:
case DIOCRSETTFLAGS:
@@ -1816,16 +1816,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
 
-   case DIOCRADDADDRS: {
+   case DIOCRADDADDR: {
struct pfioc_table *io = (struct pfioc_table *)addr;
 
if (io->pfrio_esize != sizeof(struct pfr_addr)) {
error = ENODEV;
break;
}
-   error = pfr_add_addrs(&io->pfrio_table, io->pfrio_buffer,
-   io->pfrio_size, &io->pfrio_nadd, io->pfrio_flags |
-   PFR_FLAG_USERIOCTL);
+   error = pfr_add_addr(&io->pfrio_table, io->pfrio_buffer,
+   io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
break;
}
 
Index: sys/net/pf_table.c
===
RCS file: /cvs/src/sys/net/pf_table.c,v
retrieving revision 1.115
diff -u -p -r1.115 pf_table.c
--- sys/net/pf_table.c  7 Oct 2015 11:57:44 -   1.115
+++ sys/net/pf_table.c  27 Oct 2015 23:25:26 -
@@ -266,6 +266,54 @@ pfr_clr_addrs(struct pfr_table *tbl, int
 }
 
 int
+pfr_add_addr(struct pfr_table *tbl, struct pfr_addr *addr, int size, int flags)
+{
+   struct pfr_ktable   *kt;
+   struct pfr_kentry   *p;
+   struct pfr_addr  ad;
+   int  rv;
+   time_t   tzero = time_second;
+
+   ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
+   if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL))
+   return (EINVAL);
+   kt = pfr_lookup_table(tbl);
+   if (kt == NULL || !(kt->pfrkt_flags & PFR_TFLAG_ACTIVE))
+   return (ESRCH);
+   if (kt->pfrkt_flags & PFR_TFLAG_CONST)
+   return (EPERM);
+   if (COPYIN(addr, &ad

Re: pledge(2) and spamd(8)

2015-10-28 Thread Benny Lofgren
On 2015-10-28 17:47, Theo de Raadt wrote:
>> I was just trying to pledge(2) spamd(8), nevertheless came across 2 
>> priviliges kern_pledge.c is missing for this to work.
>>
>> First spamd(8) needs to read sysctl kern.maxfiles in order to see if it 
>> can launch with that value or not, and second if the multicast options 
>> are passed as parameters then it also needs IP_MULTICAST_TTL since 
>> spamd(8) calls setsockopt(2) with that option set:
> 
> I am not a fan of this approach.  Your diff is very close to
> 
>   pledge("everything")
> 
> That is a very small stopgap against a problem.  Though you now have a
> list of things being done in one process, and good argument for someone
> to refactor this into privsep..
> 
> My gut reaction is to not allow these two operations.  I normally wait
> until I see evidence other programs need such operations, while being
> very strongly protected from pledge.  Not seeing that here.


Also, I wonder what the point of having a sanity check against
kern.maxfiles at all is, especially with the arbitrary-feeling
additional rule of "maxcon may not exceed kern.maxfiles - 200". It feels
redundant to me, and it sort of makes a promise of protection it can't
uphold.

I mean, it's a system-wide limit and spamd has no control over the
resource usage of the rest of the system, so it still has to (hopefully
gracefully) handle the case when it runs up against either kern.maxfiles
or RLIMIT_NOFILE.

Removing that check at least simplifies that part of spamd's pledge().


Regards,
/Benny



Re: preparing pfi_kif to MP world

2015-10-28 Thread Alexandr Nedvedicky
Hello Mike,

just a quick question:

are you going to commit your pfi_kif_find() et. al.?
or more work is needed there?

thanks a lot
regards
sasha

> 
> Turns out this is a rather simple issue that got slightly
> complicated by the code diverging quite a bit since the
> inception.  Essentially the clr->ifname comes from the
> interface specification in the "pfctl -i foo0 -Fs" for
> if-bound states (floating states use fake interface "any").
> 
> Previously states have been hanging off of kif nodes but it's
> long gone and we can simply iterate over the state table tree
> (or even a state list like it's done in the DIOCGETSTATES in
> pf_ioctl).
> 
> Calling pf_kif_get here wouldn't be prudent because spawning
> new objects while disposing of the other ones seems somewhat
> counterproductive.
> 
diff --git sys/net/if_pfsync.c sys/net/if_pfsync.c
index 7d633db..fcaf5f5 100644
--- sys/net/if_pfsync.c
+++ sys/net/if_pfsync.c
@@ -752,46 +752,28 @@ done:
 
 int
 pfsync_in_clr(caddr_t buf, int len, int count, int flags)
 {
struct pfsync_clr *clr;
-   int i;
-
struct pf_state *st, *nexts;
-   struct pf_state_key *sk, *nextsk;
-   struct pf_state_item *si;
+   struct pfi_kif *kif = NULL;
u_int32_t creatorid;
+   int i;
 
for (i = 0; i < count; i++) {
clr = (struct pfsync_clr *)buf + len * i;
creatorid = clr->creatorid;
+   if (strlen(clr->ifname) &&
+   (kif = pfi_kif_find(clr->ifname)) == NULL)
+   continue;
 
-   if (clr->ifname[0] == '\0') {
-   for (st = RB_MIN(pf_state_tree_id, &tree_id);
-   st; st = nexts) {
-   nexts = RB_NEXT(pf_state_tree_id, &tree_id, st);
-   if (st->creatorid == creatorid) {
-   SET(st->state_flags, PFSTATE_NOSYNC);
-   pf_unlink_state(st);
-   }
-   }
-   } else {
-   if (pfi_kif_get(clr->ifname) == NULL)
-   continue;
-
-   /* XXX correct? */
-   for (sk = RB_MIN(pf_state_tree, &pf_statetbl);
-   sk; sk = nextsk) {
-   nextsk = RB_NEXT(pf_state_tree,
-   &pf_statetbl, sk);
-   TAILQ_FOREACH(si, &sk->states, entry) {
-   if (si->s->creatorid == creatorid) {
-   SET(si->s->state_flags,
-   PFSTATE_NOSYNC);
-   pf_unlink_state(si->s);
-   }
-   }
+   for (st = RB_MIN(pf_state_tree_id, &tree_id); st; st = nexts) {
+   nexts = RB_NEXT(pf_state_tree_id, &tree_id, st);
+   if (st->creatorid == creatorid &&
+   ((kif && st->kif == kif) || !kif)) {
+   SET(st->state_flags, PFSTATE_NOSYNC);
+   pf_unlink_state(st);
}
}
}
 
return (0);
diff --git sys/net/pf_if.c sys/net/pf_if.c
index caaf9f9..bf77184 100644
--- sys/net/pf_if.c
+++ sys/net/pf_if.c
@@ -97,18 +97,25 @@ pfi_initialize(void)
if ((pfi_all = pfi_kif_get(IFG_ALL)) == NULL)
panic("pfi_kif_get for pfi_all failed");
 }
 
 struct pfi_kif *
-pfi_kif_get(const char *kif_name)
+pfi_kif_find(const char *kif_name)
 {
-   struct pfi_kif  *kif;
struct pfi_kif_cmp   s;
 
bzero(&s, sizeof(s));
strlcpy(s.pfik_name, kif_name, sizeof(s.pfik_name));
-   if ((kif = RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s)) != NULL)
+   return (RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s));
+}
+
+struct pfi_kif *
+pfi_kif_get(const char *kif_name)
+{
+   struct pfi_kif  *kif;
+
+   if ((kif = pfi_kif_find(kif_name)))
return (kif);
 
/* create new one */
if ((kif = malloc(sizeof(*kif), PFI_MTYPE, M_NOWAIT|M_ZERO)) == NULL)
return (NULL);
diff --git sys/net/pfvar.h sys/net/pfvar.h
index cdb2f7f..76a98a9 100644
--- sys/net/pfvar.h
+++ sys/net/pfvar.h
@@ -1808,10 +1808,11 @@ int pfr_ina_define(struct pfr_table *, struct 
pfr_addr *, int, int *,
int *, u_int32_t, int);
 
 extern struct pfi_kif  *pfi_all;
 
 voidpfi_initialize(void);
+struct pfi_kif *pfi_kif_find(const char *);
 struct pfi_kif *pfi_kif_get(const char *);
 voidpfi_kif_ref(struct pfi_kif *, enum pfi_kif_refs);
 voidpfi_kif_unref(struct pfi_kif *, enum pfi_kif_refs);
 int pfi_kif_match(struct pf

Re: pledge(2) and spamd(8)

2015-10-28 Thread Theo de Raadt
> Also, I wonder what the point of having a sanity check against
> kern.maxfiles at all is, especially with the arbitrary-feeling
> additional rule of "maxcon may not exceed kern.maxfiles - 200". It feels
> redundant to me, and it sort of makes a promise of protection it can't
> uphold.

That code predates the addition of getdtablecount().  Maybe someone
can think about improving it.

> I mean, it's a system-wide limit and spamd has no control over the
> resource usage of the rest of the system, so it still has to (hopefully
> gracefully) handle the case when it runs up against either kern.maxfiles
> or RLIMIT_NOFILE.

Graceful recovery can include the heaviest programs avoiding getting
to that point.  Shrug.  These are just descriptors.

But honestly, a lot of programs don't know how to cope correctly with
ENFILE and EMFILE.  benno and I put a lot of effort into improving this
about 3 years ago for the most sensitive programs, but fixing all the
programs is a harder job.

> Removing that check at least simplifies that part of spamd's pledge().

Yes that covers up a scratch on one deck chair.



Re: Patch 2/3 - make DIOCRDELADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexey Suslikov
Alexandr Nedvedicky  oracle.com> writes:

> Index: sbin/pfctl/pfctl_radix.c
> + io.pfrio_size = 1;

in 1/3 you have annotated like this

+   io.pfrio_size = 1;  /* TODO: check .pfrio_size is needed */

you either don't need similar comment here, or...?

> Index: sbin/pfctl/pfctl_radix.c
> + io.pfrio_size = 1;

same here.



Re: Patch 2/3 - make DIOCRDELADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexandr Nedvedicky
Hello,

> > Index: sbin/pfctl/pfctl_radix.c
> > +   io.pfrio_size = 1;
> 
> in 1/3 you have annotated like this
> 
> + io.pfrio_size = 1;  /* TODO: check .pfrio_size is needed */
> 

sorry this has leaked out from my internal repo. The .pfrio_size member will be
dropped as soon as I'll be done with the rest of PF radix table ioctls.

thanks for catching that.

regards
sasha




Re: [PATCH] Add a simple roundrobin load balancing feature to rebound(8)

2015-10-28 Thread Ted Unangst
Dimitris Papastamos wrote:
> Hi,
> 
> I thought it would be cool for rebound(8) to load balance on a number of
> DNS servers.
> 
> While I was working on this, I did not manage to convince myself as to
> whether this should be the default behaviour.
> 
> An alternative default would be to use the master server provided.  If
> requests started timing out, it would switch to the secondary.  If the
> master became available again, then it would switch back to it.
> 
> This diff may be useful regardless of what the default should be and
> given that there is no config parser at the moment that can handle
> options I thought I would send this out to get some feedback.

This is indeed an open option. Since the goal here is to have libc always
query rebound, it should probably emulate the current behavior when you have
multiple nameservers in resolv.conf. But this is a little more complicated,
as it requires tracking nexthop per server and probably changing the timeout
because libc may not know how many servers there are... I'd like to wait a
bit.



Re: boot from softraid, backspace in passphrase prompt

2015-10-28 Thread Ted Unangst
Theo de Raadt wrote:
> Since people are typing blind, can you add support for ^U as well?
> 
> > I want correct typing mistakes when booting from softraid crypto disks.
> > Can we handle at least the backspace key, plz^Hease? :)

Yes, and yes. ^U should be 0x15 and we'll just hope the terminal works?
But ok with me to start easy and just handle backspace, too.



Re: boot from softraid, backspace in passphrase prompt

2015-10-28 Thread Theo de Raadt
> Theo de Raadt wrote:
> > Since people are typing blind, can you add support for ^U as well?
> > 
> > > I want correct typing mistakes when booting from softraid crypto disks.
> > > Can we handle at least the backspace key, plz^Hease? :)
> 
> Yes, and yes. ^U should be 0x15 and we'll just hope the terminal works?
> But ok with me to start easy and just handle backspace, too.

Well it has not echo'd ^H, and should not echo ^U either.



cron: get rid of strcmp_until

2015-10-28 Thread Todd C. Miller
It was only used in env_set().  I've also removed the useless
FACILITY define and fixed a sizeof().

 - todd

Index: usr.sbin/cron/env.c
===
RCS file: /cvs/src/usr.sbin/cron/env.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 env.c
--- usr.sbin/cron/env.c 9 Feb 2015 22:35:08 -   1.29
+++ usr.sbin/cron/env.c 28 Oct 2015 20:41:54 -
@@ -22,7 +22,7 @@
 char **
 env_init(void)
 {
-   char **p = malloc(sizeof(char **));
+   char **p = malloc(sizeof(char *));
 
if (p != NULL)
p[0] = NULL;
@@ -66,46 +66,34 @@ env_copy(char **envp)
 char **
 env_set(char **envp, char *envstr)
 {
-   int count, found;
-   char **p, *envtmp;
+   size_t count, len;
+   char **p, *envcopy;
+
+   if ((envcopy = strdup(envstr)) == NULL)
+   return (NULL);
 
/*
-* count the number of elements, including the null pointer;
-* also set 'found' to -1 or index of entry if already in here.
+* look for envstr in envp and replace it if found, else we
+* will realloc envp and append the new entry.
 */
-   found = -1;
-   for (count = 0; envp[count] != NULL; count++) {
-   if (!strcmp_until(envp[count], envstr, '='))
-   found = count;
-   }
-   count++;/* for the NULL */
-
-   if (found != -1) {
-   /*
-* it exists already, so just free the existing setting,
-* save our new one there, and return the existing array.
-*/
-   if ((envtmp = strdup(envstr)) == NULL)
-   return (NULL);
-   free(envp[found]);
-   envp[found] = envtmp;
-   return (envp);
+   len = strcspn(envstr, "=");
+   for (p = envp; *p != NULL; p++) {
+   if (strcspn(*p, "=") == len &&
+   strncmp(envstr, *p, len) == 0) {
+   free(*p);
+   *p = envcopy;
+   return (envp);
+   }
}
+   count = (size_t)(p - envp);
 
-   /*
-* it doesn't exist yet, so resize the array, move null pointer over
-* one, save our string over the old null pointer, and return resized
-* array.
-*/
-   if ((envtmp = strdup(envstr)) == NULL)
-   return (NULL);
-   p = reallocarray(envp, count+1, sizeof(char **));
+   p = reallocarray(envp, count + 2, sizeof(char **));
if (p == NULL) {
-   free(envtmp);
+   free(envcopy);
return (NULL);
}
-   p[count] = p[count-1];
-   p[count-1] = envtmp;
+   p[count++] = envcopy;
+   p[count] = NULL;
return (p);
 }
 
Index: usr.sbin/cron/funcs.h
===
RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
retrieving revision 1.19
diff -u -p -u -r1.19 funcs.h
--- usr.sbin/cron/funcs.h   6 Oct 2015 14:58:37 -   1.19
+++ usr.sbin/cron/funcs.h   28 Oct 2015 20:09:39 -
@@ -50,7 +50,6 @@ int   job_runqueue(void),
load_env(char *, FILE *),
cron_pclose(FILE *, pid_t),
glue_strings(char *, size_t, const char *, const char *, char),
-   strcmp_until(const char *, const char *, char),
allowed(const char *, const char *, const char *),
open_socket(void),
safe_p(const char *, const char *),
Index: usr.sbin/cron/misc.c
===
RCS file: /cvs/src/usr.sbin/cron/misc.c,v
retrieving revision 1.60
diff -u -p -u -r1.60 misc.c
--- usr.sbin/cron/misc.c26 Oct 2015 15:16:30 -  1.60
+++ usr.sbin/cron/misc.c28 Oct 2015 20:09:47 -
@@ -20,33 +20,9 @@
 #include "cron.h"
 #include 
 
-#if defined(LOG_DAEMON) && !defined(LOG_CRON)
-# define LOG_CRON LOG_DAEMON
-#endif
-
-#ifndef FACILITY
-#define FACILITY LOG_CRON
-#endif
-
 static int LogFD = -1;
-
 static int syslog_open = FALSE;
 
-int
-strcmp_until(const char *left, const char *right, char until)
-{
-   while (*left && *left != until && *left == *right) {
-   left++;
-   right++;
-   }
-
-   if ((*left=='\0' || *left == until) &&
-   (*right=='\0' || *right == until)) {
-   return (0);
-   }
-   return (*left - *right);
-}
-
 void
 set_cron_cwd(void)
 {
@@ -272,7 +248,7 @@ log_it(const char *username, pid_t xpid,
"END EDIT", "LIST", "MAIL", "RELOAD", "REPLACE", "STARTUP", NULL };
 
if (!syslog_open) {
-   openlog(ProgramName, LOG_PID, FACILITY);
+   openlog(ProgramName, LOG_PID, LOG_CRON);
syslog_open = TRUE; /* assume openlog success */
}
 



Re: preparing pfi_kif to MP world

2015-10-28 Thread Mike Belopuhov
On 28 October 2015 at 18:41, Alexandr Nedvedicky
 wrote:
> Hello Mike,
>
> just a quick question:
>
> are you going to commit your pfi_kif_find() et. al.?
> or more work is needed there?
>

I need OKs

> thanks a lot
> regards
> sasha
>
>>
>> Turns out this is a rather simple issue that got slightly
>> complicated by the code diverging quite a bit since the
>> inception.  Essentially the clr->ifname comes from the
>> interface specification in the "pfctl -i foo0 -Fs" for
>> if-bound states (floating states use fake interface "any").
>>
>> Previously states have been hanging off of kif nodes but it's
>> long gone and we can simply iterate over the state table tree
>> (or even a state list like it's done in the DIOCGETSTATES in
>> pf_ioctl).
>>
>> Calling pf_kif_get here wouldn't be prudent because spawning
>> new objects while disposing of the other ones seems somewhat
>> counterproductive.
>>
> diff --git sys/net/if_pfsync.c sys/net/if_pfsync.c
> index 7d633db..fcaf5f5 100644
> --- sys/net/if_pfsync.c
> +++ sys/net/if_pfsync.c
> @@ -752,46 +752,28 @@ done:
>
>  int
>  pfsync_in_clr(caddr_t buf, int len, int count, int flags)
>  {
> struct pfsync_clr *clr;
> -   int i;
> -
> struct pf_state *st, *nexts;
> -   struct pf_state_key *sk, *nextsk;
> -   struct pf_state_item *si;
> +   struct pfi_kif *kif = NULL;
> u_int32_t creatorid;
> +   int i;
>
> for (i = 0; i < count; i++) {
> clr = (struct pfsync_clr *)buf + len * i;
> creatorid = clr->creatorid;
> +   if (strlen(clr->ifname) &&
> +   (kif = pfi_kif_find(clr->ifname)) == NULL)
> +   continue;
>
> -   if (clr->ifname[0] == '\0') {
> -   for (st = RB_MIN(pf_state_tree_id, &tree_id);
> -   st; st = nexts) {
> -   nexts = RB_NEXT(pf_state_tree_id, &tree_id, 
> st);
> -   if (st->creatorid == creatorid) {
> -   SET(st->state_flags, PFSTATE_NOSYNC);
> -   pf_unlink_state(st);
> -   }
> -   }
> -   } else {
> -   if (pfi_kif_get(clr->ifname) == NULL)
> -   continue;
> -
> -   /* XXX correct? */
> -   for (sk = RB_MIN(pf_state_tree, &pf_statetbl);
> -   sk; sk = nextsk) {
> -   nextsk = RB_NEXT(pf_state_tree,
> -   &pf_statetbl, sk);
> -   TAILQ_FOREACH(si, &sk->states, entry) {
> -   if (si->s->creatorid == creatorid) {
> -   SET(si->s->state_flags,
> -   PFSTATE_NOSYNC);
> -   pf_unlink_state(si->s);
> -   }
> -   }
> +   for (st = RB_MIN(pf_state_tree_id, &tree_id); st; st = nexts) 
> {
> +   nexts = RB_NEXT(pf_state_tree_id, &tree_id, st);
> +   if (st->creatorid == creatorid &&
> +   ((kif && st->kif == kif) || !kif)) {
> +   SET(st->state_flags, PFSTATE_NOSYNC);
> +   pf_unlink_state(st);
> }
> }
> }
>
> return (0);
> diff --git sys/net/pf_if.c sys/net/pf_if.c
> index caaf9f9..bf77184 100644
> --- sys/net/pf_if.c
> +++ sys/net/pf_if.c
> @@ -97,18 +97,25 @@ pfi_initialize(void)
> if ((pfi_all = pfi_kif_get(IFG_ALL)) == NULL)
> panic("pfi_kif_get for pfi_all failed");
>  }
>
>  struct pfi_kif *
> -pfi_kif_get(const char *kif_name)
> +pfi_kif_find(const char *kif_name)
>  {
> -   struct pfi_kif  *kif;
> struct pfi_kif_cmp   s;
>
> bzero(&s, sizeof(s));
> strlcpy(s.pfik_name, kif_name, sizeof(s.pfik_name));
> -   if ((kif = RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s)) != 
> NULL)
> +   return (RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s));
> +}
> +
> +struct pfi_kif *
> +pfi_kif_get(const char *kif_name)
> +{
> +   struct pfi_kif  *kif;
> +
> +   if ((kif = pfi_kif_find(kif_name)))
> return (kif);
>
> /* create new one */
> if ((kif = malloc(sizeof(*kif), PFI_MTYPE, M_NOWAIT|M_ZERO)) == NULL)
> return (NULL);
> diff --git sys/net/pfvar.h sys/net/pfvar.h
> index cdb2f7f..76a98a9 100644
> --- sys/net/pfvar.h
> +++ sys/net/pfvar.h
> @@ -1808,10 +1808,11 @@ int pfr_ina_define(struct pfr_table *, struct 
> pfr_addr *, int, int *,
> int *, u_int32_t, int);
>
>  extern struct pfi_kif  *pfi_all;
>
>  

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

2015-10-28 Thread Michael McConville
Joerg Jung wrote:
> > 
> > Michael McConville wrote:
> > 
> > Relayd, httpd, and ntpd define the functions get_data() and
> > get_string(). Both call calloc and then immediately memcpy. Calloc's
> > zeroing isn't optimized out. These functions are called in network data
> > paths in at least a couple places, so all this extra writing could have
> > a meaningful performance impact.
> 
> I think this impact is negligible small and I believe that you can not
> even measure it. So this change makes not so much sense to me.

I tried tracing calloc calls on a typical HTTP request and didn't find
much. I came across the fact that gmtime_r(3) callocs >41 KB, which is
unfortunate, but I don't know if there's an easy way around that.

> I wonder if using strndup() would make more sense for the first case
> here?

I was thinking the same thing. I guess it doesn't matter much, though.



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

2015-10-28 Thread Ted Unangst
Joerg Jung wrote:
> 
> 
> > Am 28.10.2015 um 17:05 schrieb Michael McConville :
> > 
> > Relayd, httpd, and ntpd define the functions get_data() and
> > get_string(). Both call calloc and then immediately memcpy. Calloc's
> > zeroing isn't optimized out. These functions are called in network data
> > paths in at least a couple places, so all this extra writing could have
> > a meaningful performance impact.
> 
> I think this impact is negligible small and I believe
> that you can not even measure it.
> So this change makes not so much sense to me.

I think there's no reason to be inefficient if the faster way of doing things
is just as simple and correct.

> I wonder if using strndup() would make more 
> sense for the first case here?

Agreed on that point.