Re: axen Ethernet device errors on both USB3.0 and USB2.0 ports

2018-08-06 Thread sc . dying

Hi,

On 2018/08/05 15:31, Denis wrote:

Can not patch original files in /usr/src/sys/dev/usb/ despite that
if_axenreg.h and if_axen.c files fetched directly from current CVS repo.


Previous patch is for -current, but perhaps your source version is 6.3.
Please try the new patch below for 6.3.

The dmesg in the report says "OpenBSD 6.3", I missed it.
Sorry about that.



# patch -p0 < axen.patch
Hmm.. Lookd like a unified diff to me...
The text leading up to this was:
-
|--- sys/dev/usb/if_axenreg.h Fri Sep 16 22:17:07 2016
|+++ sys/dev/usb/if_axenreg.h Mon Jun 19 10:54:28 2017
-
Patching file sys/dev/usb/if_axenreg.h using Plan A...
Hunk #1 succeeded at 26.
Hunk #2 failed at 70.
1 out of 2 hunks failed--saving rejects to sys/dev/usb/if_axenreg.h.rej
Hmm... The next patch look like a unified diff to me...
The text leading up to this was:
-
|--- sys/dev/usb/if_axen.c.orig Tue Jun 12 15:36:59 2018
|+++ sys/dev/usb/if_axen.c Sun Jul 29 01:53:43 2018
-
Patching file sys/dev/usb/if_axen.c using Plan A...
Hunk #1 succeeded at 53.
Hunk #2 succeeded at 122 with fuzz 2.
Hunk #3 failed at 246.
Hunk #4 failed at 275.
Hunk #5 failed at 440.
Hunk #6 failed at 502.
Hunk #7 failed at 515.
Hunk #8 failed at 637.
Hunk #9 failed at 711.
Hunk #10 succeeded at 810 with fuzz 1.
Hunk #11 failed at 846.
Hunk #12 failed at 877.
Hunk #13 failed at 907.
Hunk #14 failed at 932.
Hunk #15 failed at 955.
Hunk #16 failed at 975.
Hunk #17 failed at 996.
Hunk #18 failed at 1029.
Hunk #19 failed at 1054.
16 out of 19 hunks failed--saving rejects to sys/dev/usb/if_axen.c.rej
done


 - Only qctrl and buffer allocation are changed.
   (Omit some non-essential parts. It should work.)


--- sys/dev/usb/if_axen.c.orig  Sun Jan 22 10:17:39 2017
+++ sys/dev/usb/if_axen.c   Mon Aug  6 23:20:25 2018
@@ -53,6 +53,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -121,6 +122,13 @@ void	axen_unlock_mii(struct axen_softc *sc);
 
 void	axen_ax88179_init(struct axen_softc *);
 
+struct axen_qctrl axen_bulk_size[] = {

+   { 7, 0x4f, 0x00, 0x12, 0xff },
+   { 7, 0x20, 0x03, 0x16, 0xff },
+   { 7, 0xae, 0x07, 0x18, 0xff },
+   { 7, 0xcc, 0x4c, 0x18, 0x08 }
+};
+
 /* Get exclusive access to the MII registers */
 void
 axen_lock_mii(struct axen_softc *sc)
@@ -238,6 +246,8 @@ axen_miibus_statchg(struct device *dev)
int err;
uint16_tval;
uWord   wval;
+   uint8_t linkstat = 0;
+   int qctrl;
 
 	ifp = GET_IFP(sc);

if (mii == NULL || ifp == NULL ||
@@ -265,27 +275,49 @@ axen_miibus_statchg(struct device *dev)
return;
 
 	val = 0;

-   if ((IFM_OPTIONS(mii->mii_media_active) & IFM_FDX) != 0)
+   if ((IFM_OPTIONS(mii->mii_media_active) & IFM_FDX) != 0) {
val |= AXEN_MEDIUM_FDX;
+   if ((IFM_OPTIONS(mii->mii_media_active) & IFM_ETH_TXPAUSE) != 0)
+   val |= AXEN_MEDIUM_TXFLOW_CTRL_EN;
+   if ((IFM_OPTIONS(mii->mii_media_active) & IFM_ETH_RXPAUSE) != 0)
+   val |= AXEN_MEDIUM_RXFLOW_CTRL_EN;
+   }
 
-	val |= (AXEN_MEDIUM_RECV_EN | AXEN_MEDIUM_ALWAYS_ONE);

-   val |= (AXEN_MEDIUM_RXFLOW_CTRL_EN | AXEN_MEDIUM_TXFLOW_CTRL_EN);
+   val |= AXEN_MEDIUM_RECV_EN;
 
+	/* bulkin queue setting */

+   axen_lock_mii(sc);
+   axen_cmd(sc, AXEN_CMD_MAC_READ, 1, AXEN_USB_UPLINK, );
+   axen_unlock_mii(sc);
+
switch (IFM_SUBTYPE(mii->mii_media_active)) {
case IFM_1000_T:
val |= AXEN_MEDIUM_GIGA | AXEN_MEDIUM_EN_125MHZ;
+   if (linkstat & AXEN_USB_SS)
+   qctrl = 0;
+   else if (linkstat & AXEN_USB_HS)
+   qctrl = 1;
+   else
+   qctrl = 3;
break;
case IFM_100_TX:
val |= AXEN_MEDIUM_PS;
+   if (linkstat & (AXEN_USB_SS | AXEN_USB_HS))
+   qctrl = 2;
+   else
+   qctrl = 3;
break;
case IFM_10_T:
-   /* doesn't need to be handled */
+   default:
+   qctrl = 3;
break;
}
 
 	DPRINTF(("axen_miibus_statchg: val=0x%x\n", val));

USETW(wval, val);
axen_lock_mii(sc);
+   axen_cmd(sc, AXEN_CMD_MAC_SET_RXSR, 5, AXEN_RX_BULKIN_QCTRL,
+   _bulk_size[qctrl]);
err = axen_cmd(sc, AXEN_CMD_MAC_WRITE2, 2, AXEN_MEDIUM_STATUS, );
axen_unlock_mii(sc);
if (err) {
@@ -408,7 +440,6 @@ axen_ax88179_init(struct axen_softc *sc)
uWord   wval;
uByte   val;
u_int16_t   ctl, temp;
-   struct axen_qctrl qctrl;
 
 	axen_lock_mii(sc);
 
@@ -470,34 +501,18 @@ axen_ax88179_init(struct axen_softc *sc)

switch (val) {

Re: slowcgi -u user option does not change socket ownership

2018-08-06 Thread Andrew Daugherity
On Fri, Aug 3, 2018 at 2:34 AM Florian Obser  wrote:
>
> On Thu, Aug 02, 2018 at 04:22:35PM -0500, Andrew Daugherity wrote:
> > I'm also playing around with a version that accepts "-U user:group" (a
> > la chown), which I think would be preferable to a separate group
>
> Eww, do you really have a usecase for this? It seems like you would
> only need this if you want to access to fcgi socket from two daemons
> running as different users at the same time.
>
> I'd prefer to keep the code simple and not have it.

Not currently...  I threw this together as a proof-of-concept thinking
of potential cases where someone might want e.g. nginx:www vs.
nginx:nginx, but as you pointed out, there's no good reason to do that
for the socket.  Maybe for the CGI script user option, but there are
other ways to solve that, e.g. a setgid directory.

So, forget about the user:group parsing then.  I agree: it's not worth
the hassle.



Re: fcntl(2) manpage is wrong about F_GETFD return value

2018-08-06 Thread Matthieu Herrb
On Mon, Aug 06, 2018 at 06:32:01PM +0200, Claudio Jeker wrote:
> On Mon, Aug 06, 2018 at 06:23:28PM +0200, Matthieu Herrb wrote:
> > Hi,
> > 
> > fcntl(2) says:
> > 
> >  F_GETFD  Get the close-on-exec flag associated with the file
> >   descriptor fd as FD_CLOEXEC.  If the returned value
> >   ANDed with FD_CLOEXEC is 0, the file will remain open
> >   across exec(), otherwise the file will be closed upon
> >   execution of exec() (arg is ignored).
> > 
> > This is wrong. The flag is returned in the low bit of the return
> > value (see sys/kern/kern_descrip.c:443).
> > Moreover the value O_CLOEXEC is not 0x1, so doing the AND described
> > above is never going to provide the good answer.
> > 
> > I've checked that Linux and FreeBSD also return 0x1 when the O_CLOEXEC
> > is set, and none of them defines O_CLOEXEC as 0x1. The same
> > documentation bug is present on FreenBSD.
> > 
> > How to repeat:
> > 
> > #include 
> > #include 
> > #include 
> > 
> > int
> > main(int argc, char *argv[])
> > {
> > int fd, flags;
> > 
> > fd = open("/dev/null", O_RDWR | O_CLOEXEC);
> > if (fd < 0)
> > err(2, "open");
> > flags = fcntl(fd, F_GETFD, 0);
> > if (flags < 0)
> > err(2, "fcntl");
> > printf("flags 0x%08x O_CLOEXEC 0x%08x\n", flags, O_CLOEXEC);
> > return 0;
> > }
> 
> There is a difference between FD_CLOEXEC and O_CLOEXEC. FD_CLOEXEC is
> defined as 1 and so the man page is correct.

Ok. This is confusing but you're correct. sigh.
> 
>  
> > Patch
> > 
> > The patch below fixes the documentation.
> > 
> > Index: fcntl.2
> > ===
> > RCS file: /cvs/OpenBSD/src/lib/libc/sys/fcntl.2,v
> > retrieving revision 1.31
> > diff -u -p -u -r1.31 fcntl.2
> > --- fcntl.2 16 Dec 2014 00:06:49 -  1.31
> > +++ fcntl.2 6 Aug 2018 16:10:53 -
> > @@ -100,10 +100,7 @@ Get the close-on-exec flag associated wi
> >  .Fa fd
> >  as
> >  .Dv FD_CLOEXEC .
> > -If the returned value ANDed with
> > -.Dv FD_CLOEXEC
> > -is 0,
> > -the file will remain open across
> > +If the returned value is 0, the file will remain open across
> >  .Fn exec ,
> >  otherwise the file will be closed upon execution of
> >  .Fn exec
> > 
> > 
> 
> 
> -- 
> :wq Claudio

-- 
Matthieu Herrb



Re: fcntl(2) manpage is wrong about F_GETFD return value

2018-08-06 Thread Claudio Jeker
On Mon, Aug 06, 2018 at 06:23:28PM +0200, Matthieu Herrb wrote:
> Hi,
> 
> fcntl(2) says:
> 
>  F_GETFD  Get the close-on-exec flag associated with the file
>   descriptor fd as FD_CLOEXEC.  If the returned value
>   ANDed with FD_CLOEXEC is 0, the file will remain open
>   across exec(), otherwise the file will be closed upon
>   execution of exec() (arg is ignored).
> 
> This is wrong. The flag is returned in the low bit of the return
> value (see sys/kern/kern_descrip.c:443).
> Moreover the value O_CLOEXEC is not 0x1, so doing the AND described
> above is never going to provide the good answer.
> 
> I've checked that Linux and FreeBSD also return 0x1 when the O_CLOEXEC
> is set, and none of them defines O_CLOEXEC as 0x1. The same
> documentation bug is present on FreenBSD.
> 
> How to repeat:
> 
> #include 
> #include 
> #include 
> 
> int
> main(int argc, char *argv[])
> {
>   int fd, flags;
> 
>   fd = open("/dev/null", O_RDWR | O_CLOEXEC);
>   if (fd < 0)
>   err(2, "open");
>   flags = fcntl(fd, F_GETFD, 0);
>   if (flags < 0)
>   err(2, "fcntl");
>   printf("flags 0x%08x O_CLOEXEC 0x%08x\n", flags, O_CLOEXEC);
>   return 0;
> }

There is a difference between FD_CLOEXEC and O_CLOEXEC. FD_CLOEXEC is
defined as 1 and so the man page is correct.

 
> Patch
> 
> The patch below fixes the documentation.
> 
> Index: fcntl.2
> ===
> RCS file: /cvs/OpenBSD/src/lib/libc/sys/fcntl.2,v
> retrieving revision 1.31
> diff -u -p -u -r1.31 fcntl.2
> --- fcntl.2   16 Dec 2014 00:06:49 -  1.31
> +++ fcntl.2   6 Aug 2018 16:10:53 -
> @@ -100,10 +100,7 @@ Get the close-on-exec flag associated wi
>  .Fa fd
>  as
>  .Dv FD_CLOEXEC .
> -If the returned value ANDed with
> -.Dv FD_CLOEXEC
> -is 0,
> -the file will remain open across
> +If the returned value is 0, the file will remain open across
>  .Fn exec ,
>  otherwise the file will be closed upon execution of
>  .Fn exec
> 
> 


-- 
:wq Claudio



fcntl(2) manpage is wrong about F_GETFD return value

2018-08-06 Thread Matthieu Herrb
Hi,

fcntl(2) says:

 F_GETFD  Get the close-on-exec flag associated with the file
  descriptor fd as FD_CLOEXEC.  If the returned value
  ANDed with FD_CLOEXEC is 0, the file will remain open
  across exec(), otherwise the file will be closed upon
  execution of exec() (arg is ignored).

This is wrong. The flag is returned in the low bit of the return
value (see sys/kern/kern_descrip.c:443).
Moreover the value O_CLOEXEC is not 0x1, so doing the AND described
above is never going to provide the good answer.

I've checked that Linux and FreeBSD also return 0x1 when the O_CLOEXEC
is set, and none of them defines O_CLOEXEC as 0x1. The same
documentation bug is present on FreenBSD.

How to repeat:

#include 
#include 
#include 

int
main(int argc, char *argv[])
{
int fd, flags;

fd = open("/dev/null", O_RDWR | O_CLOEXEC);
if (fd < 0)
err(2, "open");
flags = fcntl(fd, F_GETFD, 0);
if (flags < 0)
err(2, "fcntl");
printf("flags 0x%08x O_CLOEXEC 0x%08x\n", flags, O_CLOEXEC);
return 0;
}

Patch

The patch below fixes the documentation.

Index: fcntl.2
===
RCS file: /cvs/OpenBSD/src/lib/libc/sys/fcntl.2,v
retrieving revision 1.31
diff -u -p -u -r1.31 fcntl.2
--- fcntl.2 16 Dec 2014 00:06:49 -  1.31
+++ fcntl.2 6 Aug 2018 16:10:53 -
@@ -100,10 +100,7 @@ Get the close-on-exec flag associated wi
 .Fa fd
 as
 .Dv FD_CLOEXEC .
-If the returned value ANDed with
-.Dv FD_CLOEXEC
-is 0,
-the file will remain open across
+If the returned value is 0, the file will remain open across
 .Fn exec ,
 otherwise the file will be closed upon execution of
 .Fn exec


-- 
Matthieu Herrb



Re: bgpd crash with invalid bgpctl network bulk del

2018-08-06 Thread Sebastian Benoit
ok benno@

Claudio Jeker(clau...@openbsd.org) on 2018.08.06 15:50:41 +0200:
> On Mon, Aug 06, 2018 at 03:34:10PM +0200, Claudio Jeker wrote:
> > On Mon, Aug 06, 2018 at 03:26:16PM +0200, Pierre Emeriaud wrote:
> > > Not sure about this, I guess if you're using 'network bulk add|del'
> > > you're supposed to know what you're doing and not shoot yourself in
> > > the foot, but here it goes:
> > > 
> > > echo | bgpctl network bulk del
> > > crashes bgpd. On 6.3 and current as of today at least.
> > > 
> > > $ doas bgpd -dvf bgpd.conf
> > > startup
> > > rereading config
> > > route decision engine ready
> > > session engine ready
> > > myas = "64751"
> > > ktable_new: rdomain_0 for rtableid 0
> > > listening on 127.0.0.1
> > > SE reconfigured
> > > RDE reconfigured
> > > 
> > >  bgpd started, time to run 'echo | bgpctl network bulk del'
> > > 
> > > fatal in RDE: pt_fill: unknown af
> > > peer closed imsg connection
> > > main: Lost connection to RDE
> > > peer closed imsg connection
> > > SE: Lost connection to parent
> > > session engine exiting
> > > kernel routing table 0 (Loc-RIB) decoupled
> > > ktable_destroy: freeing ktable Loc-RIB rtableid 0
> > > waiting for children to terminate
> > > terminating
> > > 
> > > 
> > > This somewhat fixes the empty line issue, but it's still possible to
> > > crash with any other string not beeing an ip prefix. A better
> > > workaround would be parse prefixes beforehand, or maybe just don't be
> > > a moron and don't feed bgpctl with anything stupid.
> > > 
> > > Index: bgpctl.c
> > > ===
> > > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> > > retrieving revision 1.210
> > > diff -u -p -r1.210 bgpctl.c
> > > --- bgpctl.c29 Jul 2018 13:02:01 -  1.210
> > > +++ bgpctl.c6 Aug 2018 13:11:23 -
> > > @@ -1978,6 +1978,9 @@ network_bulk(struct parse_result *res)
> > > /* Don't process commented entries */
> > > if (strchr(b, '#') != NULL)
> > > break;
> > > +   /* Don't process empty lines */
> > > +   if (strcmp(b, "\0") == 0)
> > > +   break;
> > > bzero(, sizeof(net));
> > > parse_prefix(b, strlen(b), , );
> > > net.prefix = h;
> > > 
> > 
> > Thanks for the diff and the report. I will look into this.
> > 
> 
> Here a diff to protect bgpd a bit better. It uses the same logic as for
> the IMSG_NETWORK_ADD case and does a very basic check of the prefix passed
> in.
> 
> Skipping empty lines in bgpctl is also something we should do but that
> code needs some rework.
> 
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.410
> diff -u -p -r1.410 rde.c
> --- rde.c 6 Aug 2018 08:13:31 -   1.410
> +++ rde.c 6 Aug 2018 13:47:12 -
> @@ -527,7 +527,7 @@ rde_dispatch_imsg_session(struct imsgbuf
>   break;
>   default:
>  badnet:
> - log_warnx("rde_dispatch: bad network");
> + log_warnx("request to insert invalid network");
>   break;
>   }
>   break;
> @@ -539,7 +539,23 @@ badnet:
>   }
>   memcpy(_s, imsg.data, sizeof(netconf_s));
>   TAILQ_INIT(_s.attrset);
> - network_delete(_s, 0);
> +
> + switch (netconf_s.prefix.aid) {
> + case AID_INET:
> + if (netconf_s.prefixlen > 32)
> + goto badnetdel;
> + network_delete(_s, 0);
> + break;
> + case AID_INET6:
> + if (netconf_s.prefixlen > 128)
> + goto badnetdel;
> + network_delete(_s, 0);
> + break;
> + default:
> +badnetdel:
> + log_warnx("request to remove invalid network");
> + break;
> + }
>   break;
>   case IMSG_NETWORK_FLUSH:
>   if (imsg.hdr.len != IMSG_HEADER_SIZE) {
> 



Re: bgpd crash with invalid bgpctl network bulk del

2018-08-06 Thread Pierre Emeriaud
2018-08-06 15:50 GMT+02:00 Claudio Jeker :
>
> Here a diff to protect bgpd a bit better. It uses the same logic as for
> the IMSG_NETWORK_ADD case and does a very basic check of the prefix passed
> in.
>
> Skipping empty lines in bgpctl is also something we should do but that
> code needs some rework.

Works for me with empty lines, garbage or a file that has both
multiple prefixes, comments and empty lines, with stock bgpctl. Thanks
for the quick fix Claudio!



Re: bgpd crash with invalid bgpctl network bulk del

2018-08-06 Thread Claudio Jeker
On Mon, Aug 06, 2018 at 03:34:10PM +0200, Claudio Jeker wrote:
> On Mon, Aug 06, 2018 at 03:26:16PM +0200, Pierre Emeriaud wrote:
> > Not sure about this, I guess if you're using 'network bulk add|del'
> > you're supposed to know what you're doing and not shoot yourself in
> > the foot, but here it goes:
> > 
> > echo | bgpctl network bulk del
> > crashes bgpd. On 6.3 and current as of today at least.
> > 
> > $ doas bgpd -dvf bgpd.conf
> > startup
> > rereading config
> > route decision engine ready
> > session engine ready
> > myas = "64751"
> > ktable_new: rdomain_0 for rtableid 0
> > listening on 127.0.0.1
> > SE reconfigured
> > RDE reconfigured
> > 
> >  bgpd started, time to run 'echo | bgpctl network bulk del'
> > 
> > fatal in RDE: pt_fill: unknown af
> > peer closed imsg connection
> > main: Lost connection to RDE
> > peer closed imsg connection
> > SE: Lost connection to parent
> > session engine exiting
> > kernel routing table 0 (Loc-RIB) decoupled
> > ktable_destroy: freeing ktable Loc-RIB rtableid 0
> > waiting for children to terminate
> > terminating
> > 
> > 
> > This somewhat fixes the empty line issue, but it's still possible to
> > crash with any other string not beeing an ip prefix. A better
> > workaround would be parse prefixes beforehand, or maybe just don't be
> > a moron and don't feed bgpctl with anything stupid.
> > 
> > Index: bgpctl.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> > retrieving revision 1.210
> > diff -u -p -r1.210 bgpctl.c
> > --- bgpctl.c29 Jul 2018 13:02:01 -  1.210
> > +++ bgpctl.c6 Aug 2018 13:11:23 -
> > @@ -1978,6 +1978,9 @@ network_bulk(struct parse_result *res)
> > /* Don't process commented entries */
> > if (strchr(b, '#') != NULL)
> > break;
> > +   /* Don't process empty lines */
> > +   if (strcmp(b, "\0") == 0)
> > +   break;
> > bzero(, sizeof(net));
> > parse_prefix(b, strlen(b), , );
> > net.prefix = h;
> > 
> 
> Thanks for the diff and the report. I will look into this.
> 

Here a diff to protect bgpd a bit better. It uses the same logic as for
the IMSG_NETWORK_ADD case and does a very basic check of the prefix passed
in.

Skipping empty lines in bgpctl is also something we should do but that
code needs some rework.

-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.410
diff -u -p -r1.410 rde.c
--- rde.c   6 Aug 2018 08:13:31 -   1.410
+++ rde.c   6 Aug 2018 13:47:12 -
@@ -527,7 +527,7 @@ rde_dispatch_imsg_session(struct imsgbuf
break;
default:
 badnet:
-   log_warnx("rde_dispatch: bad network");
+   log_warnx("request to insert invalid network");
break;
}
break;
@@ -539,7 +539,23 @@ badnet:
}
memcpy(_s, imsg.data, sizeof(netconf_s));
TAILQ_INIT(_s.attrset);
-   network_delete(_s, 0);
+
+   switch (netconf_s.prefix.aid) {
+   case AID_INET:
+   if (netconf_s.prefixlen > 32)
+   goto badnetdel;
+   network_delete(_s, 0);
+   break;
+   case AID_INET6:
+   if (netconf_s.prefixlen > 128)
+   goto badnetdel;
+   network_delete(_s, 0);
+   break;
+   default:
+badnetdel:
+   log_warnx("request to remove invalid network");
+   break;
+   }
break;
case IMSG_NETWORK_FLUSH:
if (imsg.hdr.len != IMSG_HEADER_SIZE) {



Re: bgpd crash with invalid bgpctl network bulk del

2018-08-06 Thread Claudio Jeker
On Mon, Aug 06, 2018 at 03:26:16PM +0200, Pierre Emeriaud wrote:
> Not sure about this, I guess if you're using 'network bulk add|del'
> you're supposed to know what you're doing and not shoot yourself in
> the foot, but here it goes:
> 
> echo | bgpctl network bulk del
> crashes bgpd. On 6.3 and current as of today at least.
> 
> $ doas bgpd -dvf bgpd.conf
> startup
> rereading config
> route decision engine ready
> session engine ready
> myas = "64751"
> ktable_new: rdomain_0 for rtableid 0
> listening on 127.0.0.1
> SE reconfigured
> RDE reconfigured
> 
>  bgpd started, time to run 'echo | bgpctl network bulk del'
> 
> fatal in RDE: pt_fill: unknown af
> peer closed imsg connection
> main: Lost connection to RDE
> peer closed imsg connection
> SE: Lost connection to parent
> session engine exiting
> kernel routing table 0 (Loc-RIB) decoupled
> ktable_destroy: freeing ktable Loc-RIB rtableid 0
> waiting for children to terminate
> terminating
> 
> 
> This somewhat fixes the empty line issue, but it's still possible to
> crash with any other string not beeing an ip prefix. A better
> workaround would be parse prefixes beforehand, or maybe just don't be
> a moron and don't feed bgpctl with anything stupid.
> 
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.210
> diff -u -p -r1.210 bgpctl.c
> --- bgpctl.c29 Jul 2018 13:02:01 -  1.210
> +++ bgpctl.c6 Aug 2018 13:11:23 -
> @@ -1978,6 +1978,9 @@ network_bulk(struct parse_result *res)
> /* Don't process commented entries */
> if (strchr(b, '#') != NULL)
> break;
> +   /* Don't process empty lines */
> +   if (strcmp(b, "\0") == 0)
> +   break;
> bzero(, sizeof(net));
> parse_prefix(b, strlen(b), , );
> net.prefix = h;
> 

Thanks for the diff and the report. I will look into this.

-- 
:wq Claudio



bgpd crash with invalid bgpctl network bulk del

2018-08-06 Thread Pierre Emeriaud
Not sure about this, I guess if you're using 'network bulk add|del'
you're supposed to know what you're doing and not shoot yourself in
the foot, but here it goes:

echo | bgpctl network bulk del
crashes bgpd. On 6.3 and current as of today at least.

$ doas bgpd -dvf bgpd.conf
startup
rereading config
route decision engine ready
session engine ready
myas = "64751"
ktable_new: rdomain_0 for rtableid 0
listening on 127.0.0.1
SE reconfigured
RDE reconfigured

 bgpd started, time to run 'echo | bgpctl network bulk del'

fatal in RDE: pt_fill: unknown af
peer closed imsg connection
main: Lost connection to RDE
peer closed imsg connection
SE: Lost connection to parent
session engine exiting
kernel routing table 0 (Loc-RIB) decoupled
ktable_destroy: freeing ktable Loc-RIB rtableid 0
waiting for children to terminate
terminating


This somewhat fixes the empty line issue, but it's still possible to
crash with any other string not beeing an ip prefix. A better
workaround would be parse prefixes beforehand, or maybe just don't be
a moron and don't feed bgpctl with anything stupid.

Index: bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.210
diff -u -p -r1.210 bgpctl.c
--- bgpctl.c29 Jul 2018 13:02:01 -  1.210
+++ bgpctl.c6 Aug 2018 13:11:23 -
@@ -1978,6 +1978,9 @@ network_bulk(struct parse_result *res)
/* Don't process commented entries */
if (strchr(b, '#') != NULL)
break;
+   /* Don't process empty lines */
+   if (strcmp(b, "\0") == 0)
+   break;
bzero(, sizeof(net));
parse_prefix(b, strlen(b), , );
net.prefix = h;