Re: uchcom(4) did not work

2014-05-14 Thread Martin Pieuchot
On 13/05/14(Tue) 21:24, Mike Larkin wrote:
 On Wed, May 14, 2014 at 11:02:49AM +0900, SASANO Takayoshi wrote:
  Hi,
  
  Simply magic values are rewrited with #define.
  If these values need to be disassembled, please take a while...
  
 
 I think we need to understand what those values mean. When I mentioned
 #defines, I meant something like:
 
 #define UCHCOM_SOME_FLAG 0x1234
 #define UCHCOM_SOME_OTHER_FLAG 0x5678
 ...
 ...
 #define UCHCOM_RESET_VALUE (UCHCOM_SOME_FLAG | UCHCOM_SOME_OTHER_FLAG)
 
 That way we know what the values do. If the value we're setting is not
 a flag, we should understand what 0x501F and 0xD90A actually mean.

Unfortunately a lot of USB drivers are written without spec. and some
values are taken by analysing the traffic generated by drivers on other
OS.

 If we don't do it this way, the next person to come along and try to
 work on this code won't have any idea what to do.

I also really don't like magic values, but I don't see how we could do
it differently.  Maybe somebody can send an email to the author of the
linux driver and ask him what these values are.  But I'd bet he doesn't
no neither.

  
  SASANO Takayoshi u...@mx5.nisiq.net
  
  Index: uchcom.c
  ===
  RCS file: /cvs/src/sys/dev/usb/uchcom.c,v
  retrieving revision 1.19
  diff -u -p -r1.19 uchcom.c
  --- uchcom.c15 Nov 2013 10:17:39 -  1.19
  +++ uchcom.c14 May 2014 01:43:34 -
  @@ -91,18 +91,14 @@ int uchcomdebug = 0;
   #define UCHCOM_BRK1_MASK   0x01
   #define UCHCOM_BRK2_MASK   0x40
   
  -#define UCHCOM_LCR1_MASK   0xAF
  -#define UCHCOM_LCR2_MASK   0x07
  -#define UCHCOM_LCR1_PARENB 0x80
  -#define UCHCOM_LCR2_PAREVEN0x07
  -#define UCHCOM_LCR2_PARODD 0x06
  -#define UCHCOM_LCR2_PARMARK0x05
  -#define UCHCOM_LCR2_PARSPACE   0x04
  -
   #define UCHCOM_INTR_STAT1  0x02
   #define UCHCOM_INTR_STAT2  0x03
   #define UCHCOM_INTR_LEAST  4
   
  +/* these values come from Linux (drivers/usb/serial/ch341.c) */
  +#define UCHCOM_RESET_VALUE 0x501F  /* XXX default line mode? */
  +#define UCHCOM_RESET_INDEX 0xD90A  /* XXX default baud rate? */
  +
   #define UCHCOMIBUFSIZE 256
   #define UCHCOMOBUFSIZE 256
   
  @@ -707,27 +703,10 @@ uchcom_set_dte_rate(struct uchcom_softc 
   int
   uchcom_set_line_control(struct uchcom_softc *sc, tcflag_t cflag)
   {
  -   usbd_status err;
  -   uint8_t lcr1 = 0, lcr2 = 0;
  -
  -   err = uchcom_read_reg(sc, UCHCOM_REG_LCR1, lcr1, UCHCOM_REG_LCR2,
  -   lcr2);
  -   if (err) {
  -   printf(%s: cannot get LCR: %s\n,
  -  sc-sc_dev.dv_xname, usbd_errstr(err));
  -   return EIO;
  -   }
  -
  -   lcr1 = ~UCHCOM_LCR1_MASK;
  -   lcr2 = ~UCHCOM_LCR2_MASK;
  -
  /*
   * XXX: it is difficult to handle the line control appropriately:
  -*   - CS8, !CSTOPB and any parity mode seems ok, but
  -*   - the chip doesn't have the function to calculate parity
  -* in !CS8 mode.
  -*   - it is unclear that the chip supports CS5,6 mode.
  -*   - it is unclear how to handle stop bits.
  +*   work as chip default - CS8, no parity, !CSTOPB
  +*   other modes are not supported.
   */
   
  switch (ISSET(cflag, CSIZE)) {
  @@ -739,21 +718,8 @@ uchcom_set_line_control(struct uchcom_so
  break;
  }
   
  -   if (ISSET(cflag, PARENB)) {
  -   lcr1 |= UCHCOM_LCR1_PARENB;
  -   if (ISSET(cflag, PARODD))
  -   lcr2 |= UCHCOM_LCR2_PARODD;
  -   else
  -   lcr2 |= UCHCOM_LCR2_PAREVEN;
  -   }
  -
  -   err = uchcom_write_reg(sc, UCHCOM_REG_LCR1, lcr1, UCHCOM_REG_LCR2,
  -   lcr2);
  -   if (err) {
  -   printf(%s: cannot set LCR: %s\n,
  -  sc-sc_dev.dv_xname, usbd_errstr(err));
  -   return EIO;
  -   }
  +   if (ISSET(cflag, PARENB) || ISSET(cflag, CSTOPB))
  +   return EINVAL;
   
  return 0;
   }
  @@ -778,38 +744,12 @@ int
   uchcom_reset_chip(struct uchcom_softc *sc)
   {
  usbd_status err;
  -   uint8_t lcr1, lcr2, pre, div, mod;
  -   uint16_t val=0, idx=0;
  -
  -   err = uchcom_read_reg(sc, UCHCOM_REG_LCR1, lcr1, UCHCOM_REG_LCR2, 
  lcr2);
  -   if (err)
  -   goto failed;
  -
  -   err = uchcom_read_reg(sc, UCHCOM_REG_BPS_PRE, pre, UCHCOM_REG_BPS_DIV,
  -   div);
  -   if (err)
  -   goto failed;
  -
  -   err = uchcom_read_reg(sc, UCHCOM_REG_BPS_MOD, mod, UCHCOM_REG_BPS_PAD,
  -   NULL);
  -   if (err)
  -   goto failed;
  -
  -   val |= (uint16_t)(lcr10xF0)  8;
  -   val |= 0x01;
  -   val |= (uint16_t)(lcr20x0F)  8;
  -   val |= 0x02;
  -   idx |= pre  0x07;
  -   val |= 0x04;
  -   idx |= (uint16_t)div  8;
  -   val |= 0x08;
  -   idx |= mod  0xF8;
  -   val |= 0x10;
   
  -   DPRINTF((%s: reset v=0x%04X, i=0x%04X\n,
  -sc-sc_dev.dv_xname, val, idx));
  +   DPRINTF((%s: reset\n, sc-sc_dev.dv_xname));
   
  -   err = 

Re: uchcom(4) did not work

2014-05-14 Thread Mark Kettenis
 Date: Wed, 14 May 2014 11:04:56 +0200
 From: Martin Pieuchot mpieuc...@nolizard.org
 
 On 13/05/14(Tue) 21:24, Mike Larkin wrote:
  On Wed, May 14, 2014 at 11:02:49AM +0900, SASANO Takayoshi wrote:
   Hi,
   
   Simply magic values are rewrited with #define.
   If these values need to be disassembled, please take a while...
   
  
  I think we need to understand what those values mean. When I mentioned
  #defines, I meant something like:
  
  #define UCHCOM_SOME_FLAG 0x1234
  #define UCHCOM_SOME_OTHER_FLAG 0x5678
  ...
  ...
  #define UCHCOM_RESET_VALUE (UCHCOM_SOME_FLAG | UCHCOM_SOME_OTHER_FLAG)
  
  That way we know what the values do. If the value we're setting is not
  a flag, we should understand what 0x501F and 0xD90A actually mean.
 
 Unfortunately a lot of USB drivers are written without spec. and some
 values are taken by analysing the traffic generated by drivers on other
 OS.
 
  If we don't do it this way, the next person to come along and try to
  work on this code won't have any idea what to do.
 
 I also really don't like magic values, but I don't see how we could do
 it differently.  Maybe somebody can send an email to the author of the
 linux driver and ask him what these values are.  But I'd bet he doesn't
 no neither.

And really, the old code had magic numbers as well.

   Index: uchcom.c
   ===
   RCS file: /cvs/src/sys/dev/usb/uchcom.c,v
   retrieving revision 1.19
   diff -u -p -r1.19 uchcom.c
   --- uchcom.c  15 Nov 2013 10:17:39 -  1.19
   +++ uchcom.c  14 May 2014 01:43:34 -
   @@ -91,18 +91,14 @@ int   uchcomdebug = 0;
#define UCHCOM_BRK1_MASK 0x01
#define UCHCOM_BRK2_MASK 0x40

   -#define UCHCOM_LCR1_MASK 0xAF
   -#define UCHCOM_LCR2_MASK 0x07
   -#define UCHCOM_LCR1_PARENB   0x80
   -#define UCHCOM_LCR2_PAREVEN  0x07
   -#define UCHCOM_LCR2_PARODD   0x06
   -#define UCHCOM_LCR2_PARMARK  0x05
   -#define UCHCOM_LCR2_PARSPACE 0x04
   -
#define UCHCOM_INTR_STAT10x02
#define UCHCOM_INTR_STAT20x03
#define UCHCOM_INTR_LEAST4

   +/* these values come from Linux (drivers/usb/serial/ch341.c) */
   +#define UCHCOM_RESET_VALUE   0x501F  /* XXX default line mode? */
   +#define UCHCOM_RESET_INDEX   0xD90A  /* XXX default baud rate? */
   +
#define UCHCOMIBUFSIZE 256
#define UCHCOMOBUFSIZE 256

   @@ -707,27 +703,10 @@ uchcom_set_dte_rate(struct uchcom_softc 
int
uchcom_set_line_control(struct uchcom_softc *sc, tcflag_t cflag)
{
   - usbd_status err;
   - uint8_t lcr1 = 0, lcr2 = 0;
   -
   - err = uchcom_read_reg(sc, UCHCOM_REG_LCR1, lcr1, UCHCOM_REG_LCR2,
   - lcr2);
   - if (err) {
   - printf(%s: cannot get LCR: %s\n,
   -sc-sc_dev.dv_xname, usbd_errstr(err));
   - return EIO;
   - }
   -
   - lcr1 = ~UCHCOM_LCR1_MASK;
   - lcr2 = ~UCHCOM_LCR2_MASK;
   -
 /*
  * XXX: it is difficult to handle the line control appropriately:
   -  *   - CS8, !CSTOPB and any parity mode seems ok, but
   -  *   - the chip doesn't have the function to calculate parity
   -  * in !CS8 mode.
   -  *   - it is unclear that the chip supports CS5,6 mode.
   -  *   - it is unclear how to handle stop bits.
   +  *   work as chip default - CS8, no parity, !CSTOPB
   +  *   other modes are not supported.
  */

 switch (ISSET(cflag, CSIZE)) {
   @@ -739,21 +718,8 @@ uchcom_set_line_control(struct uchcom_so
 break;
 }

   - if (ISSET(cflag, PARENB)) {
   - lcr1 |= UCHCOM_LCR1_PARENB;
   - if (ISSET(cflag, PARODD))
   - lcr2 |= UCHCOM_LCR2_PARODD;
   - else
   - lcr2 |= UCHCOM_LCR2_PAREVEN;
   - }
   -
   - err = uchcom_write_reg(sc, UCHCOM_REG_LCR1, lcr1, UCHCOM_REG_LCR2,
   - lcr2);
   - if (err) {
   - printf(%s: cannot set LCR: %s\n,
   -sc-sc_dev.dv_xname, usbd_errstr(err));
   - return EIO;
   - }
   + if (ISSET(cflag, PARENB) || ISSET(cflag, CSTOPB))
   + return EINVAL;

 return 0;
}
   @@ -778,38 +744,12 @@ int
uchcom_reset_chip(struct uchcom_softc *sc)
{
 usbd_status err;
   - uint8_t lcr1, lcr2, pre, div, mod;
   - uint16_t val=0, idx=0;
   -
   - err = uchcom_read_reg(sc, UCHCOM_REG_LCR1, lcr1, UCHCOM_REG_LCR2, 
   lcr2);
   - if (err)
   - goto failed;
   -
   - err = uchcom_read_reg(sc, UCHCOM_REG_BPS_PRE, pre, UCHCOM_REG_BPS_DIV,
   - div);
   - if (err)
   - goto failed;
   -
   - err = uchcom_read_reg(sc, UCHCOM_REG_BPS_MOD, mod, UCHCOM_REG_BPS_PAD,
   - NULL);
   - if (err)
   - goto failed;
   -
   - val |= (uint16_t)(lcr10xF0)  8;
   - val |= 0x01;
   - val |= (uint16_t)(lcr20x0F)  8;
   - val |= 0x02;
   - idx |= pre  0x07;
   - val |= 0x04;
   - idx |= (uint16_t)div  8;
   - val |= 0x08;
   - idx |= mod  0xF8;
   - val |= 0x10;

   - DPRINTF((%s: reset v=0x%04X, i=0x%04X\n,
 

Re: Big routing table diff

2014-05-14 Thread Martin Pieuchot
On 13/05/14(Tue) 19:25, Claudio Jeker wrote:
 The last three hackathons I got sucked into one ugly dark corner of the
 network stack. Our radix tree implementation one one particular bug in it
 that caused bgpd and ospfd to freak out on semi regular basis.

Do you know why they would freak out?

 After a fair amount of versions tested by myself and benno@ I think it is
 time to send this to a larger audience. This affacts all our radix
 consumers (inclusive NFS and pf tables) so better test now then suffer
 later.

I'm putting it on my machines right now.

 I would be happy if people could throw this code on some of their (test)
 systems.  In general there should be no visible difference to the current
 state.  The one case that is no fixed has todo with routing priorities and
 the link state tracking of routes.  In short this should fix the bgpd mpath
 route not found error caused by a link state flap.
 
 The diff is hard to read because I refactored and split the radix.c code
 into smaller, independent and hopefully more digestable bits.  It may be
 easier to read the end result for those brave ones who want to review the
 diff.  I added some extra comments to help me to keep track of what is
 going on but it is probably still very terse.

Indeed it's really hard to read and I hope you won't get hit by a bus
soon :)

 I would like to commit this rather soon so that it gets enough exposure
 before the next release.

I'd really appreciate if you can split this diff in various commits.  I
see that there's a lot of refactoring or documentation bits that could
easily be committed before.  I like the functions you introduce to
replace some label/gotos and they make the code easier to understand.
I'd bet they can be committed before too.

I understand that this is not fun and that this code is already scary as
it is, but if you are able to document the various steps you did to fix
this bug, I'm sure it will be helpful to anybody trying to understand
and/or contribute to this code.

 -- 
 :wq Claudio
 
 Index: net/radix.c
 ===
 RCS file: /cvs/src/sys/net/radix.c,v
 retrieving revision 1.39
 diff -u -p -r1.39 radix.c
 --- net/radix.c   22 Jan 2014 10:17:59 -  1.39
 +++ net/radix.c   9 May 2014 17:01:23 -
 @@ -71,6 +71,13 @@ struct radix_node *rn_newpair(void *, in
  
  static inline struct radix_node *rn_search(void *, struct radix_node *);
  struct radix_node *rn_search_m(void *, struct radix_node *, void *);
 +int rn_add_dupedkey(struct radix_node *, struct radix_node_head *,
 +struct radix_node [2], u_int8_t);
 +void rn_fixup_nodes(struct radix_node *);
 +static inline struct radix_node *rn_lift_node(struct radix_node *);
 +void rn_add_radix_mask(struct radix_node *, int);
 +int rn_del_radix_mask(struct radix_node *);
 +static inline void rn_swap_nodes(struct radix_node *, struct radix_node *);
  
  /*
   * The data structure for the keys is a radix tree with one way
 @@ -167,6 +174,7 @@ rn_refines(void *m_arg, void *n_arg)
   return (!masks_are_equal);
  }
  
 +/* return a perfect match if m_arg is set, else do a regular rn_match */
  struct radix_node *
  rn_lookup(void *v_arg, void *m_arg, struct radix_node_head *head)
  {
 @@ -201,10 +209,15 @@ rn_satisfies_leaf(char *trial, struct ra
   cp3 = rn_ones;
   else
   length = min(length, *(u_char *)cp3);
 - cplim = cp + length; cp3 += skip; cp2 += skip;
 - for (cp += skip; cp  cplim; cp++, cp2++, cp3++)
 + cplim = cp + length;
 + cp += skip;
 + cp2 += skip;
 + cp3 += skip;
 + while (cp  cplim) {
   if ((*cp ^ *cp2)  *cp3)
   return 0;
 + cp++, cp2++, cp3++;
 + }
   return 1;
  }
  
 @@ -395,7 +408,7 @@ rn_addmask(void *n_arg, int search, int 
   if (skip == 0)
   skip = 1;
   if (mlen = skip)
 - return (mask_rnhead-rnh_nodes);
 + return (mask_rnhead-rnh_nodes);/* rn_zero root node */
   if (skip  1)
   memcpy(addmask_key + 1, rn_ones + 1, skip - 1);
   if ((m0 = mlen)  skip)
 @@ -432,7 +445,7 @@ rn_addmask(void *n_arg, int search, int 
   return (tm);
   }
   /*
 -  * Calculate index of mask, and check for normalcy.
 +  * Calculate index of mask, and check for normalicy.
*/
   cplim = netmask + mlen;
   isnormal = 1;
 @@ -495,388 +508,556 @@ rn_new_radix_mask(struct radix_node *tt,
   return m;
  }
  
 -struct radix_node *
 -rn_addroute(void *v_arg, void *n_arg, struct radix_node_head *head,
 -struct radix_node treenodes[2], u_int8_t prio)
 +/*
 + * Find the point where the rn_mklist needs to be changed.
 + */
 +static inline struct radix_node *
 +rn_lift_node(struct radix_node *t)
  {
 - caddr_t v = v_arg;
 - caddr_t netmask = n_arg;
 - struct radix_node *top = head-rnh_treetop;
 - struct radix_node *t, *tt, *tm = NULL, *x;
 

Re: uchcom(4) did not work

2014-05-14 Thread Creamy
/*
 * XXX: it is difficult to handle the line control 
appropriately:
-*   - CS8, !CSTOPB and any parity mode seems ok, but
-*   - the chip doesn't have the function to calculate parity
-* in !CS8 mode.
-*   - it is unclear that the chip supports CS5,6 mode.
-*   - it is unclear how to handle stop bits.
+*   work as chip default - CS8, no parity, !CSTOPB
+*   other modes are not supported.
 */

Given that 8N1 is supported, can we not support other modes entirely in 
software?
7E1, and 7O1 don't need explicit hardware support.

If you need 7N1, 7M1 will simulate 7N2, which the majority of hardware will work
with, (an extra stop bit).

If 8N1 works, and there is confusion over support for other modes, just bit bang
in 8N1 mode.

-- 
Creamy! 3



cleanup wsmoused 1

2014-05-14 Thread Alexandr Shadchin
wsmoused() called if cmd == WSDISPLAYIO_WSMOUSED only,
don't need check cmd twice

Comment ? OK ?

note: diff cteated with 'cvs diff -w' for simplicity

-- 
Alexandr Shadchin

Index: wsdisplay.c
===
RCS file: /cvs/src/sys/dev/wscons/wsdisplay.c,v
retrieving revision 1.117
diff -u -p -w -r1.117 wsdisplay.c
--- wsdisplay.c 4 Nov 2013 05:45:04 -   1.117
+++ wsdisplay.c 14 May 2014 12:34:06 -
@@ -1319,7 +1319,7 @@ wsdisplay_cfg_ioctl(struct wsdisplay_sof
switch (cmd) {
 #ifdef HAVE_WSMOUSED_SUPPORT
case WSDISPLAYIO_WSMOUSED:
-   error = wsmoused(sc, cmd, data, flag, p);
+   error = wsmoused(sc, data, flag, p);
return (error);
 #endif
case WSDISPLAYIO_ADDSCREEN:
@@ -2381,12 +2381,10 @@ wsdisplay_burner(void *v)
  * Main function, called from wsdisplay_cfg_ioctl.
  */
 int
-wsmoused(struct wsdisplay_softc *sc, u_long cmd, caddr_t data,
-int flag, struct proc *p)
+wsmoused(struct wsdisplay_softc *sc, caddr_t data, int flag, struct proc *p)
 {
struct wscons_event mouse_event = *(struct wscons_event *)data;
 
-   if (cmd == WSDISPLAYIO_WSMOUSED) {
if (IS_MOTION_EVENT(mouse_event.type)) {
if (sc-sc_focus != NULL)
motion_event(sc-sc_focus, mouse_event.type,
@@ -2410,7 +2408,6 @@ wsmoused(struct wsdisplay_softc *sc, u_l
if (IS_CTRL_EVENT(mouse_event.type)) {
return ctrl_event(sc, mouse_event.type,
mouse_event.value, p);
-   }
}
return -1;
 }
Index: wsmoused.h
===
RCS file: /cvs/src/sys/dev/wscons/wsmoused.h,v
retrieving revision 1.8
diff -u -p -w -r1.8 wsmoused.h
--- wsmoused.h  31 May 2009 17:02:20 -  1.8
+++ wsmoused.h  14 May 2014 12:34:06 -
@@ -35,7 +35,7 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-intwsmoused(struct wsdisplay_softc *, u_long, caddr_t, int, struct proc *);
+intwsmoused(struct wsdisplay_softc *, caddr_t, int, struct proc *);
 
 void   motion_event(struct wsscreen *, u_int, int);
 void   button_event(struct wsscreen *, int, int);



cleanup wsmoused 2

2014-05-14 Thread Alexandr Shadchin
Remove unused variables. OK ?

-- 
Alexandr Shadchin

Index: wsmoused.c
===
RCS file: /cvs/src/usr.sbin/wsmoused/wsmoused.c,v
retrieving revision 1.29
diff -u -p -r1.29 wsmoused.c
--- wsmoused.c  24 Nov 2013 01:06:19 -  1.29
+++ wsmoused.c  14 May 2014 12:41:59 -
@@ -93,8 +93,6 @@ mouse_t mouse = {
.old_baudrate = 1200,
.rate = MOUSE_RATE_UNKNOWN,
.resolution = MOUSE_RES_UNKNOWN,
-   .zmap = 0,
-   .wmode = 0,
.mfd = -1,
.clickthreshold = 500,  /* 0.5 sec */
 };
Index: wsmoused.h
===
RCS file: /cvs/src/usr.sbin/wsmoused/wsmoused.h,v
retrieving revision 1.8
diff -u -p -r1.8 wsmoused.h
--- wsmoused.h  21 Jun 2009 16:13:18 -  1.8
+++ wsmoused.h  14 May 2014 12:41:59 -
@@ -102,8 +102,6 @@ typedef struct mouse_s {
int old_baudrate;
unsigned char rate; /* report rate */
unsigned int resolution;/* MOUSE_RES_XXX or a positive number */
-   int zmap;   /* MOUSE_{X|Y}AXIS or a button number */
-   int wmode;  /* wheel mode button number */
int mfd;/* mouse file descriptor */
int cfd;/* console file descriptor */
long clickthreshold;/* double click speed in msec */



cleanup wsmoused 3

2014-05-14 Thread Alexandr Shadchin
Remove unused code. OK ?

-- 
Alexandr Shadchin

Index: mouse_protocols.c
===
RCS file: /cvs/src/usr.sbin/wsmoused/mouse_protocols.c,v
retrieving revision 1.13
diff -u -p -r1.13 mouse_protocols.c
--- mouse_protocols.c   5 Jun 2009 03:37:10 -   1.13
+++ mouse_protocols.c   14 May 2014 13:34:14 -
@@ -194,24 +194,6 @@ SetMouseSpeed(int old, int new, unsigned
mouse.portname, strerror(errno));
return;
}
-   /* this will query the initial baudrate only once */
-   if (mouse.old_baudrate  0) {
-   switch (cfgetispeed(tty)) {
-   case B9600:
-   mouse.old_baudrate = 9600;
-   break;
-   case B4800:
-   mouse.old_baudrate = 4800;
-   break;
-   case B2400:
-   mouse.old_baudrate = 2400;
-   break;
-   case B1200:
-   default:
-   mouse.old_baudrate = 1200;
-   break;
-   }
-   }
tty.c_iflag = IGNBRK | IGNPAR;
tty.c_oflag = 0;
tty.c_lflag = 0;
Index: wsmoused.c
===
RCS file: /cvs/src/usr.sbin/wsmoused/wsmoused.c,v
retrieving revision 1.29
diff -u -p -r1.29 wsmoused.c
--- wsmoused.c  24 Nov 2013 01:06:19 -  1.29
+++ wsmoused.c  14 May 2014 13:34:14 -
@@ -90,7 +90,6 @@ mouse_t mouse = {
.ttyname = NULL,
.proto = P_UNKNOWN,
.baudrate = 1200,
-   .old_baudrate = 1200,
.rate = MOUSE_RATE_UNKNOWN,
.resolution = MOUSE_RES_UNKNOWN,
.zmap = 0,
Index: wsmoused.h
===
RCS file: /cvs/src/usr.sbin/wsmoused/wsmoused.h,v
retrieving revision 1.8
diff -u -p -r1.8 wsmoused.h
--- wsmoused.h  21 Jun 2009 16:13:18 -  1.8
+++ wsmoused.h  14 May 2014 13:34:14 -
@@ -99,7 +99,6 @@ typedef struct mouse_s {
char *ttyname;  /* wsdisplay control tty device */
int proto;  /* MOUSE_PROTO_XXX */
int baudrate;
-   int old_baudrate;
unsigned char rate; /* report rate */
unsigned int resolution;/* MOUSE_RES_XXX or a positive number */
int zmap;   /* MOUSE_{X|Y}AXIS or a button number */



Re: Big routing table diff

2014-05-14 Thread Claudio Jeker
On Wed, May 14, 2014 at 12:11:30PM +0200, Martin Pieuchot wrote:
 On 13/05/14(Tue) 19:25, Claudio Jeker wrote:
  The last three hackathons I got sucked into one ugly dark corner of the
  network stack. Our radix tree implementation one one particular bug in it
  that caused bgpd and ospfd to freak out on semi regular basis.
 
 Do you know why they would freak out?

Because it was possible to trigger a prio inversion on routes which then
resulted in failing RTM_DELETEs. See also rttest8 and rttest9 of the route
regress tests.
 
  After a fair amount of versions tested by myself and benno@ I think it is
  time to send this to a larger audience. This affacts all our radix
  consumers (inclusive NFS and pf tables) so better test now then suffer
  later.
 
 I'm putting it on my machines right now.
 
  I would be happy if people could throw this code on some of their (test)
  systems.  In general there should be no visible difference to the current
  state.  The one case that is no fixed has todo with routing priorities and
  the link state tracking of routes.  In short this should fix the bgpd mpath
  route not found error caused by a link state flap.
  
  The diff is hard to read because I refactored and split the radix.c code
  into smaller, independent and hopefully more digestable bits.  It may be
  easier to read the end result for those brave ones who want to review the
  diff.  I added some extra comments to help me to keep track of what is
  going on but it is probably still very terse.
 
 Indeed it's really hard to read and I hope you won't get hit by a bus
 soon :)

Just to be clear the code is complex because the patricia tree is a smart
but complex data structure. I already did a fair amount of easy
refactorings that helped a lot to make it less arcane code from the 80s.

  I would like to commit this rather soon so that it gets enough exposure
  before the next release.
 
 I'd really appreciate if you can split this diff in various commits.  I
 see that there's a lot of refactoring or documentation bits that could
 easily be committed before.  I like the functions you introduce to
 replace some label/gotos and they make the code easier to understand.
 I'd bet they can be committed before too.

It is possible to do this but I would like to get this tested and not each
and every step. The problem is that some of the changes depend on each
other so you can not untangle them easily without causing a breakage
in between. So I guess there will be some fairly large parts that can not
be commited independent from the rest.
 
 I understand that this is not fun and that this code is already scary as
 it is, but if you are able to document the various steps you did to fix
 this bug, I'm sure it will be helpful to anybody trying to understand
 and/or contribute to this code.

The main fix is the change of rn_mpath_next() it is use throughout the
tree. Also I try to have less multipath code exposed in the upper layers.
 
  -- 
  :wq Claudio
  
  Index: net/radix.c
  ===
  RCS file: /cvs/src/sys/net/radix.c,v
  retrieving revision 1.39
  diff -u -p -r1.39 radix.c
  --- net/radix.c 22 Jan 2014 10:17:59 -  1.39
  +++ net/radix.c 9 May 2014 17:01:23 -
  @@ -71,6 +71,13 @@ struct radix_node *rn_newpair(void *, in
   
   static inline struct radix_node *rn_search(void *, struct radix_node *);
   struct radix_node *rn_search_m(void *, struct radix_node *, void *);
  +int rn_add_dupedkey(struct radix_node *, struct radix_node_head *,
  +struct radix_node [2], u_int8_t);
  +void rn_fixup_nodes(struct radix_node *);
  +static inline struct radix_node *rn_lift_node(struct radix_node *);
  +void rn_add_radix_mask(struct radix_node *, int);
  +int rn_del_radix_mask(struct radix_node *);
  +static inline void rn_swap_nodes(struct radix_node *, struct radix_node *);
   
   /*
* The data structure for the keys is a radix tree with one way
  @@ -167,6 +174,7 @@ rn_refines(void *m_arg, void *n_arg)
  return (!masks_are_equal);
   }
   
  +/* return a perfect match if m_arg is set, else do a regular rn_match */
   struct radix_node *
   rn_lookup(void *v_arg, void *m_arg, struct radix_node_head *head)
   {
  @@ -201,10 +209,15 @@ rn_satisfies_leaf(char *trial, struct ra
  cp3 = rn_ones;
  else
  length = min(length, *(u_char *)cp3);
  -   cplim = cp + length; cp3 += skip; cp2 += skip;
  -   for (cp += skip; cp  cplim; cp++, cp2++, cp3++)
  +   cplim = cp + length;
  +   cp += skip;
  +   cp2 += skip;
  +   cp3 += skip;
  +   while (cp  cplim) {
  if ((*cp ^ *cp2)  *cp3)
  return 0;
  +   cp++, cp2++, cp3++;
  +   }
  return 1;
   }
   
  @@ -395,7 +408,7 @@ rn_addmask(void *n_arg, int search, int 
  if (skip == 0)
  skip = 1;
  if (mlen = skip)
  -   return (mask_rnhead-rnh_nodes);
  +   return (mask_rnhead-rnh_nodes);/* 

cleanup wsmoused 4

2014-05-14 Thread Alexandr Shadchin
Remove mouse.baudrate, it is always 1200. OK ?

-- 
Alexandr Shadchin

diff --git mouse_protocols.c mouse_protocols.c
index b250f65..9ee1039 100644
--- mouse_protocols.c
+++ mouse_protocols.c
@@ -635,16 +635,15 @@ mouse_init(void)
 * The baud rate selection command must be sent at the current
 * baud rate; try all likely settings
 */
-   SetMouseSpeed(9600, mouse.baudrate, mousecflags[mouse.proto]);
-   SetMouseSpeed(4800, mouse.baudrate, mousecflags[mouse.proto]);
-   SetMouseSpeed(2400, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(9600, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(4800, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(2400, 1200, mousecflags[mouse.proto]);
 #if 0
-   SetMouseSpeed(1200, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
 #endif
/* select MM series data format */
write(mouse.mfd, S, 1);
-   SetMouseSpeed(mouse.baudrate, mouse.baudrate,
- mousecflags[P_MM]);
+   SetMouseSpeed(1200, 1200, mousecflags[P_MM]);
/* select report rate/frequency */
if (mouse.rate = 0)
write(mouse.mfd, O, 1);
@@ -668,11 +667,11 @@ mouse_init(void)
/* The command must always be sent at 1200 baud */
SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
write(mouse.mfd, *X, 2);
-   SetMouseSpeed(1200, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
break;
 
case P_MMHIT:
-   SetMouseSpeed(1200, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
 
/*
 * Initialize Hitachi PUMA Plus - Model 1212E to desired 
settings.
@@ -722,7 +721,7 @@ mouse_init(void)
break;
 
case P_THINKING:
-   SetMouseSpeed(1200, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
/* the PnP ID string may be sent again, discard it */
usleep(20);
i = FREAD;
@@ -741,7 +740,7 @@ mouse_init(void)
break;
 
case P_MSC:
-   SetMouseSpeed(1200, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
 #if 0
if (mouse.flags  ClearDTR) {
i = TIOCM_DTR;
@@ -755,7 +754,7 @@ mouse_init(void)
break;
 
default:
-   SetMouseSpeed(1200, mouse.baudrate, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
break;
}
 }
diff --git wsmoused.c wsmoused.c
index aea621a..7875ce8 100644
--- wsmoused.c
+++ wsmoused.c
@@ -89,7 +89,6 @@ mouse_t mouse = {
.portname = NULL,
.ttyname = NULL,
.proto = P_UNKNOWN,
-   .baudrate = 1200,
.rate = MOUSE_RATE_UNKNOWN,
.resolution = MOUSE_RES_UNKNOWN,
.zmap = 0,
diff --git wsmoused.h wsmoused.h
index 2d62e2a..6cae5a2 100644
--- wsmoused.h
+++ wsmoused.h
@@ -98,7 +98,6 @@ typedef struct mouse_s {
char *portname; /* mouse device */
char *ttyname;  /* wsdisplay control tty device */
int proto;  /* MOUSE_PROTO_XXX */
-   int baudrate;
unsigned char rate; /* report rate */
unsigned int resolution;/* MOUSE_RES_XXX or a positive number */
int zmap;   /* MOUSE_{X|Y}AXIS or a button number */



cleanup wsmoused 5

2014-05-14 Thread Alexandr Shadchin
Remove second arg in SetMouseSpeed(), it is always 1200. OK ?

-- 
Alexandr Shadchin

diff --git mouse_protocols.c mouse_protocols.c
index 9ee1039..a41d618 100644
--- mouse_protocols.c
+++ mouse_protocols.c
@@ -184,7 +184,7 @@ mouse_name(int type)
 }
 
 void
-SetMouseSpeed(int old, int new, unsigned int cflag)
+SetMouseSpeed(int old, unsigned int cflag)
 {
struct termios tty;
char*c;
@@ -223,28 +223,9 @@ SetMouseSpeed(int old, int new, unsigned int cflag)
if (tcsetattr(mouse.mfd, TCSADRAIN, tty)  0)
logerr(1, unable to get mouse status. Exiting...\n);
 
-   switch (new) {
-   case 9600:
-   c = *q;
-   cfsetispeed(tty, B9600);
-   cfsetospeed(tty, B9600);
-   break;
-   case 4800:
-   c = *p;
-   cfsetispeed(tty, B4800);
-   cfsetospeed(tty, B4800);
-   break;
-   case 2400:
-   c = *o;
-   cfsetispeed(tty, B2400);
-   cfsetospeed(tty, B2400);
-   break;
-   case 1200:
-   default:
-   c = *n;
-   cfsetispeed(tty, B1200);
-   cfsetospeed(tty, B1200);
-   }
+   c = *n;
+   cfsetispeed(tty, B1200);
+   cfsetospeed(tty, B1200);
 
if (mouse.proto == P_LOGIMAN || mouse.proto == P_LOGI) {
if (write(mouse.mfd, c, 2) != 2)
@@ -316,7 +297,7 @@ pnpgets(int mouse_fd, char *buf)
goto disconnect_idle;
 
/* port setup, 1st phase (2.1.3) */
-   SetMouseSpeed(1200, 1200, (CS7 | CREAD | CLOCAL | HUPCL));
+   SetMouseSpeed(1200, (CS7 | CREAD | CLOCAL | HUPCL));
i = TIOCM_DTR | TIOCM_RTS;  /* DTR = 0, RTS = 0 */
ioctl(mouse_fd, TIOCMBIC, i);
usleep(20);
@@ -350,7 +331,7 @@ pnpgets(int mouse_fd, char *buf)
/*
 * This is a simplified procedure; it simply toggles RTS.
 */
-   SetMouseSpeed(1200, 1200, (CS7 | CREAD | CLOCAL | HUPCL));
+   SetMouseSpeed(1200, (CS7 | CREAD | CLOCAL | HUPCL));
 
ioctl(mouse_fd, TIOCMGET, i);
i |= TIOCM_DTR; /* DTR = 1 */
@@ -635,15 +616,15 @@ mouse_init(void)
 * The baud rate selection command must be sent at the current
 * baud rate; try all likely settings
 */
-   SetMouseSpeed(9600, 1200, mousecflags[mouse.proto]);
-   SetMouseSpeed(4800, 1200, mousecflags[mouse.proto]);
-   SetMouseSpeed(2400, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(9600, mousecflags[mouse.proto]);
+   SetMouseSpeed(4800, mousecflags[mouse.proto]);
+   SetMouseSpeed(2400, mousecflags[mouse.proto]);
 #if 0
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
 #endif
/* select MM series data format */
write(mouse.mfd, S, 1);
-   SetMouseSpeed(1200, 1200, mousecflags[P_MM]);
+   SetMouseSpeed(1200, mousecflags[P_MM]);
/* select report rate/frequency */
if (mouse.rate = 0)
write(mouse.mfd, O, 1);
@@ -665,13 +646,13 @@ mouse_init(void)
 
case P_LOGIMAN:
/* The command must always be sent at 1200 baud */
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
write(mouse.mfd, *X, 2);
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
break;
 
case P_MMHIT:
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
 
/*
 * Initialize Hitachi PUMA Plus - Model 1212E to desired 
settings.
@@ -721,7 +702,7 @@ mouse_init(void)
break;
 
case P_THINKING:
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
/* the PnP ID string may be sent again, discard it */
usleep(20);
i = FREAD;
@@ -740,7 +721,7 @@ mouse_init(void)
break;
 
case P_MSC:
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
 #if 0
if (mouse.flags  ClearDTR) {
i = TIOCM_DTR;
@@ -754,7 +735,7 @@ mouse_init(void)
break;
 
default:
-   SetMouseSpeed(1200, 1200, mousecflags[mouse.proto]);
+   SetMouseSpeed(1200, mousecflags[mouse.proto]);
break;
}
 }



[PATCH] login_yubikey.c with optional PIN

2014-05-14 Thread Jeff Clement
Hi everyone,

Sorry for the repeat post.  I originally sent this 2014-05-05 and I
think it slipped under the radar.  Here is an updated version with a
properly produced diff and without the attachment.

-

I'd like to propose an addition to login_yubikey.c to support an
optional PIN.  

I think that using the Yubikey for authentication is worthwhile. The
current implementation of login_yubikey.c, however, relies entirely on
the one-time password.  I think the system would be stronger combining
the Yubikey with an additional PIN so that a compromise of the physical
security of the token doesn't compromise the associated account.

My work is loosely based off of Remi Locherer's suggested patch (link
below).  Where it differs is that I'd like to add an optional additional
PIN to the authentication rather than use an existing credential, such
as the system password.  My thinking is, if you are using the Yubikey
token already, the PIN can be a fairly low strength password. 
The system password shouldn't be set to something simple.  This allows
for relaxed rules the the Yubikey PIN without affecting the system
password policy as a whole.

http://comments.gmane.org/gmane.os.openbsd.tech/34693

I propose adding a new /var/db/yubikey/$user.pin file that contains an
encrypted additional PIN (password).  If present, this PIN must precede
the Yubikey one-time password when authenticating.  

The password in $user.pin is encrypted in a manner similar to those in
/etc/master.passwd.  Obviously, in a multi-user system some tool would
need to be devised to maintain these PIN/passwords.  For my purposes,
the following works

---
# encrypt  /var/db/yubikey/$user.pin
password_goes_here
---

Both the Yubikey and PIN are verified on every attempt.  The PIN is
hashed 
and the hashed version is what is verified so that it should not
introduce any
timing attacks.

This has a couple nice side-effects:

1) Physical compromise of the Yubikey token does not immediately yield
access to the associated account

2) Compromise (read) of the contents of /var/db/yubikey does not
immediately allow an attacker to access those accounts although in this
case, because the attacker now has access to the yubikey secrets, they
can brute force the PIN for more efficiently than if the had the
physical
token.  But it's still an improvement.

This change is non-breaking in that, if the $user.pin file is not
present, login_yubikey works as before.

Thoughts?

Thank you,
Jeff

-

Index: libexec/login_yubikey/login_yubikey.8
===
RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.8,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 login_yubikey.8
--- libexec/login_yubikey/login_yubikey.8  14 Aug 2013 08:39:31 - 
1.8
+++ libexec/login_yubikey/login_yubikey.8  7 May 2014 13:13:50 -
@@ -85,8 +85,10 @@ will read the user's UID (12 hex digits)
 .Em user.uid ,
 the user's key (32 hex digits) from
 .Em user.key ,
-and the user's last-use counter from
-.Em user.ctr
+the user's last-use counter from
+.Em user.ctr ,
+and the user's PIN (optional) from
+.Em user.pin
 in the
 .Em /var/db/yubikey
 directory.
@@ -99,6 +101,14 @@ If
 does not have a last-use counter, a value of zero is used and
 any counter is accepted during the first login.
 .Pp
+If
+.Ar user
+does have a PIN file, the PIN must be provided before the one-time
password
+and the PIN will be verified (using 
+.Xr crypt 8 ) against the contents of the PIN
+file.  If the PIN file is not present, the user must provide only the
one-time
+password.
+.Pp
 The one-time password provided by the user is decrypted using the
 user's key.
 After the decryption, the checksum embedded in the one-time password
@@ -124,4 +134,5 @@ Directory containing user entries for Yu
 .El
 .Sh SEE ALSO
 .Xr login 1 ,
-.Xr login.conf 5
+.Xr login.conf 5 ,
+.Xr crypt 8
Index: libexec/login_yubikey/login_yubikey.c
===
RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.c,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 login_yubikey.c
--- libexec/login_yubikey/login_yubikey.c  27 Nov 2013 21:25:25 - 
1.8
+++ libexec/login_yubikey/login_yubikey.c  7 May 2014 13:13:50 -
@@ -44,6 +44,7 @@
 #include syslog.h
 #include unistd.h
 #include errno.h
+#include util.h
 
 #include yubikey.h
 
@@ -54,15 +55,18 @@
 #defineAUTH_OK 0
 #defineAUTH_FAILED -1
 
+#define YUBIKEY_LENGTH 44
+
 static const char *path = /var/db/yubikey;
 
 static int clean_string(const char *);
 static int yubikey_login(const char *, const char *);
+static int pin_login(const char *, const char *);
 
 int
 main(int argc, char *argv[])
 {
-  int ch, ret, mode = MODE_LOGIN;
+  int ch, ret, ret_pin, mode = MODE_LOGIN;
FILE *f = NULL;
char *username, *password = NULL;
char response[1024];
@@ -151,9 +155,33 @@ main(int argc, char 

recvmsg, fd passing and soreceive

2014-05-14 Thread Jérémie Courrèges-Anglas

Hi,

Initially, I just wanted to try the examples in CMSG_DATA(3).
They are a bit incomplete, so I had to guess a few things.

This is the result of my tests:

#include sys/types.h
#include sys/socket.h
#include sys/uio.h
#include sys/wait.h

#include err.h
#include errno.h
#include limits.h
#include stdio.h
#include stdlib.h
#include string.h
#include unistd.h

#define RECVFD_DO_CLOSE 0x1
#define RECVFD_PASS_IOVEC   0x2
#define RECVFD_DEBUG0x4
#define SENDFD_DEBUG0x8

int sendfd(int, u_int, u_int);
int recvfd(int, u_int, u_int);

/*
 * Usage: send-recv-fd [-bc] [-d debug] [-n iterations] [-t socktype]
 *
 * -b passes a dummy iovec structure to recvmsg (fixes -t stream -n $n)
 * -c makes the child process close the fd received via recvmsg
 * -r verbose recvmsg calls
 * -s verbose sendmsg calls
 * -n specifies how many copies of a fd should be passed from the parent
 *to the child
 * -t specifies the socket type.  can be stream (default), dgram, or seqtype
 */

int
main(int argc, char **argv)
{
int  c;
u_intflags = 0;
u_intcount = 5;
int  socktype = SOCK_STREAM;
const char  *errstr = NULL;
int  fds[2];

while ((c = getopt(argc, argv, bcn:rst:)) != -1)
switch (c) {
case 'b':
flags |= RECVFD_PASS_IOVEC;
break;
case 'c':
flags |= RECVFD_DO_CLOSE;
break;
case 'n':
count = strtonum(optarg, 1, INT_MAX, errstr);
if (errstr)
errx(1, option n: %s is %s, optarg, errstr);
break;
case 'r':
flags |= RECVFD_DEBUG;
break;
case 's':
flags |= SENDFD_DEBUG;
break;
case 't':
if (!strcmp(optarg, dgram))
socktype = SOCK_DGRAM;
else if (!strcmp(optarg, seqpacket))
socktype = SOCK_SEQPACKET;
else if (!strcmp(optarg, stream))
socktype = SOCK_STREAM;
else
errx(1, bad type %s, optarg);
break;
default:
errx(1, unknown option %c, c);
}

if (socketpair(AF_LOCAL, socktype, 0, fds) == -1)
err(1, socketpair);

switch (fork()) {
case -1:
err(1, fork);
/* NOTREACHED */
case 0:
sendfd(fds[0], count, flags);
break;
default:
recvfd(fds[1], count, flags);
wait(NULL);
break;
}
return 0;
}

int
sendfd(int fd, u_int count, u_int flags)
{
struct msghdrmsg;
struct cmsghdr  *cmsg;
union {
struct cmsghdr  hdr;
unsigned char   buf[CMSG_SPACE(sizeof(int))];
} cmsgbuf;
ssize_t  sent;
u_inti = 1;

while (i = count) {
memset(msg, 0, sizeof(msg));
msg.msg_control = cmsgbuf.buf;
msg.msg_controllen = sizeof(cmsgbuf.buf);

cmsg = CMSG_FIRSTHDR(msg);
cmsg-cmsg_len = CMSG_LEN(sizeof(int));
cmsg-cmsg_level = SOL_SOCKET;
cmsg-cmsg_type = SCM_RIGHTS;
*(int *)CMSG_DATA(cmsg) = 2;

if (flags  SENDFD_DEBUG)
fprintf(stderr, debug: iteration %d, calling 
sendmsg\n,
i);

sent = sendmsg(fd, msg, 0);

if (flags  SENDFD_DEBUG)
fprintf(stderr, debug: iteration %d, sendmsg sent %zd 
bytes\n,
i, sent);

if (sent == -1) {
if (errno == ENOBUFS)
usleep(2);
else
err(1, sendmsg);
} else {
i++;
}
}

return 0;
}

int
recvfd(int fd, u_int count, u_int flags)
{
struct msghdrmsg;
struct cmsghdr  *cmsg;
union {
struct cmsghdr  hdr;
unsigned char   buf[CMSG_SPACE(sizeof(int))];
} cmsgbuf;
chariov_buffer[1024];
struct ioveciov;
charfdstr[32];
ssize_t received;
int passed_fd;
u_int   i = 1;

while (i = count) {
memset(msg, 0, sizeof(msg));

Re: rtadvd prints error on receiving RA with Route Information

2014-05-14 Thread Jérémie Courrèges-Anglas

Redirecting this to tech@

Consus has confirmed this fixed his issue.  I plan to commit it this
week-end if I hear no objection.

j...@wxcvbn.org (Jérémie Courrèges-Anglas) writes:

 Thanks for the detailed report.

 This diff makes rtadvd aware of route info messages on the listening
 side, no functional change except that the error message goes away.
 We just ignore the message since in that case there's no good reason to
 try to validate the more specifics advertized by other routers.

 ok?

Index: rtadvd.c
===
RCS file: /cvs/src/usr.sbin/rtadvd/rtadvd.c,v
retrieving revision 1.45
diff -u -p -r1.45 rtadvd.c
--- rtadvd.c5 May 2013 14:25:52 -   1.45
+++ rtadvd.c6 May 2014 14:21:42 -
@@ -121,6 +121,7 @@ union nd_opts {
 #define NDOPT_FLAG_MTU (1  4)
 #define NDOPT_FLAG_RDNSS   (1  5)
 #define NDOPT_FLAG_DNSSL   (1  6)
+#define NDOPT_FLAG_ROUTE_INFO  (1  7)
 
 u_int32_t ndopt_flags[] = {
[ND_OPT_SOURCE_LINKADDR]= NDOPT_FLAG_SRCLINKADDR,
@@ -128,6 +129,7 @@ u_int32_t ndopt_flags[] = {
[ND_OPT_PREFIX_INFORMATION] = NDOPT_FLAG_PREFIXINFO,
[ND_OPT_REDIRECTED_HEADER]  = NDOPT_FLAG_RDHDR,
[ND_OPT_MTU]= NDOPT_FLAG_MTU,
+   [ND_OPT_ROUTE_INFO] = NDOPT_FLAG_ROUTE_INFO,
[ND_OPT_RDNSS]  = NDOPT_FLAG_RDNSS,
[ND_OPT_DNSSL]  = NDOPT_FLAG_DNSSL,
 };
@@ -809,7 +811,8 @@ ra_input(int len, struct nd_router_adver
if (nd6_options((struct nd_opt_hdr *)(ra + 1),
len - sizeof(struct nd_router_advert),
ndopts, NDOPT_FLAG_SRCLINKADDR | NDOPT_FLAG_PREFIXINFO
-   | NDOPT_FLAG_MTU | NDOPT_FLAG_RDNSS | 
NDOPT_FLAG_DNSSL)) {
+   | NDOPT_FLAG_MTU | NDOPT_FLAG_ROUTE_INFO
+   | NDOPT_FLAG_RDNSS | NDOPT_FLAG_DNSSL)) {
log_warnx(ND option check failed for an RA from %s on %s,
inet_ntop(AF_INET6, from-sin6_addr,
ntopbuf, INET6_ADDRSTRLEN),
@@ -1109,6 +1112,7 @@ nd6_options(struct nd_opt_hdr *hdr, int 
}
 
if (hdr-nd_opt_type  ND_OPT_MTU 
+   hdr-nd_opt_type != ND_OPT_ROUTE_INFO 
hdr-nd_opt_type != ND_OPT_RDNSS 
hdr-nd_opt_type != ND_OPT_DNSSL)
{
@@ -1142,6 +1146,7 @@ nd6_options(struct nd_opt_hdr *hdr, int 
case ND_OPT_SOURCE_LINKADDR:
case ND_OPT_TARGET_LINKADDR:
case ND_OPT_REDIRECTED_HEADER:
+   case ND_OPT_ROUTE_INFO:
case ND_OPT_RDNSS:
case ND_OPT_DNSSL:
break;  /* we don't care about these options */

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



Re: recvmsg, fd passing and soreceive

2014-05-14 Thread Matthew Dempsky
On Wed, May 14, 2014 at 4:58 AM, Jérémie Courrèges-Anglas
j...@wxcvbn.org wrote:
 if (cmsg-cmsg_len == CMSG_LEN(sizeof(int)) 
 cmsg-cmsg_level == SOL_SOCKET 
 cmsg-cmsg_type == SCM_RIGHTS) {
 passed_fd = *(int *)CMSG_DATA(cmsg);

In your test program you only ever send one FD, so this is fine; but
in general multiple FDs can be sent at a time, and you'll receive a
single SOL_SOCKET/SCM_RIGHTS control message with an array of N ints.

E.g., see 
https://code.google.com/p/chromium/codesearch#chromium/src/base/posix/unix_domain_socket_linux.ccl=130.

 So the two issues show up when you want to call sendmsg and recvmsg
 with no data at all, but only control messages.  sendmsg allocates an
 empty mbuf for data (m_len == 0), this mbuf is not discarded and freed
 by soreceive for SOCK_STREAM sockets, and stays in the socket buffer.

 I understand that this API was probably not designed to pass only
 control messages, and no data at all.  But it is easily fixable, and the
 manpage could provide portability information.

Blah, I'm not really a fan of allowing sending zero-length data
messages.  It makes it tricky for receivers to tell if they actually
received an EOF or not.

But if we're going to allow it (which we do currently it seems), then
your diff seems like a reasonable fix to me.  mbufs aren't my
specialty though. =/



Re: UPDATE: xf86-input-synaptics 1.8.0

2014-05-14 Thread Claudio Jeker
On Tue, May 13, 2014 at 02:03:57PM +0600, Alexandr Shadchin wrote:
 Hi,
 
 This diff updates xf86-input-synaptics to the latest release 1.8.0.
 
 Comments ? OK ?

Running with this on a Thinkpad X240 and it works for me just fine.
Using the following config:

Section InputClass
Identifier Lenovo TrackPoint top software buttons
MatchDriver synaptics
Option ClickPad true
Option HasSecondarySoftButtons true
Option TouchpadOff 1
Option AreaTopEdge 20%
Option SoftButtonAreas 60% 0 82% 0 40% 60% 82% 0
Option SecondarySoftButtonAreas 60% 0 0 20% 40% 60% 0 20%
EndSection

-- 
:wq Claudio



better free checking

2014-05-14 Thread Ted Unangst
As I learned the hard way not long ago, free() doesn't detect all
errors because of the delay mechanism. We can make two improvements.

1. Perform the sanity checking from free_bytes before we insert
something into the delay array. This detects many kinds of badness
much sooner.

2. Check that the freed pointer isn't the same as the pointer in the
delay array. Checking the entire array would be more complete, but
slower. Randomly crashing immediately is a modest improvement.

Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.163
diff -u -p -r1.163 malloc.c
--- malloc.c12 May 2014 19:02:20 -  1.163
+++ malloc.c14 May 2014 19:21:15 -
@@ -966,16 +966,11 @@ malloc_bytes(struct dir_info *d, size_t 
return ((char *)bp-page + k);
 }
 
-
-/*
- * Free a chunk, and possibly the page it's on, if the page becomes empty.
- */
 static void
-free_bytes(struct dir_info *d, struct region_info *r, void *ptr)
+check_free_chunk(struct dir_info *d, struct region_info *r, void *ptr)
 {
-   struct chunk_head *mp;
struct chunk_info *info;
-   int i, listnum;
+   int i;
 
info = (struct chunk_info *)r-size;
if (info-canary != d-canary1)
@@ -992,6 +987,24 @@ free_bytes(struct dir_info *d, struct re
wrterror(chunk is already free, ptr);
return;
}
+}
+
+/*
+ * Free a chunk, and possibly the page it's on, if the page becomes empty.
+ */
+static void
+free_bytes(struct dir_info *d, struct region_info *r, void *ptr)
+{
+   struct chunk_head *mp;
+   struct chunk_info *info;
+   int i, listnum;
+
+   check_free_chunk(d, r, ptr);
+
+   info = (struct chunk_info *)r-size;
+
+   /* Find the chunk number on the page */
+   i = ((uintptr_t)ptr  MALLOC_PAGEMASK)  info-shift;
 
info-bits[i / MALLOC_BITS] |= 1U  (i % MALLOC_BITS);
info-free++;
@@ -1204,9 +1217,14 @@ ofree(void *p)
if (mopts.malloc_junk  sz  0)
memset(p, SOME_FREEJUNK, sz);
if (!mopts.malloc_freenow) {
+   check_free_chunk(g_pool, r, p);
i = getrbyte()  MALLOC_DELAYED_CHUNK_MASK;
tmp = p;
p = g_pool-delayed_chunks[i];
+   if (tmp == p) {
+   wrterror(double free, p);
+   return;
+   }
g_pool-delayed_chunks[i] = tmp;
}
if (p != NULL) {




Re: fun with smtpd, (two bugs)

2014-05-14 Thread Frank Brodbeck
Hi,

here's a new version for the manpage bug Creamy mentioned.

Frank.

Index: usr.sbin/smtpd/smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.117
diff -u -r1.117 smtpd.conf.5
--- usr.sbin/smtpd/smtpd.conf.5 16 Feb 2014 21:59:34 -  1.117
+++ usr.sbin/smtpd/smtpd.conf.5 14 May 2014 21:11:17 -
@@ -109,7 +109,7 @@
 .Ic from
 .Op Ic \!
 .Ic source
-.Ic table
+.Aq Ar table
 .Xc
 The rule matches if the connection is made from a client whose address
 is declared in the table
@@ -121,7 +121,7 @@
 .It Xo
 .Ic sender
 .Op Ic \!
-.Ic senders
+.Aq Ar senders
 .Xc
 If specified, the rule will only be matched if the sender email address
 is found in the table
@@ -253,7 +253,7 @@
 .It Xo
 .Ic recipient
 .Op Ic \!
-.Ar recipients
+.Aq Ar recipients
 .Xc
 If specified, the rule will only be matched if the recipient email address
 is found in the table
@@ -307,7 +307,7 @@
 .Ic relay
 .Op Ic backup Op Ar mx
 .Op Ic as Ar address
-.Op Ic source Ar source
+.Op Ic source Aq Ar source
 .Bk -words
 .Op Ic hostname Ar name
 .Op Ic hostnames Ar names
@@ -592,7 +592,11 @@
 .Op Ic port Ar port
 .Op Ic tls | tls-require | tls-require verify | smtps | secure
 .Op Ic pki Ar pkiname
-.Op Ic auth | auth-optional | auth Ar authtable | Ic auth-optional Ar authtable
+.Oo
+.Ic auth | auth-optional | auth 
+.Aq Ar authtable
+.Ic | auth-optional Aq Ar authtable
+.Oc
 .Op Ic tag Ar tag
 .Op Ic hostname Ar hostname
 .Op Ic hostnames Ar names
-- 
Frank Brodbeck f...@guug.de



libcrypto: stop some ASN.1 bleeding

2014-05-14 Thread Miod Vallat
... or, in other words, try to fix most memory leak upon failure.
This kind of change is difficult to test, the more eyes reviewing it,
the better.

Miod

Index: a_gentm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_gentm.c
--- a_gentm.c   19 Apr 2014 11:43:07 -  1.17
+++ a_gentm.c   14 May 2014 21:18:03 -
@@ -212,41 +212,48 @@ ASN1_GENERALIZEDTIME *
 ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
 long offset_sec)
 {
+   ASN1_GENERALIZEDTIME *ret;
char *p;
struct tm *ts;
struct tm data;
size_t len = 20;
 
-   if (s == NULL)
-   s = M_ASN1_GENERALIZEDTIME_new();
-   if (s == NULL)
-   return (NULL);
+   if (s == NULL) {
+   ret = M_ASN1_GENERALIZEDTIME_new();
+   if (ret == NULL)
+   return (NULL);
+   } else
+   ret = s;
 
ts = gmtime_r(t, data);
if (ts == NULL)
-   return (NULL);
+   goto err;
 
if (offset_day || offset_sec) {
if (!OPENSSL_gmtime_adj(ts, offset_day, offset_sec))
-   return NULL;
+   goto err;
}
 
-   p = (char *)s-data;
-   if ((p == NULL) || ((size_t)s-length  len)) {
+   p = (char *)ret-data;
+   if ((p == NULL) || ((size_t)ret-length  len)) {
p = malloc(len);
if (p == NULL) {
ASN1err(ASN1_F_ASN1_GENERALIZEDTIME_ADJ,
ERR_R_MALLOC_FAILURE);
-   return (NULL);
+   goto err;
}
-   if (s-data != NULL)
-   free(s-data);
-   s-data = (unsigned char *)p;
+   if (ret-data != NULL)
+   free(ret-data);
+   ret-data = (unsigned char *)p;
}
 
snprintf(p, len, %04d%02d%02d%02d%02d%02dZ, ts-tm_year + 1900,
ts-tm_mon + 1, ts-tm_mday, ts-tm_hour, ts-tm_min, ts-tm_sec);
-   s-length = strlen(p);
-   s-type = V_ASN1_GENERALIZEDTIME;
-   return (s);
+   ret-length = strlen(p);
+   ret-type = V_ASN1_GENERALIZEDTIME;
+   return (ret);
+err:
+   if (ret != s)
+   M_ASN1_GENERALIZEDTIME_free(ret);
+   return NULL;
 }
Index: a_time.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
retrieving revision 1.17
diff -u -p -r1.17 a_time.c
--- a_time.c21 Apr 2014 00:52:00 -  1.17
+++ a_time.c14 May 2014 21:18:03 -
@@ -123,7 +123,7 @@ ASN1_TIME_check(ASN1_TIME *t)
 ASN1_GENERALIZEDTIME *
 ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
 {
-   ASN1_GENERALIZEDTIME *ret;
+   ASN1_GENERALIZEDTIME *ret = NULL;
char *str;
int newlen;
int i;
@@ -132,33 +132,41 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
return NULL;
 
if (!out || !*out) {
-   if (!(ret = ASN1_GENERALIZEDTIME_new ()))
+   if (!(ret = ASN1_GENERALIZEDTIME_new()))
return NULL;
-   if (out)
-   *out = ret;
} else
ret = *out;
 
/* If already GeneralizedTime just copy across */
if (t-type == V_ASN1_GENERALIZEDTIME) {
if (!ASN1_STRING_set(ret, t-data, t-length))
-   return NULL;
-   return ret;
+   goto err;
+   goto done;
}
 
/* grow the string */
if (!ASN1_STRING_set(ret, NULL, t-length + 2))
-   return NULL;
+   goto err;
/* ASN1_STRING_set() allocated 'len + 1' bytes. */
newlen = t-length + 2 + 1;
str = (char *)ret-data;
+   /* XXX ASN1_TIME is not Y2050 compatible */
i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
(char *) t-data);
if (i == -1 || i = newlen) {
-   ASN1_STRING_free(ret);
+   M_ASN1_GENERALIZEDTIME_free(ret);
+   if (out  *out == ret)
+   *out = NULL;
return NULL;
}
+done:
+   if (out)
+   *out = ret;
return ret;
+err:
+   if (out != NULL  ret != *out)
+   ASN1_GENERALIZEDTIME_free(ret);
+   return NULL;
 }
 
 int
Index: a_utctm.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_utctm.c,v
retrieving revision 1.22
diff -u -p -r1.22 a_utctm.c
--- a_utctm.c   21 Apr 2014 11:23:09 -  1.22
+++ a_utctm.c   14 May 2014 21:18:03 -
@@ -152,34 +152,37 @@ ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t
 ASN1_UTCTIME *
 ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t 

NOINET6 by default

2014-05-14 Thread Henning Brauer
so as discussed recently having the inet6 link-local addrs on every
interface by default is stupid and a security risk.

this diff fixes that. well, really two independent parts.
one: set the NOINET6 flag by default on each and every interface.
two: implement ifconfig if +inet6 to turn inet6 on and assign
the link-local addr.

this should be transparent for almost all real use cases of inet6
since assigning any inet6 address also resets the flag (and ll is
assigned then as well).
lo0 still gets it's ::1 and fe80::1%lo0 by default.

the only use case that needs config adoption: people ONLY using
link-local, they will need to put +inet6 in the corresponding
hostname.if file.

ok?

Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.288
diff -u -p -r1.288 if.c
--- sys/net/if.c13 May 2014 14:33:25 -  1.288
+++ sys/net/if.c14 May 2014 21:03:45 -
@@ -429,6 +429,9 @@ if_attach(struct ifnet *ifp)
 #else
TAILQ_INSERT_TAIL(ifnet, ifp, if_list);
 #endif
+#ifdef INET6
+   ifp-if_xflags |= IFXF_NOINET6;
+#endif
 
m_clinitifp(ifp);
 
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.241
diff -u -p -r1.241 ifconfig.8
--- sbin/ifconfig/ifconfig.822 Apr 2014 10:11:32 -  1.241
+++ sbin/ifconfig/ifconfig.813 May 2014 14:58:58 -
@@ -283,8 +283,12 @@ Disable
 on the given interface and remove all configured
 .Xr inet6 4
 addresses, including the link-local ones.
-To turn it on again, assign any inet6 address or run
+To turn it on again, use +inet6, assign any inet6 address or run
 .Xr rtsol 8 .
+.It +inet6
+Enable
+.Xr inet6 4
+and assign a link local address if the interface doesn't have one yet.
 .It Cm instance Ar minst
 Set the media instance to
 .Ar minst .
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.283
diff -u -p -r1.283 ifconfig.c
--- sbin/ifconfig/ifconfig.c12 May 2014 08:47:37 -  1.283
+++ sbin/ifconfig/ifconfig.c13 May 2014 14:56:25 -
@@ -412,6 +412,7 @@ const structcmd {
{ -flowdst, 1,0,  unsetpflow_receiver },
{ pflowproto, NEXTARG,0,  setpflowproto },
{ -inet6, IFXF_NOINET6,   0,  setifxflags } ,
+   { +inet6, 0,  0,  setia6eui64 },
{ keepalive,  NEXTARG2,   0,  NULL, setkeepalive },
{ -keepalive, 1,  0,  unsetkeepalive },
{ add,NEXTARG,0,  bridge_add },
@@ -1310,7 +1311,9 @@ setia6eui64(const char *cmd, int val)
const struct in6_addr *lladdr = NULL;
struct in6_addr *in6;
 
-   if (afp-af_af != AF_INET6)
+   if (!strcmp(cmd, +inet6))
+   setifxflags(inet6, -IFXF_NOINET6);
+   else if (afp-af_af != AF_INET6)
errx(1, %s not allowed for the AF, cmd);
in6 = (struct in6_addr *)in6_addreq.ifra_addr.sin6_addr;
if (memcmp(in6addr_any.s6_addr[8], in6-s6_addr[8], 8) != 0)



-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual  Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: rtadvd prints error on receiving RA with Route Information

2014-05-14 Thread Alexander Bluhm
On Wed, May 14, 2014 at 07:55:44PM +0200, J??r??mie Courr??ges-Anglas wrote:
 
 Redirecting this to tech@
 
 Consus has confirmed this fixed his issue.  I plan to commit it this
 week-end if I hear no objection.
 
 j...@wxcvbn.org (J??r??mie Courr??ges-Anglas) writes:
 
  Thanks for the detailed report.
 
  This diff makes rtadvd aware of route info messages on the listening
  side, no functional change except that the error message goes away.
  We just ignore the message since in that case there's no good reason to
  try to validate the more specifics advertized by other routers.
 
  ok?

OK bluhm@

 
 Index: rtadvd.c
 ===
 RCS file: /cvs/src/usr.sbin/rtadvd/rtadvd.c,v
 retrieving revision 1.45
 diff -u -p -r1.45 rtadvd.c
 --- rtadvd.c  5 May 2013 14:25:52 -   1.45
 +++ rtadvd.c  6 May 2014 14:21:42 -
 @@ -121,6 +121,7 @@ union nd_opts {
  #define NDOPT_FLAG_MTU   (1  4)
  #define NDOPT_FLAG_RDNSS (1  5)
  #define NDOPT_FLAG_DNSSL (1  6)
 +#define NDOPT_FLAG_ROUTE_INFO(1  7)
  
  u_int32_t ndopt_flags[] = {
   [ND_OPT_SOURCE_LINKADDR]= NDOPT_FLAG_SRCLINKADDR,
 @@ -128,6 +129,7 @@ u_int32_t ndopt_flags[] = {
   [ND_OPT_PREFIX_INFORMATION] = NDOPT_FLAG_PREFIXINFO,
   [ND_OPT_REDIRECTED_HEADER]  = NDOPT_FLAG_RDHDR,
   [ND_OPT_MTU]= NDOPT_FLAG_MTU,
 + [ND_OPT_ROUTE_INFO] = NDOPT_FLAG_ROUTE_INFO,
   [ND_OPT_RDNSS]  = NDOPT_FLAG_RDNSS,
   [ND_OPT_DNSSL]  = NDOPT_FLAG_DNSSL,
  };
 @@ -809,7 +811,8 @@ ra_input(int len, struct nd_router_adver
   if (nd6_options((struct nd_opt_hdr *)(ra + 1),
   len - sizeof(struct nd_router_advert),
   ndopts, NDOPT_FLAG_SRCLINKADDR | NDOPT_FLAG_PREFIXINFO
 - | NDOPT_FLAG_MTU | NDOPT_FLAG_RDNSS | 
 NDOPT_FLAG_DNSSL)) {
 + | NDOPT_FLAG_MTU | NDOPT_FLAG_ROUTE_INFO
 + | NDOPT_FLAG_RDNSS | NDOPT_FLAG_DNSSL)) {
   log_warnx(ND option check failed for an RA from %s on %s,
   inet_ntop(AF_INET6, from-sin6_addr,
   ntopbuf, INET6_ADDRSTRLEN),
 @@ -1109,6 +1112,7 @@ nd6_options(struct nd_opt_hdr *hdr, int 
   }
  
   if (hdr-nd_opt_type  ND_OPT_MTU 
 + hdr-nd_opt_type != ND_OPT_ROUTE_INFO 
   hdr-nd_opt_type != ND_OPT_RDNSS 
   hdr-nd_opt_type != ND_OPT_DNSSL)
   {
 @@ -1142,6 +1146,7 @@ nd6_options(struct nd_opt_hdr *hdr, int 
   case ND_OPT_SOURCE_LINKADDR:
   case ND_OPT_TARGET_LINKADDR:
   case ND_OPT_REDIRECTED_HEADER:
 + case ND_OPT_ROUTE_INFO:
   case ND_OPT_RDNSS:
   case ND_OPT_DNSSL:
   break;  /* we don't care about these options */
 
 -- 
 jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
 



IFT_L2VLAN is unused

2014-05-14 Thread Henning Brauer
don't kill the define, since this is userland visible, but there is
never ever an interface in our kernel with if_type == IFT_L2VLAN - see
my commit from 2 weeks ago or so. To clarify this once again, I didn't
remove the L2VLAN use, it was never really used.

ok?

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.288
diff -u -p -r1.288 if.c
--- net/if.c13 May 2014 14:33:25 -  1.288
+++ net/if.c14 May 2014 21:41:01 -
@@ -1625,7 +1625,6 @@ ifioctl(struct socket *so, u_long cmd, c
case IFT_CARP:
case IFT_XETHER:
case IFT_ISO88025:
-   case IFT_L2VLAN:
bcopy((caddr_t)ifr-ifr_addr.sa_data,
(caddr_t)((struct arpcom *)ifp)-ac_enaddr,
ETHER_ADDR_LEN);
Index: net/if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.38
diff -u -p -r1.38 if_pppoe.c
--- net/if_pppoe.c  14 Apr 2014 09:06:42 -  1.38
+++ net/if_pppoe.c  24 Apr 2014 23:38:10 -
@@ -924,9 +924,7 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
struct ifnet*eth_if;
 
eth_if = ifunit(parms-eth_ifname);
-   if (eth_if == NULL ||
-   (eth_if-if_type != IFT_ETHER 
-eth_if-if_type != IFT_L2VLAN)) {
+   if (eth_if == NULL || eth_if-if_type != IFT_ETHER) {
sc-sc_eth_if = NULL;
return (ENXIO);
}
Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.116
diff -u -p -r1.116 nd6.c
--- netinet6/nd6.c  7 May 2014 08:14:59 -   1.116
+++ netinet6/nd6.c  13 May 2014 14:38:16 -
@@ -1830,7 +1830,6 @@ nd6_need_cache(struct ifnet *ifp)
case IFT_ETHER:
case IFT_IEEE1394:
case IFT_PROPVIRTUAL:
-   case IFT_L2VLAN:
case IFT_IEEE80211:
case IFT_CARP:
case IFT_GIF:   /* XXX need more cases? */
Index: netinet6/nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.78
diff -u -p -r1.78 nd6_nbr.c
--- netinet6/nd6_nbr.c  18 Apr 2014 10:48:30 -  1.78
+++ netinet6/nd6_nbr.c  24 Apr 2014 23:38:10 -
@@ -1064,7 +1064,6 @@ nd6_ifptomac(struct ifnet *ifp)
case IFT_IEEE1394:
case IFT_PROPVIRTUAL:
case IFT_CARP:
-   case IFT_L2VLAN:
case IFT_IEEE80211:
return ((caddr_t)(ifp + 1));
default:

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual  Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: NOINET6 by default

2014-05-14 Thread Paul de Weerd
Hi Henning,

Thanks, I really like this.

On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote:
| the only use case that needs config adoption: people ONLY using
| link-local, they will need to put +inet6 in the corresponding
| hostname.if file.

People that set net.inet6.ip6.accept_rtadv=1 but don't actively do
router solicitation will now no longer get an IPv6 address (a real
one, not just link local) after some time (when there are router
advertisements on the network).  This seems blatantly obvious, but
might still warrant an entry in current.html as it's contrary to
previous behavior.  Suggested diff included,

Anyway, I've verified that my system now comes up with ::1 and
fe80::1%lo0 on lo0 but other interfaces (software ones like vether(4)
and real ones like re(4) and iwn(4)) don't get an address after just
bringing the interface up.  Using +inet6 works and gets me a
link-local address (and, when I do this on a network with router
advertisements, I get an address and an autoconfprivacy address after
some time).  And of course, configuring IPv6 works as usual.

| ok?

This makes a whole lot of sense to me.  Please make OpenBSD the first
OS to do (this part of) v6 in a sensible way.

Paul 'WEiRD' de Weerd

Index: current.html
===
RCS file: /cvs/www/faq/current.html,v
retrieving revision 1.508
diff -u -p -r1.508 current.html
--- current.html7 May 2014 13:38:43 -   1.508
+++ current.html14 May 2014 22:01:30 -
@@ -52,6 +52,7 @@
 lia href=#201404212014/04/21 - lpd(8): hosts.equiv removed/a
 lia href=#201404222014/04/22 - kerberosV removed/a
 lia href=#201405032014/05/03 - bcrypt hash advanced to $2b$ mode/a
+lia href=#201405162014/05/16 - IPv6 defaults to off when not 
configured/a
 /ul
 
 hr
@@ -490,6 +491,14 @@ manually or use the kerberos packages wh
 The bcrypt(3) hash has been advanced to the new $2b$ mode.  Newly
 created hashes will follow this form.  Older systems may not
 understand this format.
+
+a name=20140516/a
+h32014/05/16 - IPv6 defaults to off when not configured/h3
+
+Interfaces no longer get a link-local address configured by default
+when no other IPv6 configuration is present.  If you want the old
+behaviour restored for sepcific interfaces, add +inet6 to the
+hostname.if(5) file or on the ifconfig(8) command line.
 
 hr
 br


| Index: sys/net/if.c
| ===
| RCS file: /cvs/src/sys/net/if.c,v
| retrieving revision 1.288
| diff -u -p -r1.288 if.c
| --- sys/net/if.c  13 May 2014 14:33:25 -  1.288
| +++ sys/net/if.c  14 May 2014 21:03:45 -
| @@ -429,6 +429,9 @@ if_attach(struct ifnet *ifp)
|  #else
|   TAILQ_INSERT_TAIL(ifnet, ifp, if_list);
|  #endif
| +#ifdef INET6
| + ifp-if_xflags |= IFXF_NOINET6;
| +#endif
|  
|   m_clinitifp(ifp);
|  
| Index: sbin/ifconfig/ifconfig.8
| ===
| RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
| retrieving revision 1.241
| diff -u -p -r1.241 ifconfig.8
| --- sbin/ifconfig/ifconfig.8  22 Apr 2014 10:11:32 -  1.241
| +++ sbin/ifconfig/ifconfig.8  13 May 2014 14:58:58 -
| @@ -283,8 +283,12 @@ Disable
|  on the given interface and remove all configured
|  .Xr inet6 4
|  addresses, including the link-local ones.
| -To turn it on again, assign any inet6 address or run
| +To turn it on again, use +inet6, assign any inet6 address or run
|  .Xr rtsol 8 .
| +.It +inet6
| +Enable
| +.Xr inet6 4
| +and assign a link local address if the interface doesn't have one yet.
|  .It Cm instance Ar minst
|  Set the media instance to
|  .Ar minst .
| Index: sbin/ifconfig/ifconfig.c
| ===
| RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
| retrieving revision 1.283
| diff -u -p -r1.283 ifconfig.c
| --- sbin/ifconfig/ifconfig.c  12 May 2014 08:47:37 -  1.283
| +++ sbin/ifconfig/ifconfig.c  13 May 2014 14:56:25 -
| @@ -412,6 +412,7 @@ const struct  cmd {
|   { -flowdst, 1,0,  unsetpflow_receiver },
|   { pflowproto, NEXTARG,0,  setpflowproto },
|   { -inet6, IFXF_NOINET6,   0,  setifxflags } ,
| + { +inet6, 0,  0,  setia6eui64 },
|   { keepalive,  NEXTARG2,   0,  NULL, setkeepalive },
|   { -keepalive, 1,  0,  unsetkeepalive },
|   { add,NEXTARG,0,  bridge_add },
| @@ -1310,7 +1311,9 @@ setia6eui64(const char *cmd, int val)
|   const struct in6_addr *lladdr = NULL;
|   struct in6_addr *in6;
|  
| - if (afp-af_af != AF_INET6)
| + if (!strcmp(cmd, +inet6))
| + setifxflags(inet6, -IFXF_NOINET6);
| + else if (afp-af_af != AF_INET6)
|   errx(1, %s not allowed for the AF, cmd);
|   in6 = (struct in6_addr 

[PATCH] rcs regression tests

2014-05-14 Thread Fritjof Bornebusch
Hi tech,

I added some missing ; to the rlog out files, to make sure these tests don't 
fail.


fritjof


Index: rlog-rflag2.out
===
RCS file: /cvs/src/regress/usr.bin/rcs/rlog-rflag2.out,v
retrieving revision 1.1
diff -u -p -r1.1 rlog-rflag2.out
--- rlog-rflag2.out 20 Apr 2006 17:17:22 -  1.1
+++ rlog-rflag2.out 14 May 2014 22:01:55 -
@@ -12,11 +12,11 @@ description:
 descr
 
 revision 1.3
-date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0
+date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0;
 third rev
 
 revision 1.2
-date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0
+date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0;
 second rev
 
 revision 1.1


Index: rlog-rflag3.out
===
RCS file: /cvs/src/regress/usr.bin/rcs/rlog-rflag3.out,v
retrieving revision 1.1
diff -u -p -r1.1 rlog-rflag3.out
--- rlog-rflag3.out 20 Apr 2006 17:17:22 -  1.1
+++ rlog-rflag3.out 14 May 2014 22:02:02 -
@@ -12,10 +12,10 @@ description:
 descr
 
 revision 1.3
-date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0
+date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0;
 third rev
 
 revision 1.2
-date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0
+date: 2006/01/01 00:00:00;  author: foo;  state: Exp;  lines: +1 -0;
 second rev
 =



Re: NOINET6 by default

2014-05-14 Thread Stuart Henderson
On 2014/05/15 00:02, Paul de Weerd wrote:
 Hi Henning,
 
 Thanks, I really like this.
 
 On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote:
 | the only use case that needs config adoption: people ONLY using
 | link-local, they will need to put +inet6 in the corresponding
 | hostname.if file.
 
 People that set net.inet6.ip6.accept_rtadv=1 but don't actively do
 router solicitation will now no longer get an IPv6 address (a real
 one, not just link local) after some time (when there are router
 advertisements on the network).  This seems blatantly obvious, but
 might still warrant an entry in current.html as it's contrary to
 previous behavior.  Suggested diff included,
 
 Anyway, I've verified that my system now comes up with ::1 and
 fe80::1%lo0 on lo0 but other interfaces (software ones like vether(4)
 and real ones like re(4) and iwn(4)) don't get an address after just
 bringing the interface up.  Using +inet6 works and gets me a
 link-local address (and, when I do this on a network with router
 advertisements, I get an address and an autoconfprivacy address after
 some time).  And of course, configuring IPv6 works as usual.
 
 | ok?
 
 This makes a whole lot of sense to me.  Please make OpenBSD the first
 OS to do (this part of) v6 in a sensible way.
 
 Paul 'WEiRD' de Weerd
 
 Index: current.html
 ===
 RCS file: /cvs/www/faq/current.html,v
 retrieving revision 1.508
 diff -u -p -r1.508 current.html
 --- current.html  7 May 2014 13:38:43 -   1.508
 +++ current.html  14 May 2014 22:01:30 -
 @@ -52,6 +52,7 @@
  lia href=#201404212014/04/21 - lpd(8): hosts.equiv removed/a
  lia href=#201404222014/04/22 - kerberosV removed/a
  lia href=#201405032014/05/03 - bcrypt hash advanced to $2b$ mode/a
 +lia href=#201405162014/05/16 - IPv6 defaults to off when not 
 configured/a
  /ul
  
  hr
 @@ -490,6 +491,14 @@ manually or use the kerberos packages wh
  The bcrypt(3) hash has been advanced to the new $2b$ mode.  Newly
  created hashes will follow this form.  Older systems may not
  understand this format.
 +
 +a name=20140516/a
 +h32014/05/16 - IPv6 defaults to off when not configured/h3
 +
 +Interfaces no longer get a link-local address configured by default
 +when no other IPv6 configuration is present.  If you want the old

Maybe s/when no other/unless explicit/ ?

 +behaviour restored for sepcific interfaces, add +inet6 to the
 +hostname.if(5) file or on the ifconfig(8) command line.

Cue a bunch of people needlessly adding +inet6 to hostname.if files
when they already have configuration, but I don't think there's anything
we can do about that and it doesn't matter anyway. ;)



Re: NOINET6 by default

2014-05-14 Thread Mark Kettenis
 Date: Wed, 14 May 2014 23:29:20 +0200
 From: Henning Brauer lists-openbsdt...@bsws.de

 this diff fixes that. well, really two independent parts.
 one: set the NOINET6 flag by default on each and every interface.

ok on that bit

 two: implement ifconfig if +inet6 to turn inet6 on and assign
 the link-local addr.

I don't think this is a good idea; didn't we establish the other day
that ifconfig if eui64 already did what your +inet6 does?



Re: fun with smtpd, (two bugs)

2014-05-14 Thread Jason McIntyre
On Wed, May 14, 2014 at 11:10:03PM +0200, Frank Brodbeck wrote:
 Hi,
 
 here's a new version for the manpage bug Creamy mentioned.
 
 Frank.
 

it's still missing some . also i think we can wholly simplify the
auth/auth-optional line.

how's this?
jmc

Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.117
diff -u -r1.117 smtpd.conf.5
--- smtpd.conf.516 Feb 2014 21:59:34 -  1.117
+++ smtpd.conf.514 May 2014 22:13:44 -
@@ -109,7 +109,7 @@
 .Ic from
 .Op Ic \!
 .Ic source
-.Ic table
+.Aq Ar table
 .Xc
 The rule matches if the connection is made from a client whose address
 is declared in the table
@@ -121,7 +121,7 @@
 .It Xo
 .Ic sender
 .Op Ic \!
-.Ic senders
+.Aq Ar senders
 .Xc
 If specified, the rule will only be matched if the sender email address
 is found in the table
@@ -253,7 +253,7 @@
 .It Xo
 .Ic recipient
 .Op Ic \!
-.Ar recipients
+.Aq Ar recipients
 .Xc
 If specified, the rule will only be matched if the recipient email address
 is found in the table
@@ -304,17 +304,18 @@
 This parameter may use conversion specifiers that are expanded before use
 .Pq see Sx FORMAT SPECIFIERS .
 .It Xo
+.Bk -words
 .Ic relay
 .Op Ic backup Op Ar mx
 .Op Ic as Ar address
-.Op Ic source Ar source
-.Bk -words
+.Op Ic source Aq Ar source
 .Op Ic hostname Ar name
-.Op Ic hostnames Ar names
-.Ek
+.Op Ic hostnames Aq Ar names
 .Op Ic pki Ar pkiname
 .Op Ic tls | verify
+.Ek
 .Xc
+.Pp
 Mail is relayed.
 The routing decision is based on the DNS system.
 .Pp
@@ -407,12 +408,13 @@
 .Ar host
 .Op Ic auth Aq Ar auth
 .Op Ic as Ar address
-.Op Ic source Ar source
+.Op Ic source Aq Ar source
 .Op Ic hostname Ar name
-.Op Ic hostnames Ar names
+.Op Ic hostnames Aq Ar names
 .Op Ic pki Ar pkiname
 .Op Ic verify
 .Xc
+.Pp
 Mail is relayed through the specified
 .Ar host
 expressed as a URL.
@@ -477,7 +479,7 @@
 parameter is specified,
 .Xr smtpd 8
 will explicitly bind to an address found in the table referenced by
-.Ar table
+.Aq Ar source
 when connecting to the relay.
 If the table contains more than one address, they are picked in turn each
 time a new connection is opened.
@@ -592,13 +594,14 @@
 .Op Ic port Ar port
 .Op Ic tls | tls-require | tls-require verify | smtps | secure
 .Op Ic pki Ar pkiname
-.Op Ic auth | auth-optional | auth Ar authtable | Ic auth-optional Ar authtable
+.Op Ic auth | auth-optional Aq Ar authtable
 .Op Ic tag Ar tag
 .Op Ic hostname Ar hostname
-.Op Ic hostnames Ar names
+.Op Ic hostnames Aq Ar names
 .Op Ic mask-source
 .Ek
 .Xc
+.Pp
 Specify an
 .Ar interface
 and
@@ -662,7 +665,7 @@
 .Ic auth
 and
 .Ic auth-optional
-accept a table as parameter.
+accept an optional table as a parameter.
 When provided, credentials are looked up in this table.
 Credentials format is described in
 .Xr table 5 .



Re: IFT_L2VLAN is unused

2014-05-14 Thread Alexander Bluhm
On Wed, May 14, 2014 at 11:51:13PM +0200, Henning Brauer wrote:
 don't kill the define, since this is userland visible, but there is
 never ever an interface in our kernel with if_type == IFT_L2VLAN - see
 my commit from 2 weeks ago or so. To clarify this once again, I didn't
 remove the L2VLAN use, it was never really used.
 
 ok?

OK bluhm@

 
 Index: net/if.c
 ===
 RCS file: /cvs/src/sys/net/if.c,v
 retrieving revision 1.288
 diff -u -p -r1.288 if.c
 --- net/if.c  13 May 2014 14:33:25 -  1.288
 +++ net/if.c  14 May 2014 21:41:01 -
 @@ -1625,7 +1625,6 @@ ifioctl(struct socket *so, u_long cmd, c
   case IFT_CARP:
   case IFT_XETHER:
   case IFT_ISO88025:
 - case IFT_L2VLAN:
   bcopy((caddr_t)ifr-ifr_addr.sa_data,
   (caddr_t)((struct arpcom *)ifp)-ac_enaddr,
   ETHER_ADDR_LEN);
 Index: net/if_pppoe.c
 ===
 RCS file: /cvs/src/sys/net/if_pppoe.c,v
 retrieving revision 1.38
 diff -u -p -r1.38 if_pppoe.c
 --- net/if_pppoe.c14 Apr 2014 09:06:42 -  1.38
 +++ net/if_pppoe.c24 Apr 2014 23:38:10 -
 @@ -924,9 +924,7 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
   struct ifnet*eth_if;
  
   eth_if = ifunit(parms-eth_ifname);
 - if (eth_if == NULL ||
 - (eth_if-if_type != IFT_ETHER 
 -  eth_if-if_type != IFT_L2VLAN)) {
 + if (eth_if == NULL || eth_if-if_type != IFT_ETHER) {
   sc-sc_eth_if = NULL;
   return (ENXIO);
   }
 Index: netinet6/nd6.c
 ===
 RCS file: /cvs/src/sys/netinet6/nd6.c,v
 retrieving revision 1.116
 diff -u -p -r1.116 nd6.c
 --- netinet6/nd6.c7 May 2014 08:14:59 -   1.116
 +++ netinet6/nd6.c13 May 2014 14:38:16 -
 @@ -1830,7 +1830,6 @@ nd6_need_cache(struct ifnet *ifp)
   case IFT_ETHER:
   case IFT_IEEE1394:
   case IFT_PROPVIRTUAL:
 - case IFT_L2VLAN:
   case IFT_IEEE80211:
   case IFT_CARP:
   case IFT_GIF:   /* XXX need more cases? */
 Index: netinet6/nd6_nbr.c
 ===
 RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
 retrieving revision 1.78
 diff -u -p -r1.78 nd6_nbr.c
 --- netinet6/nd6_nbr.c18 Apr 2014 10:48:30 -  1.78
 +++ netinet6/nd6_nbr.c24 Apr 2014 23:38:10 -
 @@ -1064,7 +1064,6 @@ nd6_ifptomac(struct ifnet *ifp)
   case IFT_IEEE1394:
   case IFT_PROPVIRTUAL:
   case IFT_CARP:
 - case IFT_L2VLAN:
   case IFT_IEEE80211:
   return ((caddr_t)(ifp + 1));
   default:
 
 -- 
 Henning Brauer, h...@bsws.de, henn...@openbsd.org
 BS Web Services GmbH, http://bsws.de, Full-Service ISP
 Secure Hosting, Mail and DNS. Virtual  Dedicated Servers, Root to Fully 
 Managed
 Henning Brauer Consulting, http://henningbrauer.com/



Re: NOINET6 by default

2014-05-14 Thread Ted Unangst
On Thu, May 15, 2014 at 00:14, Alexander Bluhm wrote:
 On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote:
 so as discussed recently having the inet6 link-local addrs on every
 interface by default is stupid and a security risk.
 
 Connecting a computer to the internet is a security risk.  
 IPv4 is on by default, and so IPv6 should be on by default.
 I want both to be handled the same way.

Huh? Running 'ifconfig em0 up' does not assign an IPv4 address.




Re: NOINET6 by default

2014-05-14 Thread Kenneth Westerback
On 14 May 2014 18:14, Alexander Bluhm alexander.bl...@gmx.net wrote:
 On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote:
 so as discussed recently having the inet6 link-local addrs on every
 interface by default is stupid and a security risk.

 Connecting a computer to the internet is a security risk.
 IPv4 is on by default, and so IPv6 should be on by default.
 I want both to be handled the same way.

And that, to me, is what this does, sans the +inet6 bit on which I
agree with Mark. i.e. 'ifconfig if up' does not magically make IPv6
work while not making IPv4 work.


 the only use case that needs config adoption: people ONLY using
 link-local, they will need to put +inet6 in the corresponding
 hostname.if file.

I don't think this is needed. The eui64 thing seemed more obvious to
me. Hmm. '-eui64' to remove it once it was added?


 There is a use case for running IPv6 over an interface without
 setting an address.  Configure a global IPv6 address on lo0, run
 ospf6d on any physical interface and it will provide connection.
 IPv6 autoconfiguration with link-local addresses is useful.

So instead of everyone putting -inet6 in their hostname.if files,
possibly even having to create hostname.if files to prevent interfaces
from magically being portals into your system, now the people who know
they want/need IPv6 have to put 'eui64' lines in the interfaces they
want to be addressable.

Again, this seems more consistant with IPv4 usage than today's setup.
So I vote ok.

 Ken


 ok?

 No

 bluhm




Re: NOINET6 by default

2014-05-14 Thread Henning Brauer
* Mark Kettenis mark.kette...@xs4all.nl [2014-05-15 00:15]:
 I don't think this is a good idea; didn't we establish the other day
 that ifconfig if eui64 already did what your +inet6 does?

almost, it's ifconfig if inet6 eui64 - but that isn't all THAT
intuitive. I like +inet6 as the opposite of -inet6.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual  Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: NOINET6 by default

2014-05-14 Thread Stuart Henderson
On 2014/05/15 00:13, Mark Kettenis wrote:
  Date: Wed, 14 May 2014 23:29:20 +0200
  From: Henning Brauer lists-openbsdt...@bsws.de
 
  this diff fixes that. well, really two independent parts.
  one: set the NOINET6 flag by default on each and every interface.
 
 ok on that bit
 
  two: implement ifconfig if +inet6 to turn inet6 on and assign
  the link-local addr.
 
 I don't think this is a good idea; didn't we establish the other day
 that ifconfig if eui64 already did what your +inet6 does?
 

Actually it's ifconfig if inet6 eui64, so in hostname.if with
the current parser it would probably be something like up inet6 eui64
or !ifconfig $if inet6 eui64 instead ..

Using +inet6 gets around that problem and seems to me like a better
counterpart to the existing -inet6.

(If we were to make this work more nicely in netstart which is a bad
place for any required changes; it's in etc*.tgz which is usually
not updated until after the kernel/binaries).



Re: NOINET6 by default

2014-05-14 Thread Henning Brauer
* Alexander Bluhm alexander.bl...@gmx.net [2014-05-15 00:15]:
 On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote:
  so as discussed recently having the inet6 link-local addrs on every
  interface by default is stupid and a security risk.
 Connecting a computer to the internet is a security risk.  
 IPv4 is on by default, and so IPv6 should be on by default.
 I want both to be handled the same way.

WITH my diff they finally become the same, IPv4 does NOT assign some
special address to the interface by default.

The analogy really breaks here since v4 just doesn't have link local.

  the only use case that needs config adoption: people ONLY using
  link-local, they will need to put +inet6 in the corresponding
  hostname.if file.
 There is a use case for running IPv6 over an interface without
 setting an address.  Configure a global IPv6 address on lo0, run
 ospf6d on any physical interface and it will provide connection.
 IPv6 autoconfiguration with link-local addresses is useful.

so you put +inet6 in the corresponding hostname.if file and everything
works like you want it to.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual  Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: What platforms grow the stack upwards?

2014-05-14 Thread Stuart Henderson
On 2014/05/15 00:47, Juan Francisco Cantero Hurtado wrote:
 I have not found a complete list with this information. Can someone tell
 me what OpenBSD platforms grow the stack upwards?

hppa



Re: NOINET6 by default

2014-05-14 Thread Reyk Flöter

 On 15.05.2014, at 00:46, Henning Brauer lists-openbsdt...@bsws.de wrote:
 
 * Mark Kettenis mark.kette...@xs4all.nl [2014-05-15 00:15]:
 I don't think this is a good idea; didn't we establish the other day
 that ifconfig if eui64 already did what your +inet6 does?
 
 almost, it's ifconfig if inet6 eui64 - but that isn't all THAT
 intuitive. I like +inet6 as the opposite of -inet6.
 

We don't have + something. It is foo or -foo but not +foo. I know that inet6 
is already used for the regular addresses, but +inet6 sounds like an 
inconsistent workaround for a workaround. I don't like it.

To enable IPv6 link-local I would rather prefer two options to put either 
inet6 eui64 (or an alias like inet6 link-local) or an actual inet6 address 
in your hostname.if. The latter should automatically remove the flag and enable 
the link-local address - does it work this way?

Reyk



Re: NOINET6 by default

2014-05-14 Thread Alexander Hall

On 05/15/14 00:48, Henning Brauer wrote:

* Alexander Bluhm alexander.bl...@gmx.net [2014-05-15 00:15]:

On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote:

so as discussed recently having the inet6 link-local addrs on every
interface by default is stupid and a security risk.

Connecting a computer to the internet is a security risk.
IPv4 is on by default, and so IPv6 should be on by default.
I want both to be handled the same way.


WITH my diff they finally become the same, IPv4 does NOT assign some
special address to the interface by default.

The analogy really breaks here since v4 just doesn't have link local.


the only use case that needs config adoption: people ONLY using
link-local, they will need to put +inet6 in the corresponding
hostname.if file.

There is a use case for running IPv6 over an interface without
setting an address.  Configure a global IPv6 address on lo0, run
ospf6d on any physical interface and it will provide connection.
IPv6 autoconfiguration with link-local addresses is useful.


so you put +inet6 in the corresponding hostname.if file and everything
works like you want it to.


While my powers in this field are limited, I would really hate to see 
another operator type (e.g. +foo +bar) being introduced in ifconfig. 
It's cludgy enough already IMO.


If we need +inet6, it's a pointer that inet6 is already busy and we 
should look something else.


eui64 sounds better so me.

/Alexander



Re: NOINET6 by default

2014-05-14 Thread Stuart Henderson
On 2014/05/14 23:47, Stuart Henderson wrote:
 On 2014/05/15 00:13, Mark Kettenis wrote:
   Date: Wed, 14 May 2014 23:29:20 +0200
   From: Henning Brauer lists-openbsdt...@bsws.de
  
   this diff fixes that. well, really two independent parts.
   one: set the NOINET6 flag by default on each and every interface.
  
  ok on that bit
  
   two: implement ifconfig if +inet6 to turn inet6 on and assign
   the link-local addr.
  
  I don't think this is a good idea; didn't we establish the other day
  that ifconfig if eui64 already did what your +inet6 does?
  
 
 Actually it's ifconfig if inet6 eui64, so in hostname.if with
 the current parser it would probably be something like up inet6 eui64
 or !ifconfig $if inet6 eui64 instead ..

oh, actually reading netstart it seems we're ok with inet6 eui64
in hostname.if, providing there is nothing else on the line.. still
the handling of inet / inet6 lines in this file is an area of
confusion and unexpected behaviour that I would quite like to avoid
adding to if possible..

 Using +inet6 gets around that problem and seems to me like a better
 counterpart to the existing -inet6.
 
 (If we were to make this work more nicely in netstart which is a bad
 place for any required changes; it's in etc*.tgz which is usually
 not updated until after the kernel/binaries).
 




Re: libcrypto: stop some ASN.1 bleeding

2014-05-14 Thread macdonellba
Miod Vallat m...@online.fr writes:

 ... or, in other words, try to fix most memory leak upon failure.
 This kind of change is difficult to test, the more eyes reviewing it,
 the better.

Well, I'll try to take a stab at it then.


 Miod

 Index: a_gentm.c
 ===
 RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_gentm.c,v
 retrieving revision 1.17
 diff -u -p -r1.17 a_gentm.c
 --- a_gentm.c 19 Apr 2014 11:43:07 -  1.17
 +++ a_gentm.c 14 May 2014 21:18:03 -
 @@ -212,41 +212,48 @@ ASN1_GENERALIZEDTIME *
  ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, time_t t, int offset_day,
  long offset_sec)
  {
 + ASN1_GENERALIZEDTIME *ret;
   char *p;
   struct tm *ts;
   struct tm data;
   size_t len = 20;
  
 - if (s == NULL)
 - s = M_ASN1_GENERALIZEDTIME_new();
 - if (s == NULL)
 - return (NULL);
 + if (s == NULL) {
 + ret = M_ASN1_GENERALIZEDTIME_new();
 + if (ret == NULL)
 + return (NULL);
 + } else
 + ret = s;
  
   ts = gmtime_r(t, data);
   if (ts == NULL)
 - return (NULL);
 + goto err;
  
   if (offset_day || offset_sec) {
   if (!OPENSSL_gmtime_adj(ts, offset_day, offset_sec))
 - return NULL;
 + goto err;
   }
  
 - p = (char *)s-data;
 - if ((p == NULL) || ((size_t)s-length  len)) {
 + p = (char *)ret-data;
 + if ((p == NULL) || ((size_t)ret-length  len)) {
   p = malloc(len);
   if (p == NULL) {
   ASN1err(ASN1_F_ASN1_GENERALIZEDTIME_ADJ,
   ERR_R_MALLOC_FAILURE);
 - return (NULL);
 + goto err;
   }
 - if (s-data != NULL)
 - free(s-data);
 - s-data = (unsigned char *)p;
 + if (ret-data != NULL)
 + free(ret-data);
 + ret-data = (unsigned char *)p;
   }

You might be able to use ASN1_STRING_set(ret, NULL, len) to handle
resizing / allocating the string the same way that it's used in
ASN1_TIME_to_generalizedtime(). Alternately, you might want to at least
use 'p = realloc(ret-data, len)' instead of malloc + free.

  
   snprintf(p, len, %04d%02d%02d%02d%02d%02dZ, ts-tm_year + 1900,
   ts-tm_mon + 1, ts-tm_mday, ts-tm_hour, ts-tm_min, ts-tm_sec);
 - s-length = strlen(p);
 - s-type = V_ASN1_GENERALIZEDTIME;
 - return (s);
 + ret-length = strlen(p);
 + ret-type = V_ASN1_GENERALIZEDTIME;
 + return (ret);
 +err:
 + if (ret != s)
 + M_ASN1_GENERALIZEDTIME_free(ret);
 + return NULL;
  }
 Index: a_time.c
 ===
 RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_time.c,v
 retrieving revision 1.17
 diff -u -p -r1.17 a_time.c
 --- a_time.c  21 Apr 2014 00:52:00 -  1.17
 +++ a_time.c  14 May 2014 21:18:03 -
 @@ -123,7 +123,7 @@ ASN1_TIME_check(ASN1_TIME *t)
  ASN1_GENERALIZEDTIME *
  ASN1_TIME_to_generalizedtime(ASN1_TIME *t, ASN1_GENERALIZEDTIME **out)
  {
 - ASN1_GENERALIZEDTIME *ret;
 + ASN1_GENERALIZEDTIME *ret = NULL;
   char *str;
   int newlen;
   int i;
 @@ -132,33 +132,41 @@ ASN1_TIME_to_generalizedtime(ASN1_TIME *
   return NULL;
  
   if (!out || !*out) {
 - if (!(ret = ASN1_GENERALIZEDTIME_new ()))
 + if (!(ret = ASN1_GENERALIZEDTIME_new()))
   return NULL;
 - if (out)
 - *out = ret;
   } else
   ret = *out;
  
   /* If already GeneralizedTime just copy across */
   if (t-type == V_ASN1_GENERALIZEDTIME) {
   if (!ASN1_STRING_set(ret, t-data, t-length))
 - return NULL;
 - return ret;
 + goto err;
 + goto done;
   }
  
   /* grow the string */
   if (!ASN1_STRING_set(ret, NULL, t-length + 2))
 - return NULL;
 + goto err;
   /* ASN1_STRING_set() allocated 'len + 1' bytes. */
   newlen = t-length + 2 + 1;
   str = (char *)ret-data;
 + /* XXX ASN1_TIME is not Y2050 compatible */
   i = snprintf(str, newlen, %s%s, (t-data[0] = '5') ? 19 : 20,
   (char *) t-data);
   if (i == -1 || i = newlen) {
 - ASN1_STRING_free(ret);
 + M_ASN1_GENERALIZEDTIME_free(ret);
 + if (out  *out == ret)
 + *out = NULL;
   return NULL;
   }

The fact that *out can be released here seems a bit unexpected, but I
can't figure out how to get i = newlen for this case anyway.

 +done:
 + if (out)
 + *out = ret;
   return ret;
 +err:
 + if (out != NULL  ret != *out)
 + ASN1_GENERALIZEDTIME_free(ret);
 + return NULL;
  }
  
  int
 

Re: NOINET6 by default

2014-05-14 Thread Henning Brauer
* Reyk Flöter reyk.floe...@googlemail.com [2014-05-15 01:04]:
  On 15.05.2014, at 00:46, Henning Brauer lists-openbsdt...@bsws.de wrote:
  * Mark Kettenis mark.kette...@xs4all.nl [2014-05-15 00:15]:
  I don't think this is a good idea; didn't we establish the other day
  that ifconfig if eui64 already did what your +inet6 does?
  almost, it's ifconfig if inet6 eui64 - but that isn't all THAT
  intuitive. I like +inet6 as the opposite of -inet6.
 We don't have + something. It is foo or -foo but not +foo. I know that 
 inet6 is already used for the regular addresses, but +inet6 sounds like an 
 inconsistent workaround for a workaround. I don't like it.

just inet6 doesn't work, since that is already used to show all inet6
addrs. 
i find +inet6 very intuitive...

 To enable IPv6 link-local I would rather prefer two options to put
 either inet6 eui64 (or an alias like inet6 link-local) or an actual
 inet6 address in your hostname.if. The latter should automatically
 remove the flag and enable the link-local address - does it work this
 way? 

as said many times, yes it does.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual  Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: What platforms grow the stack upwards?

2014-05-14 Thread Miod Vallat
 I have not found a complete list with this information. Can someone tell
 me what OpenBSD platforms grow the stack upwards?

grep MACHINE_STACK_GROWS_UP /usr/src/sys/arch/*/include/param.h



Re: NOINET6 by default

2014-05-14 Thread Todd T. Fries
Penned by Henning Brauer on 20140514 22:48.16, we have:
| * Reyk Flöter reyk.floe...@googlemail.com [2014-05-15 01:04]:
|   On 15.05.2014, at 00:46, Henning Brauer lists-openbsdt...@bsws.de wrote:
|   * Mark Kettenis mark.kette...@xs4all.nl [2014-05-15 00:15]:
|   I don't think this is a good idea; didn't we establish the other day
|   that ifconfig if eui64 already did what your +inet6 does?
|   almost, it's ifconfig if inet6 eui64 - but that isn't all THAT
|   intuitive. I like +inet6 as the opposite of -inet6.
|  We don't have + something. It is foo or -foo but not +foo. I know that 
inet6 is already used for the regular addresses, but +inet6 sounds like an 
inconsistent workaround for a workaround. I don't like it.
| 
| just inet6 doesn't work, since that is already used to show all inet6
| addrs. 
| i find +inet6 very intuitive...
| 
|  To enable IPv6 link-local I would rather prefer two options to put
|  either inet6 eui64 (or an alias like inet6 link-local) or an actual
|  inet6 address in your hostname.if. The latter should automatically
|  remove the flag and enable the link-local address - does it work this
|  way? 
| 
| as said many times, yes it does.

I ack that it is a security risk to auto address interfaces without some admin
action.

The proposed solution seems sound, 'inet6 eui64' seems sane.  In theory it
should work, but I must be doing something wrong:

 # ifconfig vether0 create
 # ifconfig vether0 -inet6
 # ifconfig vether0 inet6 eui64
 ifconfig: could not determine link local address

Once that works properly, I say we let the diff in and bikeshed if we
truly need to invent more syntax ('+inet6') that is unlike anything else
vs let the few of us that want this apparently obsecure case add 'inet6
eui64' and be done with it.

Aka, lets not hold up the rest of the functionality just because we
can't agree if we need a further diff to make 'inet6 eui64'
better/faster/easier/another way to skin the cat...

IMHO, its time to polish in the tree.  This is, afterall, a _security_
related diff, no?

Thanks,
-- 
Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries



Re: recvmsg, fd passing and soreceive

2014-05-14 Thread Jérémie Courrèges-Anglas
Matthew Dempsky matt...@dempsky.org writes:

 On Wed, May 14, 2014 at 4:58 AM, Jérémie Courrèges-Anglas
 j...@wxcvbn.org wrote:
 if (cmsg-cmsg_len == CMSG_LEN(sizeof(int)) 
 cmsg-cmsg_level == SOL_SOCKET 
 cmsg-cmsg_type == SCM_RIGHTS) {
 passed_fd = *(int *)CMSG_DATA(cmsg);

 In your test program you only ever send one FD, so this is fine; but
 in general multiple FDs can be sent at a time, and you'll receive a
 single SOL_SOCKET/SCM_RIGHTS control message with an array of N ints.

 E.g., see 
 https://code.google.com/p/chromium/codesearch#chromium/src/base/posix/unix_domain_socket_linux.ccl=130.

Yup, I was trying to emulate the behavior of a a father process which
sends its child one socket to consume, from time to time.

 So the two issues show up when you want to call sendmsg and recvmsg
 with no data at all, but only control messages.  sendmsg allocates an
 empty mbuf for data (m_len == 0), this mbuf is not discarded and freed
 by soreceive for SOCK_STREAM sockets, and stays in the socket buffer.

 I understand that this API was probably not designed to pass only
 control messages, and no data at all.  But it is easily fixable, and the
 manpage could provide portability information.

 Blah, I'm not really a fan of allowing sending zero-length data
 messages.  It makes it tricky for receivers to tell if they actually
 received an EOF or not.

For SOCK_DGRAM / SOCK_SEQPACKET, a zero-length message is fine.

For SOCK_STREAM, it sounds weird to wake up the listening process when
it's going to receive 0 as a return value, indeed.

 But if we're going to allow it (which we do currently it seems), then
 your diff seems like a reasonable fix to me.  mbufs aren't my
 specialty though. =/

I'm not saying it's reasonable.  If people have a clearer view of the
situation than me, I'd be glad to hear it.  I lack available brain cells
these days...  and after all it's not really an issue, just a funny
corner case.

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



Re: libcrypto: stop some ASN.1 bleeding

2014-05-14 Thread Miod Vallat
  ... or, in other words, try to fix most memory leak upon failure.
  This kind of change is difficult to test, the more eyes reviewing it,
  the better.
 
 Well, I'll try to take a stab at it then.

 You might be able to use ASN1_STRING_set(ret, NULL, len) to handle
 resizing / allocating the string the same way that it's used in
 ASN1_TIME_to_generalizedtime(). Alternately, you might want to at least
 use 'p = realloc(ret-data, len)' instead of malloc + free.

I'm trying to stick to the existing logic and only plug the memory leaks
here. Code factoring can happen as a later diff (in other words: I'm
writing this down on the list).

 Looks like there's a bit of an oversight here -- ret isn't referred to
 after this point in the function.

Oops, you're right.

I'll send an updated diff this evening.

Miod



Re: NOINET6 by default

2014-05-14 Thread Jérémie Courrèges-Anglas
Paul de Weerd we...@weirdnet.nl writes:

[...]

 This makes a whole lot of sense to me.  Please make OpenBSD the first
 OS to do (this part of) v6 in a sensible way.

Actually that makes me laugh.  Sensible, weeeh.  Let's protect our users
from the dangers of IPv6 link-local addresses!  Wait, what users are we
talking about?  We have no users, right, we have developers that break
stuff, and other developers that clean up the feces.  Between +inet6 and
eui64 all we have are non-idiomatic or broken alternatives.

Link-local addresses have been exposed since almost 15 years now, it's
a good time to decide that they are persona non grata in OpenBSD land.
After all, the v6 stack has had all the testing it could get, now that
we know that it works well, we don't need testing anymore.

What could be sensible, though, is accepting the fact that IPv6 exists,
and the fact that link-local addresses are part of it, whether you have
global connectivity or not.  Now call me a v6 zealot, I probably live
and work in an imaginary world.

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



Re: recvmsg, fd passing and soreceive

2014-05-14 Thread Philip Guenther
On Wed, May 14, 2014 at 4:58 AM, Jérémie Courrèges-Anglas 
j...@wxcvbn.orgwrote:
...

 So here's the diff I came up with:
 - set MSG_TRUNC for atomic protocols only if there is actually a data
   loss
 - drop the remaining mbuf(s) if the protocol is atomic *or* it is an
   empty message

 I'm running this, and I can't see the downsides, but I could use eyes
 and comments.

 Index: kern/uipc_socket.c
 ===
 RCS file: /cvs/src/sys/kern/uipc_socket.c,v
 retrieving revision 1.127
 diff -u -p -r1.127 uipc_socket.c
 --- kern/uipc_socket.c  7 Apr 2014 10:04:17 -   1.127
 +++ kern/uipc_socket.c  14 May 2014 02:56:09 -
 @@ -935,13 +935,15 @@ dontblock:
 }
 }

 -   if (m  pr-pr_flags  PR_ATOMIC) {
 +   if (m != NULL  m-m_len != 0  pr-pr_flags  PR_ATOMIC)
 +   /* Drop the remaining data later. */
 flags |= MSG_TRUNC;
 -   if ((flags  MSG_PEEK) == 0)
 -   (void) sbdroprecord(so-so_rcv);
 -   }
 +


bikeshedWith the comment 'inside' the 'if', I would probably keep the
braces./bikeshed


if ((flags  MSG_PEEK) == 0) {
 -   if (m == NULL) {
 +   if (m != NULL) {
 +   if (m-m_len == 0 || pr-pr_flags  PR_ATOMIC)
 +   sbdroprecord(so-so_rcv);
 +   } else {
 /*
  * First part is an inline SB_EMPTY_FIXUP().
  Second
  * part makes sure sb_lastrecord is up-to-date if




Re: recvmsg, fd passing and soreceive

2014-05-14 Thread Philip Guenther
On Wed, May 14, 2014 at 10:02 PM, Philip Guenther guent...@gmail.comwrote:

 On Wed, May 14, 2014 at 4:58 AM, Jérémie Courrèges-Anglas 
 j...@wxcvbn.orgwrote:
 ...

 So here's the diff I came up with:
 - set MSG_TRUNC for atomic protocols only if there is actually a data
   loss
 - drop the remaining mbuf(s) if the protocol is atomic *or* it is an
   empty message

 I'm running this, and I can't see the downsides, but I could use eyes
 and comments.

 Index: kern/uipc_socket.c
 ===
 RCS file: /cvs/src/sys/kern/uipc_socket.c,v
 retrieving revision 1.127
 diff -u -p -r1.127 uipc_socket.c
 --- kern/uipc_socket.c  7 Apr 2014 10:04:17 -   1.127
 +++ kern/uipc_socket.c  14 May 2014 02:56:09 -
 @@ -935,13 +935,15 @@ dontblock:
 }
 }

 -   if (m  pr-pr_flags  PR_ATOMIC) {
 +   if (m != NULL  m-m_len != 0  pr-pr_flags  PR_ATOMIC)
 +   /* Drop the remaining data later. */
 flags |= MSG_TRUNC;
 -   if ((flags  MSG_PEEK) == 0)
 -   (void) sbdroprecord(so-so_rcv);
 -   }
 +


 bikeshedWith the comment 'inside' the 'if', I would probably keep the
 braces./bikeshed


 if ((flags  MSG_PEEK) == 0) {
 -   if (m == NULL) {
 +   if (m != NULL) {
 +   if (m-m_len == 0 || pr-pr_flags  PR_ATOMIC)
 +   sbdroprecord(so-so_rcv);
 +   } else {
 /*
  * First part is an inline SB_EMPTY_FIXUP().
  Second
  * part makes sure sb_lastrecord is up-to-date if


...and that's an ok guenther@. I would suggest prodding claudio@ for good
measure though.


Philip


libedit patch

2014-05-14 Thread Eitan Adler
Hi all,

The following patch has been committed in various forms to NetBSD and
FreeBSD.  I ported it to OpenBSD and 'mildly' tested it.

I'm hoping someone on this list could pick this patch up, make sure it
fully conforms to OpenBSD style, and commits it.

For additional context: this patch enables saving history when run in
the capsicum sandbox.  In particular new Fds can't be opened after
cap_enter( ) has been called.

Index: editline.3
===
RCS file: /cvs/src/lib/libedit/editline.3,v
retrieving revision 1.33
diff -u -p -r1.33 editline.3
--- editline.3  10 Jan 2013 14:21:47 -  1.33
+++ editline.3  15 May 2014 05:19:18 -
@@ -735,6 +735,11 @@ Load the history list stored in
 .It Dv H_SAVE , Fa const char *file
 Save the history list to
 .Fa file .
+.It Dv H_SAVE_FP , Fa FILE *fp
+Save the history list to the opened
+.Fa fp
+.Ft FILE
+pointer .
 .It Dv H_SETUNIQUE , Fa int unique
 Set flag that adjacent identical event strings should not be entered
 into the history.
Index: hist.h
===
RCS file: /cvs/src/lib/libedit/hist.h,v
retrieving revision 1.8
diff -u -p -r1.8 hist.h
--- hist.h  30 Jun 2010 00:05:35 -  1.8
+++ hist.h  15 May 2014 05:19:18 -
@@ -1,5 +1,5 @@
 /* $OpenBSD: hist.h,v 1.8 2010/06/30 00:05:35 nicm Exp $   */
-/* $NetBSD: hist.h,v 1.12 2009/12/30 23:54:52 christos Exp $   */
+/* $NetBSD: hist.h,v 1.14 2014/05/11 01:05:17 christos Exp $   */

 /*-
  * Copyright (c) 1992, 1993
@@ -74,6 +74,7 @@ typedef struct el_history_t {
 #defineHIST_SET(el, num)   HIST_FUN(el, H_SET, num)
 #defineHIST_LOAD(el, fname)HIST_FUN(el, H_LOAD fname)
 #defineHIST_SAVE(el, fname)HIST_FUN(el, H_SAVE fname)
+#defineHIST_SAVE_FP(el, fp)HIST_FUN(el, H_SAVE_FP fp)

 protected int  hist_init(EditLine *);
 protected void hist_end(EditLine *);
Index: histedit.h
===
RCS file: /cvs/src/lib/libedit/histedit.h,v
retrieving revision 1.11
diff -u -p -r1.11 histedit.h
--- histedit.h  7 Jul 2011 05:40:42 -   1.11
+++ histedit.h  15 May 2014 05:19:19 -
@@ -1,5 +1,5 @@
 /* $OpenBSD: histedit.h,v 1.11 2011/07/07 05:40:42 okan Exp $  */
-/* $NetBSD: histedit.h,v 1.46 2010/04/15 00:50:03 christos Exp $   */
+/* $NetBSD: histedit.h,v 1.52 2014/05/11 01:05:17 christos Exp $   */

 /*-
  * Copyright (c) 1992, 1993
@@ -225,6 +225,7 @@ int history(History *, HistEvent *, int
 #defineH_NEXT_EVDATA   23  /* , const int, histdata_t *);  */
 #defineH_DELDATA   24  /* , int, histdata_t *);*/
 #defineH_REPLACE   25  /* , const char *, histdata_t); */
+#defineH_SAVE_FP   26  /* , FILE *);   */



Index: history.c
===
RCS file: /cvs/src/lib/libedit/history.c,v
retrieving revision 1.17
diff -u -p -r1.17 history.c
--- history.c   19 Jan 2014 11:48:54 -  1.17
+++ history.c   15 May 2014 05:19:20 -
@@ -1,5 +1,5 @@
 /* $OpenBSD: history.c,v 1.17 2014/01/19 11:48:54 tobias Exp $ */
-/* $NetBSD: history.c,v 1.37 2010/01/03 18:27:10 christos Exp $*/
+/* $NetBSD: history.c,v 1.47 2014/05/11 01:05:17 christos Exp $*/

 /*-
  * Copyright (c) 1992, 1993
@@ -103,6 +103,7 @@ private int history_getunique(TYPE(Histo
 private int history_set_fun(TYPE(History) *, TYPE(History) *);
 private int history_load(TYPE(History) *, const char *);
 private int history_save(TYPE(History) *, const char *);
+private int history_save_fp(TYPE(History) *, FILE *);
 private int history_prev_event(TYPE(History) *, TYPE(HistEvent) *, int);
 private int history_next_event(TYPE(History) *, TYPE(HistEvent) *, int);
 private int history_next_string(TYPE(History) *, TYPE(HistEvent) *,
const Char *);
@@ -784,13 +785,12 @@ done:
 }


-/* history_save():
+/* history_save_fp():
  * TYPE(History) save function
  */
 private int
-history_save(TYPE(History) *h, const char *fname)
+history_save_fp(TYPE(History) *h, FILE *fp)
 {
-   FILE *fp;
TYPE(HistEvent) ev;
int i = -1, retval;
size_t len, max_size;
@@ -799,9 +799,6 @@ history_save(TYPE(History) *h, const cha
static ct_buffer_t conv;
 #endif

-   if ((fp = fopen(fname, w)) == NULL)
-   return (-1);
-
if (fchmod(fileno(fp), S_IRUSR|S_IWUSR) == -1)
goto done;
if (fputs(hist_cookie, fp) == EOF)
@@ -830,10 +827,22 @@ history_save(TYPE(History) *h, const cha
 oomem:
h_free((ptr_t)ptr);
 done:
-   (void) fclose(fp);
return (i);
 }

+private int
+history_save(TYPE(History) *h, const char *fname)
+{
+   FILE *fp;
+   int i;
+   if ((fp = fopen(fname, w)) == NULL)
+   return -1;
+   i = history_save_fp(h, fp);
+