Re: iked(8): fix some leaks in parse.y

2019-09-25 Thread Alexander Bluhm
On Wed, Sep 25, 2019 at 10:34:53PM +0200, Tobias Heider wrote:
> Thanks, makes sense. Not sure how I didn't think of this.
> Here is a cleaned up version:

OK bluhm@

> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 parse.y
> --- parse.y   26 Aug 2019 16:41:08 -  1.83
> +++ parse.y   25 Sep 2019 20:28:31 -
> @@ -354,10 +354,13 @@ int  get_id_type(char *);
>  uint8_t   x2i(unsigned char *);
>  int   parsekey(unsigned char *, size_t, struct iked_auth *);
>  int   parsekeyfile(char *, struct iked_auth *);
> +void  iaw_free(struct ipsec_addr_wrap *);
>
>  struct ipsec_transforms *ipsec_transforms;
>  struct ipsec_filters *ipsec_filters;
>  struct ipsec_mode *ipsec_mode;
> +/* interface lookup routintes */
> +struct ipsec_addr_wrap   *iftab;
>
>  typedef struct {
>   union {
> @@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
>   free(sym);
>   }
>
> + iaw_free(iftab);
> + iftab = NULL;
> +
>   return (errors ? -1 : 0);
>  }
>
> @@ -2184,10 +2190,6 @@ host_any(void)
>   return (ipa);
>  }
>
> -/* interface lookup routintes */
> -
> -struct ipsec_addr_wrap   *iftab;
> -
>  void
>  ifa_load(void)
>  {
> @@ -3040,7 +3042,17 @@ done:
>   free(p->prop_xforms);
>   free(p);
>   }
> -
> + if (peers != NULL) {
> + iaw_free(peers->src);
> + iaw_free(peers->dst);
> + /* peers is static, cannot be freed */
> + }
> + if (hosts != NULL) {
> + iaw_free(hosts->src);
> + iaw_free(hosts->dst);
> + free(hosts);
> + }
> + iaw_free(ikecfg);
>   return (ret);
>  }
>
> @@ -3066,4 +3078,24 @@ create_user(const char *user, const char
>
>   rules++;
>   return (0);
> +}
> +
> +void
> +iaw_free(struct ipsec_addr_wrap *head)
> +{
> + struct ipsec_addr_wrap *n, *cur;
> +
> + if (head == NULL)
> + return;
> +
> + for (n = head; n != NULL; ) {
> + cur = n;
> + n = n->next;
> + if (cur->srcnat != NULL) {
> + free(cur->srcnat->name);
> + free(cur->srcnat);
> + }
> + free(cur->name);
> + free(cur);
> + }
>  }



Re: iked(8): improve logging output

2019-09-25 Thread Alexander Bluhm
On Wed, Sep 25, 2019 at 10:45:50PM +0200, Tobias Heider wrote:
> ok?
>
> @@ -4084,8 +4094,8 @@ ikev2_send_informational(struct iked *en
>   case IKEV2_N_NO_PROPOSAL_CHOSEN:
>   break;
>   default:
> - log_debug("%s: unsupported notification %s", __func__,
> - print_map(msg->msg_error, ikev2_n_map));
> + log_info("%s: unsupported notification %s", SPI_SA(sa,
> + __func__), print_map(msg->msg_error, ikev2_n_map));
>   goto done;
>   }
>

Are you sure that sa != NULL?  A few lines below this is checked.
if (sa != NULL && msg->msg_e) {

> @@ -4510,20 +4525,22 @@ ikev2_sa_responder_dh(struct iked_kex *k
>   if (kex->kex_dhgroup == NULL) {
>   if ((xform = config_findtransform(proposals,
>   IKEV2_XFORMTYPE_DH, proto)) == NULL) {
> - log_debug("%s: did not find dh transform", __func__);
> + log_info("%s: did not find dh transform",
> + SPI_SA(msg->msg_sa, __func__));
>   return (-1);
>   }
>   if ((kex->kex_dhgroup =
>   group_get(xform->xform_id)) == NULL) {
> - log_debug("%s: invalid dh %d", __func__,
> - xform->xform_id);
> + log_debug("%s: invalid dh %d",
> + SPI_SA(msg->msg_sa, __func__), xform->xform_id);
>   return (-1);
>   }
>   }
>
>   /* Look for dhgroup mismatch during an IKE SA negotiation */
>   if (msg->msg_dhgroup != kex->kex_dhgroup->id) {
> - log_debug("%s: want dh %s, KE has %s", __func__,
> + log_info("%s: want dh %s, KE has %s",
> + SPI_SA(msg->msg_sa, __func__),
>   print_map(kex->kex_dhgroup->id, ikev2_xformdh_map),
>   print_map(msg->msg_dhgroup, ikev2_xformdh_map));
>   msg->msg_error = IKEV2_N_INVALID_KE_PAYLOAD;

Would it make sense to put struct iked_sa *sa = msg->msg_sa;
into ikev2_sa_responder_dh() like in all other functions?

otherwise OK bluhm@



Re: pretty borders for slitherins

2019-09-25 Thread Scott Cheloha
On Wed, Sep 25, 2019 at 08:29:47PM +0100, Nicholas Marriott wrote:
> It will use either UTF-8 characters or the ACS characters from TERM.
> 
> The way it decides which is somewhat confusing and it looks like it is
> choosing to use UTF-8 even when I wouldn't expect it to so I'm not sure
> it is actually working correctly in our ncurses version.
> 
> I don't think UTF-8 line drawing works properly, at least with the DRM
> wscons.
> 
> What do you have in TERM now? Can you see if the borders are OK with
> "TERM=xterm snake"? This seems to use ACS rather than UTF-8. If it does
> I'll see if I can figure out why it choosing UTF-8 borders for the
> others.

In wscons I have vt220.  The borders are even more screwy with
`TERM=xterm snake` in the wscons.  Now you have pairs of question
marks instead of just a single '?'.

The (maybe not so) odd thing is that in a real xterm if I do
`TERM=vt220 snake` the borders are fine.  So yeah, I don't know how
our ncurses is deciding what to do.

> You can run it in script(1) to see what it is using for the border (lots
> of 'q' or 'x' means ACS).

I see a string of 'q' in the typescript when in the xterm and TERM=xterm:

ESC[?1049hESC[1;24rESC(BESC[mESC[4lESC[?7hESC[?1hESC=ESC[HESC[2J Worm
ESC(0ESC[0mlqkESC(B
ESC(0ESC[0mxESC(BESC[79GESC(0ESC[0mxESC(B
ESC(0ESC[0mxESC(BESC[79GESC(0ESC[0mxESC(B

in xterm when TERM=vt220:

ESC)0ESC[1;24rESC[mESC(BESC[4lESC[?7hESC[HESC[J Worm
ESC[0mESC(0lqkESC(B
ESC[0mESC(0xESC(BESC[77CESC[0mESC(0xESC(B
ESC[0mESC(0xESC(BESC[77CESC[0mESC(0xESC(B

in wscons when TERM=vt220:

ESC)0ESC[1;45rESC[mESC(BESC[4lESC[?7hESC[HESC[J Worm
ESC[0mESC(0lqkESC(B
ESC[0mESC(0xESC(BESC[157CESC[0mESC(0xESC(B
ESC[0mESC(0xESC(BESC[157CESC[0mESC(0xESC(B

in wscons when TERM=xterm:

ESC[?1049hESC[1;45rESC(BESC[mESC[4lESC[?7hESC[?1hESC=ESC[HESC[2J Worm
ESC(0ESC[0mlqkESC(B
ESC(0ESC[0mxESC(BESC[159GESC(0ESC[0mxESC(B
ESC(0ESC[0mxESC(BESC[159GESC(0ESC[0mxESC(B

... so it's doing something based on TERM.



iked(8): improve logging output

2019-09-25 Thread Tobias Heider
A recent commit added the macro SPI_SA which is used to prepend log lines with
"spi=0x.." as seen in the message receive and send output.
This diff updates the rest of the ikev2.c file to use the new macro.

Because I was already fiddling with logging I changed some log_debugs to
log_info. Most of them were error messages that should not require verbose
to see them.

ok?


Index: ikev2.c
===
RCS file: /cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.175
diff -u -p -u -r1.175 ikev2.c
--- ikev2.c 29 Aug 2019 14:56:23 -  1.175
+++ ikev2.c 25 Sep 2019 20:39:08 -
@@ -275,7 +275,8 @@ ikev2_dispatch_cert(int fd, struct privs
log_debug("%s: peer certificate is valid", __func__);
sa_stateflags(sa, IKED_REQ_CERTVALID);
} else {
-   log_warnx("%s: peer certificate is invalid", __func__);
+   log_warnx("%s: peer certificate is invalid",
+   SPI_SA(sa, __func__));
ikev2_send_auth_failed(env, sa);
break;
}
@@ -332,7 +333,8 @@ ikev2_dispatch_cert(int fd, struct privs
break;
}
if (sa_stateok(sa, IKEV2_STATE_VALID)) {
-   log_warnx("%s: ignoring AUTH in state %s", __func__,
+   log_warnx("%s: ignoring AUTH in state %s",
+   SPI_SA(sa, __func__),
print_map(sa->sa_state, ikev2_state_map));
break;
}
@@ -371,7 +373,7 @@ ikev2_ikesa_info(uint64_t spi, const cha
 {
static char buf[1024];
const char *spistr;
-   
+
spistr = print_spi(spi, 8);
if (msg)
snprintf(buf, sizeof(buf), "spi=%s: %s", spistr, msg);
@@ -698,7 +700,7 @@ ikev2_ike_auth_recv(struct iked *env, st
if (ikev2_ike_auth_compatible(sa,
ikeauth.auth_method, msg->msg_auth.id_type) < 0) {
log_warnx("%s: unexpected auth method %s, was "
-   "expecting %s", __func__,
+   "expecting %s", SPI_SA(sa, __func__),
print_map(msg->msg_auth.id_type, ikev2_auth_map),
print_map(ikeauth.auth_method, ikev2_auth_map));
return (-1);
@@ -884,7 +886,8 @@ ikev2_init_recv(struct iked *env, struct
if (ikev2_init_ike_sa_peer(env, pol,
>pol_peer, msg) != 0)
log_warnx("%s: failed to initiate a "
-   "IKE_SA_INIT exchange", __func__);
+   "IKE_SA_INIT exchange", SPI_SA(sa,
+   __func__));
break;
}
(void)ikev2_init_auth(env, msg);
@@ -979,7 +982,7 @@ ikev2_init_ike_sa_peer(struct iked *env,
/* Pick peer's DH group if asked */
if (pol->pol_peerdh > 0 && sa->sa_dhgroup == NULL &&
(sa->sa_dhgroup = group_get(pol->pol_peerdh)) == NULL) {
-   log_warnx("%s: invalid peer DH group %u", __func__,
+   log_warnx("%s: invalid peer DH group %u", SPI_SA(sa, __func__),
pol->pol_peerdh);
goto closeonly;
}
@@ -2350,7 +2353,8 @@ ikev2_resp_recv(struct iked *env, struct
}
 
if (ikev2_pld_parse(env, hdr, msg, msg->msg_offset) != 0) {
-   log_debug("%s: failed to parse message", __func__);
+   log_info("%s: failed to parse message",
+   SPI_SA(msg->msg_sa, __func__));
return;
}
 
@@ -2373,7 +2377,8 @@ ikev2_resp_recv(struct iked *env, struct
switch (hdr->ike_exchange) {
case IKEV2_EXCHANGE_IKE_SA_INIT:
if (ikev2_sa_responder(env, sa, NULL, msg) != 0) {
-   log_debug("%s: failed to get IKE SA keys", __func__);
+   log_info("%s: failed to negotiate IKE SA",
+   SPI_SA(sa, __func__));
if (msg->msg_error == 0)
msg->msg_error = IKEV2_N_NO_PROPOSAL_CHOSEN;
ikev2_send_init_error(env, msg);
@@ -2605,6 +2610,8 @@ ikev2_add_error(struct iked *env, struct
default:
return (-1);
}
+   log_info("%s: %s", SPI_SA(msg->msg_sa, __func__),
+   print_map(msg->msg_error, ikev2_n_map));
len = sizeof(*n);
if ((ptr = ibuf_advance(buf, len)) == NULL)
return (-1);
@@ -3373,8 +3380,8 @@ ikev2_init_create_child_sa(struct iked *
 * Resolve simultaneous IKE SA rekeying by
 * deleting the SA with the lowest NONCE.
 */
- 

Re: iked(8): fix some leaks in parse.y

2019-09-25 Thread Tobias Heider
> Your fix reads ok, but:
> 
> please write null-pointer checks explicitly, i.e. != NULL or == NULL.

Sure.

> and a suggestion: how about putting a
> 
> if (head == NULL)
>   return;
> 
> at the top of iaw_free(), so it works like free()?
> 
> Then you can skip the if (...) infront of all the calls.

Thanks, makes sense. Not sure how I didn't think of this.
Here is a cleaned up version:

Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.83
diff -u -p -u -r1.83 parse.y
--- parse.y 26 Aug 2019 16:41:08 -  1.83
+++ parse.y 25 Sep 2019 20:28:31 -
@@ -354,10 +354,13 @@ intget_id_type(char *);
 uint8_t x2i(unsigned char *);
 int parsekey(unsigned char *, size_t, struct iked_auth *);
 int parsekeyfile(char *, struct iked_auth *);
+voidiaw_free(struct ipsec_addr_wrap *);
 
 struct ipsec_transforms *ipsec_transforms;
 struct ipsec_filters *ipsec_filters;
 struct ipsec_mode *ipsec_mode;
+/* interface lookup routintes */
+struct ipsec_addr_wrap *iftab;
 
 typedef struct {
union {
@@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
free(sym);
}
 
+   iaw_free(iftab);
+   iftab = NULL;
+
return (errors ? -1 : 0);
 }
 
@@ -2184,10 +2190,6 @@ host_any(void)
return (ipa);
 }
 
-/* interface lookup routintes */
-
-struct ipsec_addr_wrap *iftab;
-
 void
 ifa_load(void)
 {
@@ -3040,7 +3042,17 @@ done:
free(p->prop_xforms);
free(p);
}
-
+   if (peers != NULL) {
+   iaw_free(peers->src);
+   iaw_free(peers->dst);
+   /* peers is static, cannot be freed */
+   }
+   if (hosts != NULL) {
+   iaw_free(hosts->src);
+   iaw_free(hosts->dst);
+   free(hosts);
+   }
+   iaw_free(ikecfg);
return (ret);
 }
 
@@ -3066,4 +3078,24 @@ create_user(const char *user, const char
 
rules++;
return (0);
+}
+
+void
+iaw_free(struct ipsec_addr_wrap *head)
+{
+   struct ipsec_addr_wrap *n, *cur;
+
+   if (head == NULL)
+   return;
+
+   for (n = head; n != NULL; ) {
+   cur = n;
+   n = n->next;
+   if (cur->srcnat != NULL) {
+   free(cur->srcnat->name);
+   free(cur->srcnat);
+   }
+   free(cur->name);
+   free(cur);
+   }
 }



Re: new USB audio class v2.0 driver

2019-09-25 Thread Alexandre Ratchov
On Wed, Sep 25, 2019 at 08:39:10PM +0100, cho...@jtan.com wrote:
> Alexandre Ratchov writes:
> > On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> > > I have a similar problem to Alexander Hof with a presonus audio usb
> > > device, where attaching it reports 'only one AC iface allowed' and the
> > > device remains (apparently totally) inaccessible.
> > >
> > > dmes and lsusb included below.
> > >
> >
> > According to dmesg, this is 6.5, which doesn't contain the fix for
> > this problem. Could you try the device on a -current system?
> 
> I did look through the post 6.5 changes in CVS but couldn't see anything
> relevent, possibly for reasons that will become clear.
> 
> > Programmable clocks are OK, the comment states that we don't support
> > multiple clock sources simultaneously. When audio starts, all parts of
> > the device must be clocked by the same clock source.
> >
> > Pro audio interfaces use necessarily a single clock because otherwise
> > they wouldn't be unstable in DAWs and/or for real-time effects.
> >
> > FWIW, I've never seen -- or even heard of -- devices using multiple
> > clock sources simultaneously.
> 
> Good, because that was a complete red herring. The _other_ instance of
> '%s: only one AC iface allowed' is the one associated with the fault, in
> uaudio_process_conf(). Perhaps the error message at the end of
> uaudio_process_ac() could read '%s: only one distinct clock source
> allowed'?
> 
> There is good news and bad news about -current. The good news is that
> the upgrade was seamless and the device is registered without reporting
> an error. The bad news is that I'd eventually managed to notice the
> empty (?) control interface and skip it, but it still didn't/doesn't
> work:
> 
> uaudio0 at uhub3 port 2 configuration 1 interface 1 "PreSonus AudioBox USB 
> 96" rev 2.00/1.12 addr 4
> uaudio0: class v2, high-speed, async, channels: 2 play, 2 rec, 0 ctls
> audio1 at uaudio0
> umidi0 at uhub3 port 2 configuration 1 interface 4 "PreSonus AudioBox USB 96" 
> rev 2.00/1.12 addr 4
> umidi0: (genuine USB-MIDI)
> umidi0: out=1, in=1
> midi0 at umidi0: 
> ugen1 at uhub3 port 2 configuration 1 "PreSonus AudioBox USB 96" rev 
> 2.00/1.12 addr 4
> 
> ludmilla$ audioctl -f /dev/audioctl1
> name=uaudio0
> mode=
> pause=0
> active=0
> nblks=2
> blksz=960
> rate=48000
> encoding=s16le
> play.channels=2
> play.bytes=0
> play.errors=0
> record.channels=2
> record.bytes=0
> record.errors=0
> 
> ludmilla$ mixerctl -f /dev/mixer1
> record.enable=sysctl
> 
> ludmilla$ sysctl |grep -e mix -e audio
> kern.audio.record=0
> 
> And from mplayer with AUDIODEVICE=snd/1:
> ...
> ao2: can't open sndio
> ...

You need to set

sndiod_flags=-frsnd/0 -frsnd/1

in /etc/rc.conf.local and restart sndiod. This is for sndiod to
configure this device as well.

Another option is to use the new -F option, so that sndiod will use
the USB device if it's connected and the internal if it
isn't. Example:

sndiod_flags=-frsnd/0 -Frsnd/1

in this case, no need to export AUDIODEVICE=snd/1, assuming the device
works ;-)



Re: new USB audio class v2.0 driver

2019-09-25 Thread Alexandre Ratchov
On Wed, Sep 25, 2019 at 08:13:09PM +0100, cho...@jtan.com wrote:
> Alexandre Ratchov writes:
> > On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> > > I have a similar problem to Alexander Hof with a presonus audio usb
> > > device, where attaching it reports 'only one AC iface allowed' and the
> > > device remains (apparently totally) inaccessible.
> > > 
> > > dmes and lsusb included below.
> > > 
> >
> > According to dmesg, this is 6.5, which doesn't contain the fix for
> > this problem. Could you try the device on a -current system?
> 
> I did look through the post 6.5 changes in CVS but couldn't see anything
> relevent, possibly for reasons that will become clear.
> 

This is the commit:


revision 1.142
date: 2019/05/09 06:58:13;  author: ratchov;  state: Exp;  lines: +5 -8;  
commitid: 3l3sLX3Zz5BTanPk;
Skip empty control interfaces when parsing descriptors.

Even if having multiple control interface descriptors is not allowed
by the UAC spec, there's no reason to stop as long as a proper control
interface was processed.


But there are fixes for other problems, so I'd suggest using 6.6

> > Programmable clocks are OK, the comment states that we don't support
> > multiple clock sources simultaneously. When audio starts, all parts of
> > the device must be clocked by the same clock source.
> >
> > Pro audio interfaces use necessarily a single clock because otherwise
> > they wouldn't be unstable in DAWs and/or for real-time effects.
> >
> > FWIW, I've never seen -- or even heard of -- devices using multiple
> > clock sources simultaneously.
> 
> Good, because that was a complete red herring. The _other_ instance of
> '%s: only one AC iface allowed' is the one associated with the fault, in
> uaudio_process_conf().

It was removed and the code fixed. The problem was that certain
devices with MIDI ports (like yours) may expose two Audio Control (aka
AC) interfaces one for audio and one (empty) for MIDI; this is not
allowed by the standard. The new code accepts multiple AC interfaces,
it picks the right one and ignores the empty ones.

> Perhaps the error message at the end of
> uaudio_process_ac() could read '%s: only one distinct clock source
> allowed'?

Actually we support multiple clock sources, but distinct sources may
not clock distinct parts of the device.

The "domain" word is the one used in the UAC specification; maybe too
technical, but it's exact. Note that there are few other
developper-centric messages, mainly to help improving the driver in
case certain "rare" devices pop-up one day.



Re: iked(8): fix some leaks in parse.y

2019-09-25 Thread Sebastian Benoit
Tobias Heider(tobias.hei...@stusta.de) on 2019.09.25 21:29:44 +0200:
> As the subject says this diff fixes a few leaks in the config parser.

Your fix reads ok, but:

please write null-pointer checks explicitly, i.e. != NULL or == NULL.

and a suggestion: how about putting a

if (head == NULL)
return;

at the top of iaw_free(), so it works like free()?

Then you can skip the if (...) infront of all the calls.

/B.


> ok?
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 parse.y
> --- parse.y   26 Aug 2019 16:41:08 -  1.83
> +++ parse.y   25 Sep 2019 19:28:24 -
> @@ -354,10 +354,13 @@ int  get_id_type(char *);
>  uint8_t   x2i(unsigned char *);
>  int   parsekey(unsigned char *, size_t, struct iked_auth *);
>  int   parsekeyfile(char *, struct iked_auth *);
> +void  iaw_free(struct ipsec_addr_wrap *);
>  
>  struct ipsec_transforms *ipsec_transforms;
>  struct ipsec_filters *ipsec_filters;
>  struct ipsec_mode *ipsec_mode;
> +/* interface lookup routintes */
> +struct ipsec_addr_wrap   *iftab;
>  
>  typedef struct {
>   union {
> @@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
>   free(sym);
>   }
>  
> + iaw_free(iftab);
> + iftab = NULL;
> +
>   return (errors ? -1 : 0);
>  }
>  
> @@ -2184,10 +2190,6 @@ host_any(void)
>   return (ipa);
>  }
>  
> -/* interface lookup routintes */
> -
> -struct ipsec_addr_wrap   *iftab;
> -
>  void
>  ifa_load(void)
>  {
> @@ -3040,7 +3042,22 @@ done:
>   free(p->prop_xforms);
>   free(p);
>   }
> -
> + if (peers) {
> + if (peers->src)
> + iaw_free(peers->src);
> + if (peers->dst)
> + iaw_free(peers->dst);
> + /* peers is static, cannot be freed */
> + }
> + if (hosts) {
> + if (hosts->src)
> + iaw_free(hosts->src);
> + if (hosts->dst)
> + iaw_free(hosts->dst);
> + free(hosts);
> + }
> + if (ikecfg)
> + iaw_free(ikecfg);
>   return (ret);
>  }
>  
> @@ -3066,4 +3083,21 @@ create_user(const char *user, const char
>  
>   rules++;
>   return (0);
> +}
> +
> +void
> +iaw_free(struct ipsec_addr_wrap *head)
> +{
> + struct ipsec_addr_wrap *n, *cur;
> +
> + for (n = head; n; ) {
> + cur = n;
> + n = n->next;
> + if (cur->srcnat) {
> + free(cur->srcnat->name);
> + free(cur->srcnat);
> + }
> + free(cur->name);
> + free(cur);
> + }
>  }
> 



Re: new USB audio class v2.0 driver

2019-09-25 Thread chohag
Alexandre Ratchov writes:
> On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> > I have a similar problem to Alexander Hof with a presonus audio usb
> > device, where attaching it reports 'only one AC iface allowed' and the
> > device remains (apparently totally) inaccessible.
> >
> > dmes and lsusb included below.
> >
>
> According to dmesg, this is 6.5, which doesn't contain the fix for
> this problem. Could you try the device on a -current system?

I did look through the post 6.5 changes in CVS but couldn't see anything
relevent, possibly for reasons that will become clear.

> Programmable clocks are OK, the comment states that we don't support
> multiple clock sources simultaneously. When audio starts, all parts of
> the device must be clocked by the same clock source.
>
> Pro audio interfaces use necessarily a single clock because otherwise
> they wouldn't be unstable in DAWs and/or for real-time effects.
>
> FWIW, I've never seen -- or even heard of -- devices using multiple
> clock sources simultaneously.

Good, because that was a complete red herring. The _other_ instance of
'%s: only one AC iface allowed' is the one associated with the fault, in
uaudio_process_conf(). Perhaps the error message at the end of
uaudio_process_ac() could read '%s: only one distinct clock source
allowed'?

There is good news and bad news about -current. The good news is that
the upgrade was seamless and the device is registered without reporting
an error. The bad news is that I'd eventually managed to notice the
empty (?) control interface and skip it, but it still didn't/doesn't
work:

uaudio0 at uhub3 port 2 configuration 1 interface 1 "PreSonus AudioBox USB 96" 
rev 2.00/1.12 addr 4
uaudio0: class v2, high-speed, async, channels: 2 play, 2 rec, 0 ctls
audio1 at uaudio0
umidi0 at uhub3 port 2 configuration 1 interface 4 "PreSonus AudioBox USB 96" 
rev 2.00/1.12 addr 4
umidi0: (genuine USB-MIDI)
umidi0: out=1, in=1
midi0 at umidi0: 
ugen1 at uhub3 port 2 configuration 1 "PreSonus AudioBox USB 96" rev 2.00/1.12 
addr 4

ludmilla$ audioctl -f /dev/audioctl1
name=uaudio0
mode=
pause=0
active=0
nblks=2
blksz=960
rate=48000
encoding=s16le
play.channels=2
play.bytes=0
play.errors=0
record.channels=2
record.bytes=0
record.errors=0

ludmilla$ mixerctl -f /dev/mixer1
record.enable=sysctl

ludmilla$ sysctl |grep -e mix -e audio
kern.audio.record=0

And from mplayer with AUDIODEVICE=snd/1:
...
ao2: can't open sndio
...

Matthew



Re: pretty borders for slitherins

2019-09-25 Thread Nicholas Marriott
It will use either UTF-8 characters or the ACS characters from TERM.

The way it decides which is somewhat confusing and it looks like it is
choosing to use UTF-8 even when I wouldn't expect it to so I'm not sure
it is actually working correctly in our ncurses version.

I don't think UTF-8 line drawing works properly, at least with the DRM
wscons.

What do you have in TERM now? Can you see if the borders are OK with
"TERM=xterm snake"? This seems to use ACS rather than UTF-8. If it does
I'll see if I can figure out why it choosing UTF-8 borders for the
others.

You can run it in script(1) to see what it is using for the border (lots
of 'q' or 'x' means ACS).


On Wed, Sep 25, 2019 at 11:15:01AM -0500, Scott Cheloha wrote:
> On Mon, Sep 23, 2019 at 06:23:32PM -0400, Ted Unangst wrote:
> > snake and worm draw boxes, but they can be prettier by using the default
> > style, which will use line drawing instead of ugly -*| characters.
> > 
> > should do the right thing on a vt100, but only tested in xterm.
> 
> It looks much prettier in an xterm but in the system console I'm
> getting question marks.  Is that a limitation of the terminal or
> am I missing the glyphs?  Dunno if this is the expected behavior
> but it is uglier than what we had before.
> 
> Also, you forgot to delete the prototype for drawbox().
> 
> > Index: snake/snake.c
> > ===
> > RCS file: /home/cvs/src/games/snake/snake.c,v
> > retrieving revision 1.34
> > diff -u -p -r1.34 snake.c
> > --- snake/snake.c   28 Jun 2019 13:32:52 -  1.34
> > +++ snake/snake.c   13 Sep 2019 17:05:19 -
> > @@ -450,23 +450,8 @@ setup(void)
> > pchar([i], SNAKETAIL);
> > }
> > pchar([0], SNAKEHEAD);
> > -   drawbox();
> > +   border(0, 0, 0, 0, 0, 0, 0, 0);
> > refresh();
> > -}
> > -
> > -void
> > -drawbox(void)
> > -{
> > -   int i;
> > -
> > -   for (i = 1; i <= ccnt; i++) {
> > -   mvaddch(0, i, '-');
> > -   mvaddch(lcnt + 1, i, '-');
> > -   }
> > -   for (i = 0; i <= lcnt + 1; i++) {
> > -   mvaddch(i, 0, '|');
> > -   mvaddch(i, ccnt + 1, '|');
> > -   }
> >  }
> >  
> >  void
> > Index: worm/worm.c
> > ===
> > RCS file: /home/cvs/src/games/worm/worm.c,v
> > retrieving revision 1.39
> > diff -u -p -r1.39 worm.c
> > --- worm/worm.c 24 Aug 2018 11:14:49 -  1.39
> > +++ worm/worm.c 13 Sep 2019 16:51:39 -
> > @@ -122,7 +122,7 @@ main(int argc, char **argv)
> > }
> > stw = newwin(1, COLS-1, 0, 0);
> > tv = newwin(LINES-1, COLS-1, 1, 0);
> > -   box(tv, '*', '*');
> > +   box(tv, 0, 0);
> > scrollok(tv, FALSE);
> > scrollok(stw, FALSE);
> > wmove(stw, 0, 0);
> > 
> 



iked(8): fix some leaks in parse.y

2019-09-25 Thread Tobias Heider
As the subject says this diff fixes a few leaks in the config parser.

ok?

Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.83
diff -u -p -u -r1.83 parse.y
--- parse.y 26 Aug 2019 16:41:08 -  1.83
+++ parse.y 25 Sep 2019 19:28:24 -
@@ -354,10 +354,13 @@ intget_id_type(char *);
 uint8_t x2i(unsigned char *);
 int parsekey(unsigned char *, size_t, struct iked_auth *);
 int parsekeyfile(char *, struct iked_auth *);
+voidiaw_free(struct ipsec_addr_wrap *);
 
 struct ipsec_transforms *ipsec_transforms;
 struct ipsec_filters *ipsec_filters;
 struct ipsec_mode *ipsec_mode;
+/* interface lookup routintes */
+struct ipsec_addr_wrap *iftab;
 
 typedef struct {
union {
@@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
free(sym);
}
 
+   iaw_free(iftab);
+   iftab = NULL;
+
return (errors ? -1 : 0);
 }
 
@@ -2184,10 +2190,6 @@ host_any(void)
return (ipa);
 }
 
-/* interface lookup routintes */
-
-struct ipsec_addr_wrap *iftab;
-
 void
 ifa_load(void)
 {
@@ -3040,7 +3042,22 @@ done:
free(p->prop_xforms);
free(p);
}
-
+   if (peers) {
+   if (peers->src)
+   iaw_free(peers->src);
+   if (peers->dst)
+   iaw_free(peers->dst);
+   /* peers is static, cannot be freed */
+   }
+   if (hosts) {
+   if (hosts->src)
+   iaw_free(hosts->src);
+   if (hosts->dst)
+   iaw_free(hosts->dst);
+   free(hosts);
+   }
+   if (ikecfg)
+   iaw_free(ikecfg);
return (ret);
 }
 
@@ -3066,4 +3083,21 @@ create_user(const char *user, const char
 
rules++;
return (0);
+}
+
+void
+iaw_free(struct ipsec_addr_wrap *head)
+{
+   struct ipsec_addr_wrap *n, *cur;
+
+   for (n = head; n; ) {
+   cur = n;
+   n = n->next;
+   if (cur->srcnat) {
+   free(cur->srcnat->name);
+   free(cur->srcnat);
+   }
+   free(cur->name);
+   free(cur);
+   }
 }



Re: new USB audio class v2.0 driver

2019-09-25 Thread Alexandre Ratchov
On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> I have a similar problem to Alexander Hof with a presonus audio usb
> device, where attaching it reports 'only one AC iface allowed' and the
> device remains (apparently totally) inaccessible.
> 
> dmes and lsusb included below.
> 

According to dmesg, this is 6.5, which doesn't contain the fix for
this problem. Could you try the device on a -current system?

> The device and the box it plugs into are both available for prodding and
> poking if it might help as neither is in use (but note I am not on tech@).
> 
> It would appear that this assumption doesn't fully hold on some,
> professional?, usb audio hardware:
> 
> /*
>  * Find common clock unit. We assume all terminals
>  * belong to the same clock domain (ie are connected
>  * to the same source)
>  */
> 
> A cheap USB sound card I have to hand does not include the programmable
> clock in its lsusb output, or indeed anything marked CLOCK_SOURCE.
> 

Programmable clocks are OK, the comment states that we don't support
multiple clock sources simultaneously. When audio starts, all parts of
the device must be clocked by the same clock source.

Pro audio interfaces use necessarily a single clock because otherwise
they wouldn't be unstable in DAWs and/or for real-time effects.

FWIW, I've never seen -- or even heard of -- devices using multiple
clock sources simultaneously.



Re: pretty borders for slitherins

2019-09-25 Thread Mike Larkin
On Wed, Sep 25, 2019 at 10:53:21AM -0600, Theo de Raadt wrote:
> Ted Unangst  wrote:
>
> > Scott Cheloha wrote:
> > > On Mon, Sep 23, 2019 at 06:23:32PM -0400, Ted Unangst wrote:
> > > > snake and worm draw boxes, but they can be prettier by using the default
> > > > style, which will use line drawing instead of ugly -*| characters.
> > > >
> > > > should do the right thing on a vt100, but only tested in xterm.
> > >
> > > It looks much prettier in an xterm but in the system console I'm
> > > getting question marks.  Is that a limitation of the terminal or
> > > am I missing the glyphs?  Dunno if this is the expected behavior
> > > but it is uglier than what we had before.
> >
> > ah, that's disappointing. my understanding is it should revert to ascii, but
> > there seems to be a flaw, either in curses or my understanding. curses!
>
> If I recall, curses needs a little bit more initialization if you want
> it to do the right thing
>
> this trap was set because we don't have a curses maintainer, but look
> now we do, since it is mostly the games who need this, maybe mlarkin
> can help you
>

How did I get pulled into this? :)



Re: pretty borders for slitherins

2019-09-25 Thread Theo de Raadt
Ted Unangst  wrote:

> Scott Cheloha wrote:
> > On Mon, Sep 23, 2019 at 06:23:32PM -0400, Ted Unangst wrote:
> > > snake and worm draw boxes, but they can be prettier by using the default
> > > style, which will use line drawing instead of ugly -*| characters.
> > > 
> > > should do the right thing on a vt100, but only tested in xterm.
> > 
> > It looks much prettier in an xterm but in the system console I'm
> > getting question marks.  Is that a limitation of the terminal or
> > am I missing the glyphs?  Dunno if this is the expected behavior
> > but it is uglier than what we had before.
> 
> ah, that's disappointing. my understanding is it should revert to ascii, but
> there seems to be a flaw, either in curses or my understanding. curses!

If I recall, curses needs a little bit more initialization if you want
it to do the right thing

this trap was set because we don't have a curses maintainer, but look
now we do, since it is mostly the games who need this, maybe mlarkin
can help you 



Re: pretty borders for slitherins

2019-09-25 Thread Ted Unangst
Scott Cheloha wrote:
> On Mon, Sep 23, 2019 at 06:23:32PM -0400, Ted Unangst wrote:
> > snake and worm draw boxes, but they can be prettier by using the default
> > style, which will use line drawing instead of ugly -*| characters.
> > 
> > should do the right thing on a vt100, but only tested in xterm.
> 
> It looks much prettier in an xterm but in the system console I'm
> getting question marks.  Is that a limitation of the terminal or
> am I missing the glyphs?  Dunno if this is the expected behavior
> but it is uglier than what we had before.

ah, that's disappointing. my understanding is it should revert to ascii, but
there seems to be a flaw, either in curses or my understanding. curses!



Fix libcompiler_rt __clear_cache() on armv7

2019-09-25 Thread Josh Elsasser
I came across some code which uses __clear_cache() from
libgcc/libcompiler_rt on 32-bit arm. Currently that just falls through
to abort(), but enabling the existing sysarch() code works for me.

diff --git a/lib/libcompiler_rt/clear_cache.c b/lib/libcompiler_rt/clear_cache.c
index 451f1c0b124..4902d761b81 100644
--- a/lib/libcompiler_rt/clear_cache.c
+++ b/lib/libcompiler_rt/clear_cache.c
@@ -33,7 +33,7 @@ uintptr_t GetCurrentProcess(void);
   #include 
 #endif
 
-#if defined(__OpenBSD__) && defined(__mips__)
+#if defined(__OpenBSD__) && (defined(__mips__) || defined(__arm__))
   #include 
   #include 
 #endif
@@ -102,7 +102,7 @@ void __clear_cache(void *start, void *end) {
  * so there is nothing to do
  */
 #elif defined(__arm__) && !defined(__APPLE__)
-#if defined(__FreeBSD__) || defined(__NetBSD__)
+#if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
 struct arm_sync_icache_args arg;
 
 arg.addr = (uintptr_t)start;



Re: new USB audio class v2.0 driver

2019-09-25 Thread chohag
I have a similar problem to Alexander Hof with a presonus audio usb
device, where attaching it reports 'only one AC iface allowed' and the
device remains (apparently totally) inaccessible.

dmes and lsusb included below.

The device and the box it plugs into are both available for prodding and
poking if it might help as neither is in use (but note I am not on tech@).

It would appear that this assumption doesn't fully hold on some,
professional?, usb audio hardware:

/*
 * Find common clock unit. We assume all terminals
 * belong to the same clock domain (ie are connected
 * to the same source)
 */

A cheap USB sound card I have to hand does not include the programmable
clock in its lsusb output, or indeed anything marked CLOCK_SOURCE.

Matthew

OpenBSD 6.5 (GENERIC.MP) #0: Wed Apr 24 23:38:54 CEST 2019

r...@syspatch-65-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 34191831040 (32607MB)
avail mem = 33145958400 (31610MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0x7a9bc000 (99 entries)
bios0: vendor American Megatrends Inc. version "N.1.03" date 05/16/2018
bios0: MEDION ERAZER X6805 MD61140
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP APIC FPDT FIDT MCFG SSDT SSDT MSDM SSDT SSDT HPET SSDT 
SSDT UEFI LPIT DBGP DBG2 SSDT DMAR BGRT TPM2 SSDT WSMT
acpi0: wakeup devices PEGP(S4) PEG0(S4) PEGP(S4) PEG1(S4) PEGP(S4) PEG2(S4) 
PXSX(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) 
PXSX(S4) RP05(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz, 2096.92 MHz, 06-9e-0a
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 24MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz, 2095.14 MHz, 06-9e-0a
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz, 2095.14 MHz, 06-9e-0a
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz, 2095.13 MHz, 06-9e-0a
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
cpu4 at mainbus0: apid 8 (application processor)
cpu4: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz, 2095.13 MHz, 06-9e-0a
cpu4: 

Re: pretty borders for slitherins

2019-09-25 Thread Scott Cheloha
On Mon, Sep 23, 2019 at 06:23:32PM -0400, Ted Unangst wrote:
> snake and worm draw boxes, but they can be prettier by using the default
> style, which will use line drawing instead of ugly -*| characters.
> 
> should do the right thing on a vt100, but only tested in xterm.

It looks much prettier in an xterm but in the system console I'm
getting question marks.  Is that a limitation of the terminal or
am I missing the glyphs?  Dunno if this is the expected behavior
but it is uglier than what we had before.

Also, you forgot to delete the prototype for drawbox().

> Index: snake/snake.c
> ===
> RCS file: /home/cvs/src/games/snake/snake.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 snake.c
> --- snake/snake.c 28 Jun 2019 13:32:52 -  1.34
> +++ snake/snake.c 13 Sep 2019 17:05:19 -
> @@ -450,23 +450,8 @@ setup(void)
>   pchar([i], SNAKETAIL);
>   }
>   pchar([0], SNAKEHEAD);
> - drawbox();
> + border(0, 0, 0, 0, 0, 0, 0, 0);
>   refresh();
> -}
> -
> -void
> -drawbox(void)
> -{
> - int i;
> -
> - for (i = 1; i <= ccnt; i++) {
> - mvaddch(0, i, '-');
> - mvaddch(lcnt + 1, i, '-');
> - }
> - for (i = 0; i <= lcnt + 1; i++) {
> - mvaddch(i, 0, '|');
> - mvaddch(i, ccnt + 1, '|');
> - }
>  }
>  
>  void
> Index: worm/worm.c
> ===
> RCS file: /home/cvs/src/games/worm/worm.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 worm.c
> --- worm/worm.c   24 Aug 2018 11:14:49 -  1.39
> +++ worm/worm.c   13 Sep 2019 16:51:39 -
> @@ -122,7 +122,7 @@ main(int argc, char **argv)
>   }
>   stw = newwin(1, COLS-1, 0, 0);
>   tv = newwin(LINES-1, COLS-1, 1, 0);
> - box(tv, '*', '*');
> + box(tv, 0, 0);
>   scrollok(tv, FALSE);
>   scrollok(stw, FALSE);
>   wmove(stw, 0, 0);
> 



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-25 Thread Claudio Jeker
On Wed, Sep 25, 2019 at 12:19:23PM +0100, Stuart Henderson wrote:
> On 2019/09/24 22:06, Sebastian Benoit wrote:
> > Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.09.24 17:01:21 +0200:
> > > On Tue, Sep 24, 2019 at 03:51:43PM +0100, Stuart Henderson wrote:
> > > > On 2019/09/24 11:10, Claudio Jeker wrote:
> > > > > On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > > > > > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > > > > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > > > > > "show nei group XX terse" you can't tell which entry relates to 
> > > > > > > each
> > > > > > > neighbour.
> > > > > > > 
> > > > > > > OK to add the address to the end of the line where it's reasonably
> > > > > > > out of the way of existing parsers?
> > > > > > 
> > > > > > missing free, pointed out by benno. (not that bgpctl will stick 
> > > > > > around
> > > > > > for long anyway :)
> > > > > 
> > > > > This is fine with me. I wonder if other data e.g. the peer 
> > > > > description or
> > > > > peer AS number should be added as well.
> > > > 
> > > > That might be useful, though as the peer description could contain
> > > > spaces we'd want to be reasonably sure we have already included any
> > > > other useful data first so that it can go right at the end of the line
> > > > (so that parsing the output isn't too awkward).
> > > > 
> > > 
> > > I was thinking the same maybe even put the name in "" to make it look more
> > > like a string.
> > 
> > Yes, that makes it matchable again at least.
> > 
> > > At least adding the AS number should be done.
> > 
> > +1
> > 

OK claudio@, my diff looks the same.

> Index: bgpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.90
> diff -u -p -r1.90 bgpctl.8
> --- bgpctl.8  24 Sep 2019 14:46:09 -  1.90
> +++ bgpctl.8  25 Sep 2019 11:18:28 -
> @@ -290,7 +290,8 @@ The printed numbers are the sent and rec
>  notifications, sent and received updates, sent and received keepalives, and
>  sent and received route refresh messages plus the current and maximum
>  prefix count, the number of sent and received updates, sent and
> -received withdraws, and finally the neighbor's address.
> +received withdraws, the neighbor's address (or subnet, for a template),
> +AS number, and finally description.
>  .It Cm timers
>  Show the BGP timers.
>  .El
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.244
> diff -u -p -r1.244 bgpctl.c
> --- bgpctl.c  24 Sep 2019 14:46:09 -  1.244
> +++ bgpctl.c  25 Sep 2019 11:18:28 -
> @@ -619,8 +619,8 @@ show_neighbor_terse(struct imsg *imsg)
>   NULL)
>   err(1, "strdup");
>  
> - printf("%llu %llu %llu %llu %llu %llu %llu "
> - "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
> + printf("%llu %llu %llu %llu %llu %llu %llu %llu %llu "
> + "%llu %u %u %llu %llu %llu %llu %s %s \"%s\"\n",
>   p->stats.msg_sent_open, p->stats.msg_rcvd_open,
>   p->stats.msg_sent_notification,
>   p->stats.msg_rcvd_notification,
> @@ -630,7 +630,8 @@ show_neighbor_terse(struct imsg *imsg)
>   p->stats.prefix_cnt, p->conf.max_prefix,
>   p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
>   p->stats.prefix_sent_withdraw,
> - p->stats.prefix_rcvd_withdraw, s);
> + p->stats.prefix_rcvd_withdraw, s,
> + log_as(p->conf.remote_as), p->conf.descr);
>   free(s);
>   break;
>   case IMSG_CTL_END:
> 

-- 
:wq Claudio



smtpd: mail.* tempfail vs permfail

2019-09-25 Thread Martijn van Duren
EHLO,

Recently I moved my mailserver to a new HD resulting in temporary 
inaccessibility of my maildir (temporarily root owned because of 
recursive copy), leaving smtpd running thinking nothing of it.
This resulted in a few mails being PERMFAIL.

I talked to gilles@ about this and he's somewhat inclined to agree with
me that this is a TEMPFAIL situation. Even if the directory where the
mail is supposed to be delivered is permanently not accessible I'd
argue that we should keep this a TEMPFAIL, because an admin is inclined
to spot this misconfiguration mistake earlier this way (e.g. via mailq
monitoring) and mail won't get lost.

The diff below fixes this for mail.maildir and mail.mboxfile.
For mail.maildir I left the cases that appeared to be obvious
misconfiguration as PERMFAIL, but might be worth to convert these to
EX_TEMPFAIL as well.

Note that mail.lmtp already uses EX_TEMPFAIL in all cases.

While here I also removed the mkdirs from utils.c, which got me
confused at first (it's also part of mail.maildir.c and not used
anywhere else).

thoughts? OK?

martijn@

Index: mail.maildir.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mail.maildir.c,v
retrieving revision 1.12
diff -u -p -r1.12 mail.maildir.c
--- mail.maildir.c  3 Jul 2019 03:24:03 -   1.12
+++ mail.maildir.c  25 Sep 2019 13:21:36 -
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #defineMAILADDR_ESCAPE "!#$%&'*/?^`{|}~"
@@ -93,8 +94,11 @@ maildir_mkdirs(const char *dirname)
charpathname[PATH_MAX];
char*subdirs[] = { "cur", "tmp", "new" };
 
-   if (mkdirs(dirname, 0700) == -1 && errno != EEXIST)
-   err(1, NULL);
+   if (mkdirs(dirname, 0700) == -1 && errno != EEXIST) {
+   if (errno == EINVAL || errno == ENAMETOOLONG)
+   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
+   }
 
for (i = 0; i < nitems(subdirs); ++i) {
ret = snprintf(pathname, sizeof pathname, "%s/%s", dirname,
@@ -102,7 +106,7 @@ maildir_mkdirs(const char *dirname)
if (ret < 0 || (size_t)ret >= sizeof pathname)
errc(1, ENAMETOOLONG, "%s/%s", dirname, subdirs[i]);
if (mkdir(pathname, 0700) == -1 && errno != EEXIST)
-   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
}
 }
 
@@ -178,9 +182,9 @@ maildir_engine(const char *dirname, int 
 
fd = open(tmp, O_CREAT | O_EXCL | O_WRONLY, 0600);
if (fd == -1)
-   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
if ((fp = fdopen(fd, "w")) == NULL)
-   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
 
while ((linelen = getline(, , stdin)) != -1) {
line[strcspn(line, "\n")] = '\0';
@@ -194,19 +198,19 @@ maildir_engine(const char *dirname, int 
}
free(line);
if (ferror(stdin))
-   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
 
if (fflush(fp) == EOF ||
ferror(fp) ||
fsync(fd) == -1 ||
fclose(fp) == EOF)
-   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
 
(void)snprintf(new, sizeof new, "%s/new/%s",
is_junk ? junkpath : dirname, filename);
 
if (rename(tmp, new) == -1)
-   err(1, NULL);
+   err(EX_TEMPFAIL, NULL);
 
exit(0);
 }
@@ -223,8 +227,10 @@ mkdirs_component(const char *path, mode_
if (mkdir(path, mode | S_IWUSR | S_IXUSR) == -1)
return 0;
}
-   else if (!S_ISDIR(sb.st_mode))
+   else if (!S_ISDIR(sb.st_mode)) {
+   errno = ENOTDIR;
return 0;
+   }
 
return 1;
 }
@@ -238,12 +244,16 @@ mkdirs(const char *path, mode_t mode)
const char  *p;
 
/* absolute path required */
-   if (*path != '/')
+   if (*path != '/') {
+   errno = EINVAL;
return 0;
+   }
 
/* make sure we don't exceed PATH_MAX */
-   if (strlen(path) >= sizeof buf)
+   if (strlen(path) >= sizeof buf) {
+   errno = ENAMETOOLONG;
return 0;
+   }
 
memset(buf, 0, sizeof buf);
for (p = path; *p; p++) {
Index: mail.mboxfile.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mail.mboxfile.c,v
retrieving revision 1.2
diff -u -p -r1.2 mail.mboxfile.c
--- mail.mboxfile.c 28 Jun 2019 13:32:50 -  1.2
+++ mail.mboxfile.c 25 Sep 2019 13:21:36 -
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static voidmboxfile_engine(const char *sender, const char *filename);
@@ -73,10 +74,10 @@ mboxfile_engine(const char *sender, cons
 
fd = open(filename, O_CREAT | O_APPEND | O_WRONLY | O_EXLOCK, 0600);
  

Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-25 Thread Stuart Henderson
On 2019/09/24 22:06, Sebastian Benoit wrote:
> Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.09.24 17:01:21 +0200:
> > On Tue, Sep 24, 2019 at 03:51:43PM +0100, Stuart Henderson wrote:
> > > On 2019/09/24 11:10, Claudio Jeker wrote:
> > > > On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > > > > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > > > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > > > > "show nei group XX terse" you can't tell which entry relates to each
> > > > > > neighbour.
> > > > > > 
> > > > > > OK to add the address to the end of the line where it's reasonably
> > > > > > out of the way of existing parsers?
> > > > > 
> > > > > missing free, pointed out by benno. (not that bgpctl will stick around
> > > > > for long anyway :)
> > > > 
> > > > This is fine with me. I wonder if other data e.g. the peer description 
> > > > or
> > > > peer AS number should be added as well.
> > > 
> > > That might be useful, though as the peer description could contain
> > > spaces we'd want to be reasonably sure we have already included any
> > > other useful data first so that it can go right at the end of the line
> > > (so that parsing the output isn't too awkward).
> > > 
> > 
> > I was thinking the same maybe even put the name in "" to make it look more
> > like a string.
> 
> Yes, that makes it matchable again at least.
> 
> > At least adding the AS number should be done.
> 
> +1
> 
> > 
> > -- 
> > :wq Claudio
> > 
> 

Index: bgpctl.8
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
retrieving revision 1.90
diff -u -p -r1.90 bgpctl.8
--- bgpctl.824 Sep 2019 14:46:09 -  1.90
+++ bgpctl.825 Sep 2019 11:18:28 -
@@ -290,7 +290,8 @@ The printed numbers are the sent and rec
 notifications, sent and received updates, sent and received keepalives, and
 sent and received route refresh messages plus the current and maximum
 prefix count, the number of sent and received updates, sent and
-received withdraws, and finally the neighbor's address.
+received withdraws, the neighbor's address (or subnet, for a template),
+AS number, and finally description.
 .It Cm timers
 Show the BGP timers.
 .El
Index: bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.244
diff -u -p -r1.244 bgpctl.c
--- bgpctl.c24 Sep 2019 14:46:09 -  1.244
+++ bgpctl.c25 Sep 2019 11:18:28 -
@@ -619,8 +619,8 @@ show_neighbor_terse(struct imsg *imsg)
NULL)
err(1, "strdup");
 
-   printf("%llu %llu %llu %llu %llu %llu %llu "
-   "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
+   printf("%llu %llu %llu %llu %llu %llu %llu %llu %llu "
+   "%llu %u %u %llu %llu %llu %llu %s %s \"%s\"\n",
p->stats.msg_sent_open, p->stats.msg_rcvd_open,
p->stats.msg_sent_notification,
p->stats.msg_rcvd_notification,
@@ -630,7 +630,8 @@ show_neighbor_terse(struct imsg *imsg)
p->stats.prefix_cnt, p->conf.max_prefix,
p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
p->stats.prefix_sent_withdraw,
-   p->stats.prefix_rcvd_withdraw, s);
+   p->stats.prefix_rcvd_withdraw, s,
+   log_as(p->conf.remote_as), p->conf.descr);
free(s);
break;
case IMSG_CTL_END:



Re: Argument order fix for MCLGETI

2019-09-25 Thread Stuart Henderson
On 2019/09/25 10:22, Claudio Jeker wrote:
> On Wed, Sep 25, 2019 at 04:10:10PM +0800, Kevin Lo wrote:
> > ok?
> 
> OK claudio@
> 
> How did that even work?

/me adds ti, lge, nfe, nge to a list for next time the kernel grows too much ;-)



Re: Argument order fix for MCLGETI

2019-09-25 Thread Claudio Jeker
On Wed, Sep 25, 2019 at 04:10:10PM +0800, Kevin Lo wrote:
> ok?

OK claudio@

How did that even work?
 
> Index: sys/dev/ic/ti.c
> ===
> RCS file: /cvs/src/sys/dev/ic/ti.c,v
> retrieving revision 1.25
> diff -u -p -u -p -r1.25 ti.c
> --- sys/dev/ic/ti.c   22 Jan 2017 10:17:38 -  1.25
> +++ sys/dev/ic/ti.c   25 Sep 2019 08:06:26 -
> @@ -576,7 +576,7 @@ ti_newbuf_std(struct ti_softc *sc, int i
>   sc->ti_cdata.ti_rx_std_map[i] = dmamap;
>  
>   if (m == NULL) {
> - m_new = MCLGETI(NULL, MCLBYTES, NULL, M_DONTWAIT);
> + m_new = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
>   if (m_new == NULL)
>   return (ENOBUFS);
>  
> @@ -695,7 +695,7 @@ ti_newbuf_jumbo(struct ti_softc *sc, int
>   bus_dmamap_unload(sc->sc_dmatag, dmamap);
>  
>   if (m == NULL) {
> - m_new = MCLGETI(NULL, TI_JUMBO_FRAMELEN, NULL, M_DONTWAIT);
> + m_new = MCLGETI(NULL, M_DONTWAIT, NULL, TI_JUMBO_FRAMELEN);
>   if (m_new == NULL)
>   return (ENOBUFS);
>  
> Index: sys/dev/pci/if_lge.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_lge.c,v
> retrieving revision 1.73
> diff -u -p -u -p -r1.73 if_lge.c
> --- sys/dev/pci/if_lge.c  22 Jan 2017 10:17:38 -  1.73
> +++ sys/dev/pci/if_lge.c  25 Sep 2019 08:06:26 -
> @@ -626,7 +626,7 @@ lge_newbuf(struct lge_softc *sc, struct 
>   struct mbuf *m_new = NULL;
>  
>   if (m == NULL) {
> - m_new = MCLGETI(NULL, LGE_JLEN, NULL, M_DONTWAIT);
> + m_new = MCLGETI(NULL, M_DONTWAIT, NULL, LGE_JLEN);
>   if (m_new == NULL)
>   return (ENOBUFS);
>   } else {
> Index: sys/dev/pci/if_nfe.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_nfe.c,v
> retrieving revision 1.120
> diff -u -p -u -p -r1.120 if_nfe.c
> --- sys/dev/pci/if_nfe.c  8 Sep 2017 05:36:52 -   1.120
> +++ sys/dev/pci/if_nfe.c  25 Sep 2019 08:06:26 -
> @@ -697,7 +697,7 @@ nfe_rxeof(struct nfe_softc *sc)
>* old mbuf. In the unlikely case that the old mbuf can't be
>* reloaded either, explicitly panic.
>*/
> - mnew = MCLGETI(NULL, MCLBYTES, NULL, M_DONTWAIT);
> + mnew = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
>   if (mnew == NULL) {
>   ifp->if_ierrors++;
>   goto skip;
> @@ -1210,7 +1210,7 @@ nfe_alloc_rx_ring(struct nfe_softc *sc, 
>   for (i = 0; i < NFE_RX_RING_COUNT; i++) {
>   data = >rxq.data[i];
>  
> - data->m = MCLGETI(NULL, MCLBYTES, NULL, M_DONTWAIT);
> + data->m = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
>   if (data->m == NULL) {
>   printf("%s: could not allocate rx mbuf\n",
>   sc->sc_dev.dv_xname);
> Index: sys/dev/pci/if_nge.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_nge.c,v
> retrieving revision 1.92
> diff -u -p -u -p -r1.92 if_nge.c
> --- sys/dev/pci/if_nge.c  22 Jan 2017 10:17:38 -  1.92
> +++ sys/dev/pci/if_nge.c  25 Sep 2019 08:06:26 -
> @@ -962,7 +962,7 @@ nge_newbuf(struct nge_softc *sc, struct 
>   struct mbuf *m_new = NULL;
>  
>   if (m == NULL) {
> - m_new = MCLGETI(NULL, NGE_MCLBYTES, NULL, M_DONTWAIT);
> + m_new = MCLGETI(NULL, M_DONTWAIT, NULL, NGE_MCLBYTES);
>   if (m_new == NULL)
>   return (ENOBUFS);
>   } else {
> 

-- 
:wq Claudio



Argument order fix for MCLGETI

2019-09-25 Thread Kevin Lo
ok?

Index: sys/dev/ic/ti.c
===
RCS file: /cvs/src/sys/dev/ic/ti.c,v
retrieving revision 1.25
diff -u -p -u -p -r1.25 ti.c
--- sys/dev/ic/ti.c 22 Jan 2017 10:17:38 -  1.25
+++ sys/dev/ic/ti.c 25 Sep 2019 08:06:26 -
@@ -576,7 +576,7 @@ ti_newbuf_std(struct ti_softc *sc, int i
sc->ti_cdata.ti_rx_std_map[i] = dmamap;
 
if (m == NULL) {
-   m_new = MCLGETI(NULL, MCLBYTES, NULL, M_DONTWAIT);
+   m_new = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
if (m_new == NULL)
return (ENOBUFS);
 
@@ -695,7 +695,7 @@ ti_newbuf_jumbo(struct ti_softc *sc, int
bus_dmamap_unload(sc->sc_dmatag, dmamap);
 
if (m == NULL) {
-   m_new = MCLGETI(NULL, TI_JUMBO_FRAMELEN, NULL, M_DONTWAIT);
+   m_new = MCLGETI(NULL, M_DONTWAIT, NULL, TI_JUMBO_FRAMELEN);
if (m_new == NULL)
return (ENOBUFS);
 
Index: sys/dev/pci/if_lge.c
===
RCS file: /cvs/src/sys/dev/pci/if_lge.c,v
retrieving revision 1.73
diff -u -p -u -p -r1.73 if_lge.c
--- sys/dev/pci/if_lge.c22 Jan 2017 10:17:38 -  1.73
+++ sys/dev/pci/if_lge.c25 Sep 2019 08:06:26 -
@@ -626,7 +626,7 @@ lge_newbuf(struct lge_softc *sc, struct 
struct mbuf *m_new = NULL;
 
if (m == NULL) {
-   m_new = MCLGETI(NULL, LGE_JLEN, NULL, M_DONTWAIT);
+   m_new = MCLGETI(NULL, M_DONTWAIT, NULL, LGE_JLEN);
if (m_new == NULL)
return (ENOBUFS);
} else {
Index: sys/dev/pci/if_nfe.c
===
RCS file: /cvs/src/sys/dev/pci/if_nfe.c,v
retrieving revision 1.120
diff -u -p -u -p -r1.120 if_nfe.c
--- sys/dev/pci/if_nfe.c8 Sep 2017 05:36:52 -   1.120
+++ sys/dev/pci/if_nfe.c25 Sep 2019 08:06:26 -
@@ -697,7 +697,7 @@ nfe_rxeof(struct nfe_softc *sc)
 * old mbuf. In the unlikely case that the old mbuf can't be
 * reloaded either, explicitly panic.
 */
-   mnew = MCLGETI(NULL, MCLBYTES, NULL, M_DONTWAIT);
+   mnew = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
if (mnew == NULL) {
ifp->if_ierrors++;
goto skip;
@@ -1210,7 +1210,7 @@ nfe_alloc_rx_ring(struct nfe_softc *sc, 
for (i = 0; i < NFE_RX_RING_COUNT; i++) {
data = >rxq.data[i];
 
-   data->m = MCLGETI(NULL, MCLBYTES, NULL, M_DONTWAIT);
+   data->m = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
if (data->m == NULL) {
printf("%s: could not allocate rx mbuf\n",
sc->sc_dev.dv_xname);
Index: sys/dev/pci/if_nge.c
===
RCS file: /cvs/src/sys/dev/pci/if_nge.c,v
retrieving revision 1.92
diff -u -p -u -p -r1.92 if_nge.c
--- sys/dev/pci/if_nge.c22 Jan 2017 10:17:38 -  1.92
+++ sys/dev/pci/if_nge.c25 Sep 2019 08:06:26 -
@@ -962,7 +962,7 @@ nge_newbuf(struct nge_softc *sc, struct 
struct mbuf *m_new = NULL;
 
if (m == NULL) {
-   m_new = MCLGETI(NULL, NGE_MCLBYTES, NULL, M_DONTWAIT);
+   m_new = MCLGETI(NULL, M_DONTWAIT, NULL, NGE_MCLBYTES);
if (m_new == NULL)
return (ENOBUFS);
} else {



Re: snmpd allow walk on agentx [1/2]

2019-09-25 Thread Martijn van Duren
On 9/25/19 8:55 AM, Martijn van Duren wrote:
> Hello,
> 
> Mischa found that relayd's agentx support is pretty much unusable for 
> the uninitiated, because you have to know the tables beforehand.
> 
> I managed to track it down to two issue with both snmpd and relayd.
> The fix is far from proper support for agentx, but it's good enough
> for going on a walk.
> 
Part 2:
If the requested OID is a predecessor of what we support and the request
is getnext we should retrieve the first available mib.

OK?

martijn@

Index: snmp.c
===
RCS file: /cvs/src/usr.sbin/relayd/snmp.c,v
retrieving revision 1.29
diff -u -p -r1.29 snmp.c
--- snmp.c  28 May 2017 10:39:15 -  1.29
+++ snmp.c  25 Sep 2019 06:58:21 -
@@ -321,21 +321,27 @@ snmp_agentx_process(struct agentx_handle
 
bcopy(, , sizeof(oid));
 
-   /*
-* If the requested OID is not part of the registered
-* MIB, return "no such object", per RFC
-*/
if (snmp_oid_cmp(, ) == -1) {
-   if (snmp_agentx_varbind(resp, ,
-   AGENTX_NO_SUCH_OBJECT, NULL, 0) == -1) {
-   log_warn("%s: unable to generate"
-   " response", __func__);
-   snmp_agentx_pdu_free(resp);
-   resp = NULL;
+   /*
+* If the requested OID is not part of the 
registered
+* MIB, return "no such object", per RFC
+*/
+   if (pdu->hdr->type == AGENTX_GET) {
+   if (snmp_agentx_varbind(resp, ,
+   AGENTX_NO_SUCH_OBJECT, NULL, 0) == 
-1) {
+   log_warn("%s: unable to 
generate"
+   " response", __func__);
+   snmp_agentx_pdu_free(resp);
+   resp = NULL;
+   }
+   goto reply;
}
-   goto reply;
+   bcopy(, , sizeof(oid));
+   }
+   if (oid.o_n == 9) {
+   oid.o_id[9] = 1;
+   oid.o_n++;
}
-
if (oid.o_n != OIDIDX_relaydInfo + 2 + 1) {
/* GET requests require the exact OID */
if (pdu->hdr->type == AGENTX_GET)



snmpd allow walk on agentx [1/2]

2019-09-25 Thread Martijn van Duren
Hello,

Mischa found that relayd's agentx support is pretty much unusable for 
the uninitiated, because you have to know the tables beforehand.

I managed to track it down to two issue with both snmpd and relayd.
The fix is far from proper support for agentx, but it's good enough
for going on a walk.

This mail contains the snmp parts:
1) smi.c: I misread the manpage in that RB_NFIND can also return the
   searched object itself instead of always returning the next one.
   This oversight hasn't introduced any new issues, since the older
   code already returned the object itself.
2) mps.c: We don't look into the parent elements to see if the
   requested element is part of a registered tree.

With both diffs applied:
$ snmp walk -On 127.0.0.1 1.3.6.1.4.1.30155.3   
.1.3.6.1.4.1.30155.3.2.1.1.1 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.1.2.1 = INTEGER: 3
.1.3.6.1.4.1.30155.3.2.1.3.1 = STRING: www
.1.3.6.1.4.1.30155.3.2.1.4.1 = Counter64: 0
.1.3.6.1.4.1.30155.3.2.1.5.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.1.6.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.1.7.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.1.8.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.1.9.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.1.10.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.2.1.2 = INTEGER: 2
.1.3.6.1.4.1.30155.3.2.2.2.2 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.2.3.2 = STRING: httpproxy
.1.3.6.1.4.1.30155.3.2.2.4.2 = Counter64: 0
.1.3.6.1.4.1.30155.3.2.2.5.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.2.6.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.2.7.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.2.8.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.2.9.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.2.10.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.1.1 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.5.1.2 = INTEGER: 2
.1.3.6.1.4.1.30155.3.2.5.1.3 = INTEGER: 3
.1.3.6.1.4.1.30155.3.2.5.2.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.2.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.2.3 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.3.1 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.5.3.2 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.5.3.3 = INTEGER: 2
.1.3.6.1.4.1.30155.3.2.5.4.1 = STRING: 10.0.0.1
.1.3.6.1.4.1.30155.3.2.5.4.2 = STRING: 10.0.0.2
.1.3.6.1.4.1.30155.3.2.5.4.3 = STRING: 127.0.0.1
.1.3.6.1.4.1.30155.3.2.5.5.1 = Hex-STRING: 0A 00 00 01 00 00 00 00 00 00 00 00 
00 00 00 00
.1.3.6.1.4.1.30155.3.2.5.5.2 = Hex-STRING: 0A 00 00 02 00 00 00 00 00 00 00 00 
00 00 00 00
.1.3.6.1.4.1.30155.3.2.5.5.3 = Hex-STRING: 7F 00 00 01 00 00 00 00 00 00 00 00 
00 00 00 00
.1.3.6.1.4.1.30155.3.2.5.6.1 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.5.6.2 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.5.6.3 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.5.7.1 = INTEGER: 2
.1.3.6.1.4.1.30155.3.2.5.7.2 = INTEGER: 2
.1.3.6.1.4.1.30155.3.2.5.7.3 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.8.1 = INTEGER: 12
.1.3.6.1.4.1.30155.3.2.5.8.2 = INTEGER: 12
.1.3.6.1.4.1.30155.3.2.5.8.3 = INTEGER: 12
.1.3.6.1.4.1.30155.3.2.5.9.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.9.2 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.5.9.3 = INTEGER: 12
.1.3.6.1.4.1.30155.3.2.5.10.1 = INTEGER: 10
.1.3.6.1.4.1.30155.3.2.5.10.2 = INTEGER: 10
.1.3.6.1.4.1.30155.3.2.5.10.3 = INTEGER: 3
.1.3.6.1.4.1.30155.3.2.7.1.1 = INTEGER: 1
.1.3.6.1.4.1.30155.3.2.7.1.2 = INTEGER: 2
.1.3.6.1.4.1.30155.3.2.7.2.1 = STRING: webhosts:80
.1.3.6.1.4.1.30155.3.2.7.2.2 = STRING: fallback:80
.1.3.6.1.4.1.30155.3.2.7.3.1 = INTEGER: 0
.1.3.6.1.4.1.30155.3.2.7.3.2 = INTEGER: 0

OK?

martijn@

Index: mps.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mps.c,v
retrieving revision 1.25
diff -u -p -r1.25 mps.c
--- mps.c   16 May 2019 05:00:00 -  1.25
+++ mps.c   25 Sep 2019 06:46:17 -
@@ -208,7 +208,20 @@ mps_getnextreq(struct snmp_message *msg,
bzero(, sizeof(key));
bcopy(o, _id, sizeof(struct ber_oid));
smi_oidlen(_id);  /* Strip off any trailing .0. */
-   value = smi_nfind();
+   do {
+   value = smi_find();
+   if (value->o_flags == 0)
+   value = NULL;
+   if (key.o_id.bo_n != 0)
+   key.o_id.bo_n--;
+   } while (value == NULL && key.o_id.bo_n > 0);
+   if (value == NULL || !(value->o_flags & OID_REGISTERED)) {
+   bcopy(o, _id, sizeof(struct ber_oid));
+   smi_oidlen(_id);  /* Strip off any trailing .0. */
+   value = smi_nfind();
+   while (value != NULL && value->o_flags == 0)
+   value = smi_nfind(value);
+   }
if (value == NULL)
goto fail;
 
Index: smi.c
===
RCS file: /cvs/src/usr.sbin/snmpd/smi.c,v
retrieving revision 1.24
diff -u -p -r1.24 smi.c
--- smi.c   16 May 2019 05:00:00 -  1.24
+++ smi.c   25 Sep 2019 06:46:17 -
@@ -244,7 +244,11 @@ smi_find(struct oid *oid)
 struct oid *
 smi_nfind(struct oid *oid)
 {
-   return (RB_NFIND(oidtree, _oidtree, oid));
+   struct oid *n;
+   n = RB_NFIND(oidtree, _oidtree, oid);
+