Re: avoid scan_dmesg in armv7 md_installboot
hi
seems to make sense and reads fine but I'm not able to check it before tonight.
cheers Robert
Am 22. August 2017 05:04:22 MESZ schrieb Jonathan Gray :
>If there is whitespace in the pattern sed returns scan_dmesg will sort
>all of the words. As hw.product is available on ramdisks use that
>instead to avoid the problem.
>
>Index: install.md
>===
>RCS file: /cvs/src/distrib/armv7/ramdisk/install.md,v
>retrieving revision 1.46
>diff -u -p -r1.46 install.md
>--- install.md 28 Jul 2017 18:15:44 - 1.46
>+++ install.md 22 Aug 2017 02:40:18 -
>@@ -37,8 +37,7 @@ MOUNT_ARGS_msdos="-o-l"
> md_installboot() {
> local _disk=/dev/$1 _mdec _plat
>
>- # Identify ARMv7 platform based on dmesg.
>- case $(scan_dmesg 's/^mainbus0 at root: \(.*\)$/\1/p') in
>+ case $(sysctl -n hw.product) in
> *AM335x*) _plat=am335x;;
> *'OMAP3 BeagleBoard'*) _plat=beagle;;
> *OMAP4*)_plat=panda;;
--
-=[rpe]=-
Negative array index read (ac97.c)
Hi tech,
Check if cp->dev value is invalid prior to using it as an array index and only
then make the assignment si = &as->source_info[cp->dev].
This is mentioned in Coverity CID 1453243 and 1453334.
OK?
Index: ac97.c
===
RCS file: /cvs/src/sys/dev/ic/ac97.c,v
retrieving revision 1.82
diff -u -p -u -r1.82 ac97.c
--- ac97.c 14 Sep 2016 06:12:19 - 1.82
+++ ac97.c 22 Aug 2017 08:03:36 -
@@ -1063,13 +1063,18 @@ int
ac97_mixer_set_port(struct ac97_codec_if *codec_if, mixer_ctrl_t *cp)
{
struct ac97_softc *as = (struct ac97_softc *)codec_if;
- struct ac97_source_info *si = &as->source_info[cp->dev];
+ struct ac97_source_info *si;
u_int16_t mask;
u_int16_t val, newval;
int error, spdif;
if (cp->dev < 0 || cp->dev >= as->num_source_info ||
- cp->type == AUDIO_MIXER_CLASS || cp->type != si->type)
+ cp->type == AUDIO_MIXER_CLASS)
+ return (EINVAL);
+
+ si = &as->source_info[cp->dev];
+
+ if (cp->type != si->type)
return (EINVAL);
spdif = si->req_feature == CHECK_SPDIF &&
@@ -1340,12 +1345,16 @@ int
ac97_mixer_get_port(struct ac97_codec_if *codec_if, mixer_ctrl_t *cp)
{
struct ac97_softc *as = (struct ac97_softc *)codec_if;
- struct ac97_source_info *si = &as->source_info[cp->dev];
+ struct ac97_source_info *si;
u_int16_t mask;
u_int16_t val;
- if (cp->dev < 0 || cp->dev >= as->num_source_info ||
- cp->type != si->type)
+ if (cp->dev < 0 || cp->dev >= as->num_source_info)
+ return (EINVAL);
+
+ si = &as->source_info[cp->dev];
+
+ if (cp->type != si->type)
return (EINVAL);
ac97_read(as, si->reg, &val);
Re: Blocking mem alloc & sosetopt()
On 21/08/17(Mon) 16:21, Martin Pieuchot wrote:
> In order to stop allocating multiple mbufs and possibly waiting with
> the socket lock held in NFS, I'd like sosetopt() to no longer free the
> mbuf it receives.
>
> This diff makes sosetopt() similar to sogetopt(), the caller is
> responsible for allocating & freeing the option mbuf.
Thanks for all the encouragements. visa@ spotted two places in net/bfd.c
where I missed a m_freem(). Updated diff below fixes that. I'd
appreciates more reviews to make sure I don't introduce any double free.
ok?
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.201
diff -u -p -r1.201 uipc_socket.c
--- kern/uipc_socket.c 10 Aug 2017 19:20:43 - 1.201
+++ kern/uipc_socket.c 21 Aug 2017 14:02:56 -
@@ -1558,17 +1558,16 @@ sowwakeup(struct socket *so)
}
int
-sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
+sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
{
int error = 0;
- struct mbuf *m = m0;
soassertlocked(so);
if (level != SOL_SOCKET) {
if (so->so_proto->pr_ctloutput) {
error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
- level, optname, m0);
+ level, optname, m);
return (error);
}
error = ENOPROTOOPT;
@@ -1576,7 +1575,7 @@ sosetopt(struct socket *so, int level, i
switch (optname) {
case SO_BINDANY:
if ((error = suser(curproc, 0)) != 0) /* XXX */
- goto bad;
+ return (error);
break;
}
@@ -1585,10 +1584,8 @@ sosetopt(struct socket *so, int level, i
case SO_LINGER:
if (m == NULL || m->m_len != sizeof (struct linger) ||
mtod(m, struct linger *)->l_linger < 0 ||
- mtod(m, struct linger *)->l_linger > SHRT_MAX) {
- error = EINVAL;
- goto bad;
- }
+ mtod(m, struct linger *)->l_linger > SHRT_MAX)
+ return (EINVAL);
so->so_linger = mtod(m, struct linger *)->l_linger;
/* FALLTHROUGH */
@@ -1602,10 +1599,8 @@ sosetopt(struct socket *so, int level, i
case SO_OOBINLINE:
case SO_TIMESTAMP:
case SO_ZEROIZE:
- if (m == NULL || m->m_len < sizeof (int)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (int))
+ return (EINVAL);
if (*mtod(m, int *))
so->so_options |= optname;
else
@@ -1613,10 +1608,8 @@ sosetopt(struct socket *so, int level, i
break;
case SO_DONTROUTE:
- if (m == NULL || m->m_len < sizeof (int)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (int))
+ return (EINVAL);
if (*mtod(m, int *))
error = EOPNOTSUPP;
break;
@@ -1628,38 +1621,28 @@ sosetopt(struct socket *so, int level, i
{
u_long cnt;
- if (m == NULL || m->m_len < sizeof (int)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (int))
+ return (EINVAL);
cnt = *mtod(m, int *);
if ((long)cnt <= 0)
cnt = 1;
switch (optname) {
case SO_SNDBUF:
- if (so->so_state & SS_CANTSENDMORE) {
- error = EINVAL;
- goto bad;
- }
+ if (so->so_state & SS_CANTSENDMORE)
+ return (EINVAL);
if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
- sbreserve(so, &so->so_snd, cnt)) {
- error = ENOBUFS;
- goto bad;
- }
+ sb
Re: Negative array index read (ac97.c)
Updated version for readability
Index: ac97.c
===
RCS file: /cvs/src/sys/dev/ic/ac97.c,v
retrieving revision 1.82
diff -u -p -u -r1.82 ac97.c
--- ac97.c 14 Sep 2016 06:12:19 - 1.82
+++ ac97.c 22 Aug 2017 08:31:43 -
@@ -1063,13 +1063,17 @@ int
ac97_mixer_set_port(struct ac97_codec_if *codec_if, mixer_ctrl_t *cp)
{
struct ac97_softc *as = (struct ac97_softc *)codec_if;
- struct ac97_source_info *si = &as->source_info[cp->dev];
+ struct ac97_source_info *si;
u_int16_t mask;
u_int16_t val, newval;
int error, spdif;
- if (cp->dev < 0 || cp->dev >= as->num_source_info ||
- cp->type == AUDIO_MIXER_CLASS || cp->type != si->type)
+ if (cp->dev < 0 || cp->dev >= as->num_source_info)
+ return (EINVAL);
+
+ si = &as->source_info[cp->dev];
+
+ if (cp->type == AUDIO_MIXER_CLASS || cp->type != si->type)
return (EINVAL);
spdif = si->req_feature == CHECK_SPDIF &&
@@ -1340,12 +1344,16 @@ int
ac97_mixer_get_port(struct ac97_codec_if *codec_if, mixer_ctrl_t *cp)
{
struct ac97_softc *as = (struct ac97_softc *)codec_if;
- struct ac97_source_info *si = &as->source_info[cp->dev];
+ struct ac97_source_info *si;
u_int16_t mask;
u_int16_t val;
- if (cp->dev < 0 || cp->dev >= as->num_source_info ||
- cp->type != si->type)
+ if (cp->dev < 0 || cp->dev >= as->num_source_info)
+ return (EINVAL);
+
+ si = &as->source_info[cp->dev];
+
+ if (cp->type != si->type)
return (EINVAL);
ac97_read(as, si->reg, &val);
isakmpd leak on error
By reviewing my last isakmpd(8) diff to fix a use-after-free, hshoexer@
pointed out that if exchange_establish() fails, `arg' is leaked. This is
not a new issue. However it generally happens under memory pressure,
and when you're under memory pressure leaking memory is not a clever
thing to do.
In order to prevent this memory leak, we have to:
- check for failures of exchange_establish_p{1,2}() and call the given
`finalize' function with the `fail' argument when this happen.
Because now the various finalize functions correctly free their
corresponding argument.
- introduce some sanity checks in exchange_free() to be able to call
if even if the data structure isn't completely initialized.
Diff below does that. It also includes a fix for a double free in one
of the error paths of exchange_establish().
ok?
Index: exchange.c
===
RCS file: /cvs/src/sbin/isakmpd/exchange.c,v
retrieving revision 1.138
diff -u -p -r1.138 exchange.c
--- exchange.c 10 Mar 2016 07:32:16 - 1.138
+++ exchange.c 22 Aug 2017 08:45:21 -
@@ -529,6 +529,7 @@ exchange_enter(struct exchange *exchange
}
bucket &= bucket_mask;
LIST_INSERT_HEAD(&exchange_tab[bucket], exchange, link);
+ exchange->linked = 1;
}
/*
@@ -703,7 +704,7 @@ exchange_establish_transaction(struct ex
}
/* Establish a phase 1 exchange. */
-void
+int
exchange_establish_p1(struct transport *t, u_int8_t type, u_int32_t doi,
char *name, void *args, void (*finalize)(struct exchange *, void *, int),
void *arg, int stayalive)
@@ -738,7 +739,7 @@ exchange_establish_p1(struct transport *
else {
log_print("exchange_establish_p1: "
"DOI \"%s\" unsupported", str);
- return;
+ return -1;
}
/* What exchange type do we want? */
@@ -747,20 +748,19 @@ exchange_establish_p1(struct transport *
log_print("exchange_establish_p1: "
"no \"EXCHANGE_TYPE\" tag in [%s] section",
tag);
- return;
+ return -1;
}
type = constant_value(isakmp_exch_cst, str);
if (!type) {
log_print("exchange_establish_p1: "
"unknown exchange type %s", str);
- return;
+ return -1;
}
}
}
exchange = exchange_create(1, 1, doi, type);
if (!exchange) {
- /* XXX Do something here? */
- return;
+ return -1;
}
if (name) {
exchange->name = strdup(name);
@@ -768,7 +768,7 @@ exchange_establish_p1(struct transport *
log_error("exchange_establish_p1: "
"strdup (\"%s\") failed", name);
exchange_free(exchange);
- return;
+ return -1;
}
}
exchange->policy = name ? conf_get_str(name, "Configuration") : 0;
@@ -787,7 +787,7 @@ exchange_establish_p1(struct transport *
"calloc (1, %lu) failed",
(unsigned long)sizeof(*node));
exchange_free(exchange);
- return;
+ return -1;
}
/*
* Insert this finalization inbetween
@@ -813,7 +813,7 @@ exchange_establish_p1(struct transport *
if (!msg) {
log_print("exchange_establish_p1: message_alloc () failed");
exchange_free(exchange);
- return;
+ return 0; /* exchange_free() runs finalize */
}
msg->exchange = exchange;
@@ -828,10 +828,9 @@ exchange_establish_p1(struct transport *
sa_create(exchange, 0);
msg->isakmp_sa = TAILQ_FIRST(&exchange->sa_list);
if (!msg->isakmp_sa) {
- /* XXX Do something more here? */
message_free(msg);
exchange_free(exchange);
- return;
+ return 0; /* exchange_free() runs finalize */
}
sa_reference(msg->isakmp_sa);
@@ -841,10 +840,11 @@ exchange_establish_p1(struct transport *
msg->extra = args;
exchange_run(msg);
+ return 0;
}
/* Establish a phase 2 exchange. XXX With just one SA for now. *
Re: avoid scan_dmesg in armv7 md_installboot
> Date: Tue, 22 Aug 2017 13:04:22 +1000
> From: Jonathan Gray
>
> If there is whitespace in the pattern sed returns scan_dmesg will sort
> all of the words. As hw.product is available on ramdisks use that
> instead to avoid the problem.
Makes sense to me. ok kettenis@
In the long run we may want to export the "compatible" string from the
device tree through sysctl and use that. Or maybe even teach
installboot to install/update the firmware.
> Index: install.md
> ===
> RCS file: /cvs/src/distrib/armv7/ramdisk/install.md,v
> retrieving revision 1.46
> diff -u -p -r1.46 install.md
> --- install.md28 Jul 2017 18:15:44 - 1.46
> +++ install.md22 Aug 2017 02:40:18 -
> @@ -37,8 +37,7 @@ MOUNT_ARGS_msdos="-o-l"
> md_installboot() {
> local _disk=/dev/$1 _mdec _plat
>
> - # Identify ARMv7 platform based on dmesg.
> - case $(scan_dmesg 's/^mainbus0 at root: \(.*\)$/\1/p') in
> + case $(sysctl -n hw.product) in
> *AM335x*) _plat=am335x;;
> *'OMAP3 BeagleBoard'*) _plat=beagle;;
> *OMAP4*)_plat=panda;;
>
>
Re: Negative array index read (ac97.c)
On Tue, Aug 22, 2017 at 09:33:56AM +0100, Ricardo Mestre wrote:
> Updated version for readability
ok jsg@
>
> Index: ac97.c
> ===
> RCS file: /cvs/src/sys/dev/ic/ac97.c,v
> retrieving revision 1.82
> diff -u -p -u -r1.82 ac97.c
> --- ac97.c14 Sep 2016 06:12:19 - 1.82
> +++ ac97.c22 Aug 2017 08:31:43 -
> @@ -1063,13 +1063,17 @@ int
> ac97_mixer_set_port(struct ac97_codec_if *codec_if, mixer_ctrl_t *cp)
> {
> struct ac97_softc *as = (struct ac97_softc *)codec_if;
> - struct ac97_source_info *si = &as->source_info[cp->dev];
> + struct ac97_source_info *si;
> u_int16_t mask;
> u_int16_t val, newval;
> int error, spdif;
>
> - if (cp->dev < 0 || cp->dev >= as->num_source_info ||
> - cp->type == AUDIO_MIXER_CLASS || cp->type != si->type)
> + if (cp->dev < 0 || cp->dev >= as->num_source_info)
> + return (EINVAL);
> +
> + si = &as->source_info[cp->dev];
> +
> + if (cp->type == AUDIO_MIXER_CLASS || cp->type != si->type)
> return (EINVAL);
>
> spdif = si->req_feature == CHECK_SPDIF &&
> @@ -1340,12 +1344,16 @@ int
> ac97_mixer_get_port(struct ac97_codec_if *codec_if, mixer_ctrl_t *cp)
> {
> struct ac97_softc *as = (struct ac97_softc *)codec_if;
> - struct ac97_source_info *si = &as->source_info[cp->dev];
> + struct ac97_source_info *si;
> u_int16_t mask;
> u_int16_t val;
>
> - if (cp->dev < 0 || cp->dev >= as->num_source_info ||
> - cp->type != si->type)
> + if (cp->dev < 0 || cp->dev >= as->num_source_info)
> + return (EINVAL);
> +
> + si = &as->source_info[cp->dev];
> +
> + if (cp->type != si->type)
> return (EINVAL);
>
> ac97_read(as, si->reg, &val);
>
Race against ipsec_in_use
Diff below fixes a race reported by Hrvoje Popovski. If `ipsec_in_use'
is enabled after the softnettq checked for it but before it grabbed the
NET_LOCK(), IPsec code will be executed without KERNEL_LOCK().
ipsec_in_use is protected by the NET_LOCK(), so it has to be checked
after grabbing it.
ok?
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.511
diff -u -p -r1.511 if.c
--- net/if.c12 Aug 2017 20:27:28 - 1.511
+++ net/if.c22 Aug 2017 10:26:49 -
@@ -887,19 +887,6 @@ if_input_process(void *xifidx)
if (!ISSET(ifp->if_xflags, IFXF_CLONED))
add_net_randomness(ml_len(&ml));
-#ifdef IPSEC
- /*
-* IPsec is not ready to run without KERNEL_LOCK(). So all
-* the traffic on your machine is punished if you have IPsec
-* enabled.
-*/
- extern int ipsec_in_use;
- if (ipsec_in_use) {
- KERNEL_LOCK();
- locked = 1;
- }
-#endif /* IPSEC */
-
/*
* We grab the NET_LOCK() before processing any packet to
* ensure there's no contention on the routing table lock.
@@ -914,6 +901,22 @@ if_input_process(void *xifidx)
*/
NET_LOCK();
s = splnet();
+
+#ifdef IPSEC
+ /*
+* IPsec is not ready to run without KERNEL_LOCK(). So all
+* the traffic on your machine is punished if you have IPsec
+* enabled.
+*/
+ extern int ipsec_in_use;
+ if (ipsec_in_use) {
+ NET_UNLOCK();
+ KERNEL_LOCK();
+ NET_LOCK();
+ locked = 1;
+ }
+#endif /* IPSEC */
+
while ((m = ml_dequeue(&ml)) != NULL) {
/*
* Pass this mbuf to all input handlers of its
Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.318
diff -u -p -r1.318 ip_input.c
--- netinet/ip_input.c 11 Aug 2017 21:24:20 - 1.318
+++ netinet/ip_input.c 22 Aug 2017 10:27:09 -
@@ -1802,6 +1802,8 @@ ip_send_dispatch(void *xmq)
if (ml_empty(&ml))
return;
+ NET_LOCK();
+
#ifdef IPSEC
/*
* IPsec is not ready to run without KERNEL_LOCK(). So all
@@ -1810,12 +1812,13 @@ ip_send_dispatch(void *xmq)
*/
extern int ipsec_in_use;
if (ipsec_in_use) {
+ NET_UNLOCK();
KERNEL_LOCK();
+ NET_LOCK();
locked = 1;
}
#endif /* IPSEC */
- NET_LOCK();
while ((m = ml_dequeue(&ml)) != NULL) {
ip_output(m, NULL, NULL, 0, NULL, NULL, 0);
}
Index: netinet6/ip6_input.c
===
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.201
diff -u -p -r1.201 ip6_input.c
--- netinet6/ip6_input.c11 Aug 2017 21:24:20 - 1.201
+++ netinet6/ip6_input.c22 Aug 2017 10:28:14 -
@@ -1450,6 +1450,8 @@ ip6_send_dispatch(void *xmq)
if (ml_empty(&ml))
return;
+ NET_LOCK();
+
#ifdef IPSEC
/*
* IPsec is not ready to run without KERNEL_LOCK(). So all
@@ -1458,12 +1460,13 @@ ip6_send_dispatch(void *xmq)
*/
extern int ipsec_in_use;
if (ipsec_in_use) {
+ NET_UNLOCK();
KERNEL_LOCK();
+ NET_LOCK();
locked = 1;
}
#endif /* IPSEC */
- NET_LOCK();
while ((m = ml_dequeue(&ml)) != NULL) {
ip6_output(m, NULL, NULL, IPV6_MINMTU, NULL, NULL);
}
Re: Blocking mem alloc & sosetopt()
On Tue, Aug 22, 2017 at 10:13:31AM +0200, Martin Pieuchot wrote: > ok? OK bluhm@ > @@ -1310,7 +1308,6 @@ ip_pcbopts(struct mbuf **pcbopt, struct > return (0); > > bad: > - (void)m_free(m); > return (EINVAL); > } You could replace all goto bad with return (EINVAL).
Re: Race against ipsec_in_use
On Tue, Aug 22, 2017 at 01:13:40PM +0200, Martin Pieuchot wrote: > ipsec_in_use is protected by the NET_LOCK(), so it has to be checked > after grabbing it. > > ok? OK bluhm@
cu(1) aborted by pledge on a non-TTY device
Hi, Today while trying to connect to my ERL via serial with cu(1) instead of cuaU0 I used usb0 (yes, I keep forgetting the correct one). But this is actually the first time I typed it wrong and ran cu(1) with it and strangely enough saw it being aborted by pledge saying that ioctl(TIOCEXCL) was not allowed. Since with the correct device it works I checked pledge and the chunck below means that the break will make pledge_ioctl return with errno EPERM complaining about pledge, whereas perhaps it should return ENOTTY instead? This way it will say that ioctl(TIOCEXCL) is not an appropriate ioctl for the device used. Comments? Is there a better way to handle a non-TTY in this situation? Change cu(1) to check if it's a TTY or not prior to invoke the ioctl? Index: kern_pledge.c === RCS file: /cvs/src/sys/kern/kern_pledge.c,v retrieving revision 1.218 diff -u -p -u -r1.218 kern_pledge.c --- kern_pledge.c 21 Aug 2017 14:40:07 - 1.218 +++ kern_pledge.c 22 Aug 2017 15:35:13 - @@ -1298,7 +1298,7 @@ pledge_ioctl(struct proc *p, long com, s case TIOCSCTTY: /* forkpty(3), login_tty(3), ... */ if (fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY)) return (0); - break; + return (ENOTTY); } }
vi tilde expands without shell
Hi,
vi(1) currently has support for filename tab expanding, but to expand the
tilde '~' character to $HOME it has to vfork/exec ksh. Yikes.
770 vi CALL execve(0x2cf0b7bce80,0x7f7bf960,0x2cef006cc00)
770 vi NAMI "/bin/ksh"
770 ksh RET execve 0
90346 vi RET vfork 770/0x302
This also conflicts with the -S or 'secure' option which prevents
executing external commands using pledge(2).
"Shell expansions not supported when the secure edit option is set."
It seems to work with my use case, but it would be helpful if other
vi users could take a look!
-Bryan.
Index: ex/ex_argv.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_argv.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 ex_argv.c
--- usr.bin/vi/ex/ex_argv.c 27 May 2016 09:18:12 - 1.20
+++ usr.bin/vi/ex/ex_argv.c 22 Aug 2017 18:20:25 -
@@ -320,6 +320,21 @@ argv_fexp(SCR *sp, EXCMD *excp, char *cm
p += tlen;
F_SET(excp, E_MODIFY);
break;
+ case '~':
+ if ((t = getenv("HOME")) == NULL || *t == '\0') {
+ msgq(sp, M_ERR,
+ "Unable to substitute HOME directory");
+ return (1);
+ }
+ tlen = strlen(t);
+ len += tlen;
+ off = p - bp;
+ ADD_SPACE_RET(sp, bp, blen, len);
+ p = bp + off;
+ memcpy(p, t, tlen);
+ p += tlen;
+ F_SET(excp, E_MODIFY);
+ break;
case '%':
if ((t = sp->frp->name) == NULL) {
msgq(sp, M_ERR,
partial compat checking w/OF_is_compatible() ?
Hi,
i came across dwc2 usb node like this:
usb@101c {
compatible = "snps,dwc2";
reg = <0x101c 0x4>;
interrupts = <0x0 0x11 0x4>;
clocks = <0x2 0x1c9>;
clock-names = "otg";
dr_mode = "host";
phys = <0x6>;
phy-names = "usb2-phy";
status = "okay";
};
and as it's at /-root, there's not much i could use to identify between
broadcom & rockchip while keeping the code simple for future additions,
and not mess w/each SoC compat string individually when applying vendor-
specific things..
i guess this should not be abused ever alone w/o some && in any _match(),
but when used responsibly may make some _attach() funcs prettier:)
to export the _vend variant, or keep as strchr hack like below?
(untested diff rly, for just the discussion atm.)
-Artturi
diff --git a/sys/dev/ofw/fdt.c b/sys/dev/ofw/fdt.c
index d6a64499e88..f22ed7a9f49 100644
--- a/sys/dev/ofw/fdt.c
+++ b/sys/dev/ofw/fdt.c
@@ -42,6 +42,7 @@ intfdt_translate_reg(void *, struct fdt_reg *);
#ifdef DEBUG
voidfdt_print_node_recurse(void *, int);
#endif
+static int fdt_is_compatible_vend(void *, const char *);
static int tree_inited = 0;
static struct fdt tree;
@@ -634,6 +635,24 @@ fdt_is_compatible(void *node, const char *name)
return 0;
}
+int
+fdt_is_compatible_vend(void *node, const char *name)
+{
+ size_t vlen = strlen(name);
+ char *data;
+ int len;
+
+ len = fdt_node_property(node, "compatible", &data);
+ while (len > 0) {
+ if (strncmp(data, name, vlen) == 0)
+ return 1;
+ len -= strlen(data) + 1;
+ data += strlen(data) + 1;
+ }
+
+ return 0;
+}
+
#ifdef DEBUG
/*
* Debug methods for printing whole tree, particular odes and properies
@@ -902,6 +921,8 @@ int
OF_is_compatible(int handle, const char *name)
{
void *node = (char *)tree.header + handle;
+ if (strchr(name, ',') == NULL)
+ return fdt_is_compatible_vend(node, name);
return (fdt_is_compatible(node, name));
}
Re: partial compat checking w/OF_is_compatible() ?
> Date: Tue, 22 Aug 2017 22:12:51 +0300
> From: Artturi Alm
>
> Hi,
>
> i came across dwc2 usb node like this:
>
> usb@101c {
> compatible = "snps,dwc2";
> reg = <0x101c 0x4>;
> interrupts = <0x0 0x11 0x4>;
> clocks = <0x2 0x1c9>;
> clock-names = "otg";
> dr_mode = "host";
> phys = <0x6>;
> phy-names = "usb2-phy";
> status = "okay";
> };
>
>
> and as it's at /-root, there's not much i could use to identify between
> broadcom & rockchip while keeping the code simple for future additions,
> and not mess w/each SoC compat string individually when applying vendor-
> specific things..
>
> i guess this should not be abused ever alone w/o some && in any _match(),
> but when used responsibly may make some _attach() funcs prettier:)
>
> to export the _vend variant, or keep as strchr hack like below?
> (untested diff rly, for just the discussion atm.)
All the nodes I have seen have a more specific string included in the
compatible property. Also, many, if not all of the parameters have
sane default values or values that can be read from a configuration
register.
So no, I don't think what you're proposing is a good idea.
Regarding your mail from a few days ago. I did try something like
your diff to get the usb ports on the rk3288-tinker to work but failed
so far.
> diff --git a/sys/dev/ofw/fdt.c b/sys/dev/ofw/fdt.c
> index d6a64499e88..f22ed7a9f49 100644
> --- a/sys/dev/ofw/fdt.c
> +++ b/sys/dev/ofw/fdt.c
> @@ -42,6 +42,7 @@ int fdt_translate_reg(void *, struct fdt_reg *);
> #ifdef DEBUG
> void fdt_print_node_recurse(void *, int);
> #endif
> +static int fdt_is_compatible_vend(void *, const char *);
>
> static int tree_inited = 0;
> static struct fdt tree;
> @@ -634,6 +635,24 @@ fdt_is_compatible(void *node, const char *name)
> return 0;
> }
>
> +int
> +fdt_is_compatible_vend(void *node, const char *name)
> +{
> + size_t vlen = strlen(name);
> + char *data;
> + int len;
> +
> + len = fdt_node_property(node, "compatible", &data);
> + while (len > 0) {
> + if (strncmp(data, name, vlen) == 0)
> + return 1;
> + len -= strlen(data) + 1;
> + data += strlen(data) + 1;
> + }
> +
> + return 0;
> +}
> +
> #ifdef DEBUG
> /*
> * Debug methods for printing whole tree, particular odes and properies
> @@ -902,6 +921,8 @@ int
> OF_is_compatible(int handle, const char *name)
> {
> void *node = (char *)tree.header + handle;
> + if (strchr(name, ',') == NULL)
> + return fdt_is_compatible_vend(node, name);
> return (fdt_is_compatible(node, name));
> }
>
>
Re: vi tilde expands without shell
That will only work for ~/foo and not ~other/bar. Wouldn't it make sense to use glob(3) in argv_exp2()? - todd
relayd interrupted transfers
Hi!
relay_error() sets se_done even if write buffer is not drained and
relay_write() will close the connection if se_done is set
I have a case with 76k json payload where relay_error() detects EOF
on read socket, sets so_done but write socket is not drained yet
and socket is closed before all the content is transfered.
debug output:
version: HTTP/1.1 rescode: 200 resmsg: OK
relay_writeheader_kv: Connection: close
relay_writeheader_kv: Content-Length: 76854
relay_writeheader_kv: Content-Type: application/json;charset=utf-8
relay_read_httpcontent: session 1: size 3752, to read 76854
relay_read_httpcontent: done, size 3752, to read 73102
relay_read_httpcontent: session 1: size 16384, to read 73102
relay_read_httpcontent: done, size 16384, to read 56718
relay_write buffer len 4081 se_done 0
relay_read_httpcontent: session 1: size 56718, to read 56718
relay_read_httpcontent: done, size 56718, to read 0
relay_read_http: session 1: size 0, to read -2
relay_write buffer len 44415 se_done 0
relay_write buffer len 28031 se_done 1
relay test, session 1 (1 active), 0, 1.2.3.4 -> 5.5.66.66:, last write
(done), GET
fix:
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.225
diff -u -p -r1.225 relay.c
--- usr.sbin/relayd/relay.c 9 Aug 2017 21:29:17 - 1.225
+++ usr.sbin/relayd/relay.c 22 Aug 2017 17:07:33 -
@@ -788,9 +788,12 @@ relay_write(struct bufferevent *bev, voi
{
struct ctl_relay_event *cre = arg;
struct rsession *con = cre->con;
+ struct evbuffer *dst = EVBUFFER_OUTPUT(bev);
getmonotime(&con->se_tv_last);
+ if (EVBUFFER_LENGTH(dst))
+ return;
if (con->se_done)
goto done;
if (relay_splice(cre->dst) == -1)
@@ -962,7 +965,6 @@ relay_error(struct bufferevent *bev, sho
{
struct ctl_relay_event *cre = arg;
struct rsession *con = cre->con;
- struct evbuffer *dst;
if (error & EVBUFFER_TIMEOUT) {
if (cre->splicelen >= 0) {
@@ -1017,9 +1019,7 @@ relay_error(struct bufferevent *bev, sho
con->se_done = 1;
if (cre->dst->bev != NULL) {
- dst = EVBUFFER_OUTPUT(cre->dst->bev);
- if (EVBUFFER_LENGTH(dst))
- return;
+ return;
} else if (cre->toread == TOREAD_UNLIMITED || cre->toread == 0)
return;
Rivo
unbound 1.6.5, fix trust anchor tracking
Update to 1.6.5 to fix trust anchor installation when two anchors
are present. Concretely, fixes root.key creation if unbound is
installed between sep11 and oct11 2017.
Only one real file changed, I placed it at the top of the diff,
then there's a bunch of autoconf/manpage vomit below.
ok?
Index: validator/autotrust.c
===
RCS file: /cvs/src/usr.sbin/unbound/validator/autotrust.c,v
retrieving revision 1.6
diff -u -p -r1.6 autotrust.c
--- validator/autotrust.c 17 Feb 2017 18:53:32 - 1.6
+++ validator/autotrust.c 22 Aug 2017 20:12:28 -
@@ -1571,6 +1571,11 @@ key_matches_a_ds(struct module_env* env,
verbose(VERB_ALGO, "DS match attempt failed");
continue;
}
+ /* match of hash is sufficient for bootstrap of trust point */
+ (void)reason;
+ (void)ve;
+ return 1;
+ /* no need to check RRSIG, DS hash already matched with source
if(dnskey_verify_rrset(env, ve, dnskey_rrset,
dnskey_rrset, key_idx, &reason) == sec_status_secure) {
return 1;
@@ -1578,6 +1583,7 @@ key_matches_a_ds(struct module_env* env,
verbose(VERB_ALGO, "DS match failed because the key "
"does not verify the keyset: %s", reason);
}
+ */
}
return 0;
}
Index: aclocal.m4
===
RCS file: /cvs/src/usr.sbin/unbound/aclocal.m4,v
retrieving revision 1.3
diff -u -p -r1.3 aclocal.m4
--- aclocal.m4 17 Feb 2017 18:53:31 - 1.3
+++ aclocal.m4 22 Aug 2017 20:12:27 -
@@ -9044,9 +9044,9 @@ m4_ifndef([_LT_PROG_F77], [AC_DEFUN([_L
m4_ifndef([_LT_PROG_FC], [AC_DEFUN([_LT_PROG_FC])])
m4_ifndef([_LT_PROG_CXX], [AC_DEFUN([_LT_PROG_CXX])])
-dnl pkg.m4 - Macros to locate and utilise pkg-config. -*- Autoconf -*-
-dnl serial 11 (pkg-config-0.29.1)
-dnl
+# pkg.m4 - Macros to locate and utilise pkg-config. -*- Autoconf -*-
+# serial 11 (pkg-config-0.29.1)
+
dnl Copyright © 2004 Scott James Remnant .
dnl Copyright © 2012-2015 Dan Nicholson
dnl
@@ -9319,6 +9319,74 @@ AS_VAR_COPY([$1], [pkg_cv_][$1])
AS_VAR_IF([$1], [""], [$5], [$4])dnl
])dnl PKG_CHECK_VAR
+
+dnl PKG_WITH_MODULES(VARIABLE-PREFIX, MODULES,
+dnl [ACTION-IF-FOUND],[ACTION-IF-NOT-FOUND],
+dnl [DESCRIPTION], [DEFAULT])
+dnl --
+dnl
+dnl Prepare a "--with-" configure option using the lowercase
+dnl [VARIABLE-PREFIX] name, merging the behaviour of AC_ARG_WITH and
+dnl PKG_CHECK_MODULES in a single macro.
+AC_DEFUN([PKG_WITH_MODULES],
+[
+m4_pushdef([with_arg], m4_tolower([$1]))
+
+m4_pushdef([description],
+ [m4_default([$5], [build with ]with_arg[ support])])
+
+m4_pushdef([def_arg], [m4_default([$6], [auto])])
+m4_pushdef([def_action_if_found], [AS_TR_SH([with_]with_arg)=yes])
+m4_pushdef([def_action_if_not_found], [AS_TR_SH([with_]with_arg)=no])
+
+m4_case(def_arg,
+[yes],[m4_pushdef([with_without], [--without-]with_arg)],
+[m4_pushdef([with_without],[--with-]with_arg)])
+
+AC_ARG_WITH(with_arg,
+ AS_HELP_STRING(with_without, description[ @<:@default=]def_arg[@:>@]),,
+[AS_TR_SH([with_]with_arg)=def_arg])
+
+AS_CASE([$AS_TR_SH([with_]with_arg)],
+[yes],[PKG_CHECK_MODULES([$1],[$2],$3,$4)],
+[auto],[PKG_CHECK_MODULES([$1],[$2],
+[m4_n([def_action_if_found]) $3],
+[m4_n([def_action_if_not_found]) $4])])
+
+m4_popdef([with_arg])
+m4_popdef([description])
+m4_popdef([def_arg])
+
+])dnl PKG_WITH_MODULES
+
+dnl PKG_HAVE_WITH_MODULES(VARIABLE-PREFIX, MODULES,
+dnl [DESCRIPTION], [DEFAULT])
+dnl ---
+dnl
+dnl Convenience macro to trigger AM_CONDITIONAL after PKG_WITH_MODULES
+dnl check._[VARIABLE-PREFIX] is exported as make variable.
+AC_DEFUN([PKG_HAVE_WITH_MODULES],
+[
+PKG_WITH_MODULES([$1],[$2],,,[$3],[$4])
+
+AM_CONDITIONAL([HAVE_][$1],
+ [test "$AS_TR_SH([with_]m4_tolower([$1]))" = "yes"])
+])dnl PKG_HAVE_WITH_MODULES
+
+dnl PKG_HAVE_DEFINE_WITH_MODULES(VARIABLE-PREFIX, MODULES,
+dnl [DESCRIPTION], [DEFAULT])
+dnl --
+dnl
+dnl Convenience macro to run AM_CONDITIONAL and AC_DEFINE after
+dnl PKG_WITH_MODULES check. HAVE_[VARIABLE-PREFIX] is exported as make
+dnl and preprocessor variable.
+AC_DEFUN([PKG_HAVE_DEFINE_WITH_MODULES],
+[
+PKG_HAVE_WITH_MODULES([$1],[$2],[$3],[$4])
+
+AS_IF([test "$AS_TR_SH([with_]m4_tolower([$1]))" = "yes"],
+[AC_DEFINE([HAVE_][$1], 1, [Enable ]m4_tolower([$1])[ support])])
+])dnl PKG_HAVE_DEFINE_WITH_MODULES
# AM_CONDITIONAL-*-
Re: partial compat checking w/OF_is_compatible() ?
On Tue, Aug 22, 2017 at 09:47:27PM +0200, Mark Kettenis wrote:
> > Date: Tue, 22 Aug 2017 22:12:51 +0300
> > From: Artturi Alm
> >
> > Hi,
> >
> > i came across dwc2 usb node like this:
> >
> > usb@101c {
> > compatible = "snps,dwc2";
> > reg = <0x101c 0x4>;
> > interrupts = <0x0 0x11 0x4>;
> > clocks = <0x2 0x1c9>;
> > clock-names = "otg";
> > dr_mode = "host";
> > phys = <0x6>;
> > phy-names = "usb2-phy";
> > status = "okay";
> > };
> >
> >
> > and as it's at /-root, there's not much i could use to identify between
> > broadcom & rockchip while keeping the code simple for future additions,
> > and not mess w/each SoC compat string individually when applying vendor-
> > specific things..
> >
> > i guess this should not be abused ever alone w/o some && in any _match(),
> > but when used responsibly may make some _attach() funcs prettier:)
> >
> > to export the _vend variant, or keep as strchr hack like below?
> > (untested diff rly, for just the discussion atm.)
>
> All the nodes I have seen have a more specific string included in the
> compatible property. Also, many, if not all of the parameters have
> sane default values or values that can be read from a configuration
> register.
>
> So no, I don't think what you're proposing is a good idea.
>
ok, i'll try to get this fixed in u-boot.
but incase i was unclear, the diff was supposed to allow ie. this where the
specifics obviously doesn't matter beyond "," after vendor in compatible:
diff --git a/sys/dev/fdt/if_dwge_fdt.c b/sys/dev/fdt/if_dwge_fdt.c
index edfe5acb992..a874274c215 100644
--- a/sys/dev/fdt/if_dwge_fdt.c
+++ b/sys/dev/fdt/if_dwge_fdt.c
@@ -120,9 +120,7 @@ dwge_fdt_attach(struct device *parent, struct device *self,
void *aux)
/* Do hardware specific initializations. */
if (OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-gmac"))
dwge_fdt_attach_allwinner(fsc);
- else if (OF_is_compatible(faa->fa_node, "rockchip,rk3288-gmac"))
- dwge_fdt_attach_rockchip(fsc);
- else if (OF_is_compatible(faa->fa_node, "rockchip,rk3399-gmac"))
+ else if (OF_is_compatible(faa->fa_node, "rockchip"))
dwge_fdt_attach_rockchip(fsc);
/* Enable clock. */
> Regarding your mail from a few days ago. I did try something like
> your diff to get the usb ports on the rk3288-tinker to work but failed
> so far.
>
u-boot is still rather broken for rk3188, and success-rate on even u-boot
finding the attached devices is less than 1/10... but one try got this far:
U-Boot SPL 2017.09-rc2-00151-g2d7cb5b-dirty (Aug 21 2017 - 18:15:17)
Returning to boot ROM...
U-Boot 2017.09-rc2-00151-g2d7cb5b-dirty (Aug 21 2017 - 18:15:17 +0300)
Model: Radxa Rock
DRAM: 2 GiB
MMC: dwmmc@10214000 - probe failed: -22
*** Warning - No MMC card found, using default environment
In:serial@20064000
Out: serial@20064000
Err: serial@20064000
Model: Radxa Rock
Net: Net Initialization Skipped
No ethernet found.
Hit any key to stop autoboot: 0
starting USB...
USB0: Core Release: 2.91a
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
dwc_otg_core_host_init: Timeout!
USB1: Core Release: 2.91a
scanning bus 0 for devices... 1 USB Device(s) found
scanning bus 1 for devices... 5 USB Device(s) found
scanning usb for storage devices... 1 Storage Device(s) found
scanning usb for ethernet devices... 0 Ethernet Device(s) found
Device 0: Vendor: Kingston Rev: PMAP Prod: DataTraveler 2.0
Type: Removable Hard Disk
Capacity: 14891.4 MB = 14.5 GB (30497664 x 512)
... is now current device
Scanning usb 0:1...
Found EFI removable media binary efi/boot/bootarm.efi
reading efi/boot/bootarm.efi
64908 bytes read in 70 ms (905.3 KiB/s)
libfdt fdt_check_header(): FDT_ERR_BADMAGIC
## Starting EFI application at 6200 ...
Scanning disk dw...@10214000.blk...
Scanning disk usb_mass_storage.lun0...
Found 2 disks
>> OpenBSD/armv7 BOOTARM 0.8
boot>
booting sd0a:/bsd: 3887248+166476+499544 [281845+90+515968+242685]=0x558c3c
OpenBSD/armv7 booting ...
arg0 0xc0858c3c arg1 0x0 arg2 0x6800
Allocating page tables
freestart = 0x60859000, free_pages = 522151 (0x0007f7a7)
IRQ stack: p0x60887000 v0xc0887000
ABT stack: p0x60888000 v0xc0888000
UND stack: p0x60889000 v0xc0889000
SVC stack: p0x6088a000 v0xc088a000
Creating L1 page table at 0x6085c000
Mapping kernel
Constructing L2 page tables
undefined page pmap [ using 1041048 bytes of bsd ELF symbol table ]
board type: 0
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California. All rights reserved.
Copyright (c) 1995-2017 OpenBSD. All right
Re: partial compat checking w/OF_is_compatible() ?
> Date: Wed, 23 Aug 2017 00:17:01 +0300
> From: Artturi Alm
>
> On Tue, Aug 22, 2017 at 09:47:27PM +0200, Mark Kettenis wrote:
> > > Date: Tue, 22 Aug 2017 22:12:51 +0300
> > > From: Artturi Alm
> > >
> > > Hi,
> > >
> > > i came across dwc2 usb node like this:
> > >
> > > usb@101c {
> > > compatible = "snps,dwc2";
> > > reg = <0x101c 0x4>;
> > > interrupts = <0x0 0x11 0x4>;
> > > clocks = <0x2 0x1c9>;
> > > clock-names = "otg";
> > > dr_mode = "host";
> > > phys = <0x6>;
> > > phy-names = "usb2-phy";
> > > status = "okay";
> > > };
> > >
> > >
> > > and as it's at /-root, there's not much i could use to identify between
> > > broadcom & rockchip while keeping the code simple for future additions,
> > > and not mess w/each SoC compat string individually when applying vendor-
> > > specific things..
> > >
> > > i guess this should not be abused ever alone w/o some && in any _match(),
> > > but when used responsibly may make some _attach() funcs prettier:)
> > >
> > > to export the _vend variant, or keep as strchr hack like below?
> > > (untested diff rly, for just the discussion atm.)
> >
> > All the nodes I have seen have a more specific string included in the
> > compatible property. Also, many, if not all of the parameters have
> > sane default values or values that can be read from a configuration
> > register.
> >
> > So no, I don't think what you're proposing is a good idea.
> >
>
> ok, i'll try to get this fixed in u-boot.
>
> but incase i was unclear, the diff was supposed to allow ie. this where the
> specifics obviously doesn't matter beyond "," after vendor in compatible:
No. That is now how this stuff is supposed to work. You should drop
this idea.
> diff --git a/sys/dev/fdt/if_dwge_fdt.c b/sys/dev/fdt/if_dwge_fdt.c
> index edfe5acb992..a874274c215 100644
> --- a/sys/dev/fdt/if_dwge_fdt.c
> +++ b/sys/dev/fdt/if_dwge_fdt.c
> @@ -120,9 +120,7 @@ dwge_fdt_attach(struct device *parent, struct device
> *self, void *aux)
> /* Do hardware specific initializations. */
> if (OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-gmac"))
> dwge_fdt_attach_allwinner(fsc);
> - else if (OF_is_compatible(faa->fa_node, "rockchip,rk3288-gmac"))
> - dwge_fdt_attach_rockchip(fsc);
> - else if (OF_is_compatible(faa->fa_node, "rockchip,rk3399-gmac"))
> + else if (OF_is_compatible(faa->fa_node, "rockchip"))
> dwge_fdt_attach_rockchip(fsc);
>
> /* Enable clock. */
>
>
> > Regarding your mail from a few days ago. I did try something like
> > your diff to get the usb ports on the rk3288-tinker to work but failed
> > so far.
> >
>
> u-boot is still rather broken for rk3188, and success-rate on even u-boot
> finding the attached devices is less than 1/10... but one try got this far:
>
> U-Boot SPL 2017.09-rc2-00151-g2d7cb5b-dirty (Aug 21 2017 - 18:15:17)
> Returning to boot ROM...
>
>
> U-Boot 2017.09-rc2-00151-g2d7cb5b-dirty (Aug 21 2017 - 18:15:17 +0300)
>
> Model: Radxa Rock
> DRAM: 2 GiB
> MMC: dwmmc@10214000 - probe failed: -22
>
> *** Warning - No MMC card found, using default environment
>
> In:serial@20064000
> Out: serial@20064000
> Err: serial@20064000
> Model: Radxa Rock
> Net: Net Initialization Skipped
> No ethernet found.
> Hit any key to stop autoboot: 0
> starting USB...
> USB0: Core Release: 2.91a
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> USB1: Core Release: 2.91a
> scanning bus 0 for devices... 1 USB Device(s) found
> scanning bus 1 for devices... 5 USB Device(s) found
>scanning usb for storage devices... 1 Storage Device(s) found
>scanning usb for ethernet devices... 0 Ethernet Device(s) found
>
> Device 0: Vendor: Kingston Rev: PMAP Prod: DataTraveler 2.0
> Type: Removable Hard Disk
> Capacity: 14891.4 MB = 14.5 GB (30497664 x 512)
> ... is now current device
> Scanning usb 0:1...
> Found EFI removable media binary efi/boot/bootarm.efi
> reading efi/boot/bootarm.efi
> 64908 bytes read in 70 ms (905.3 KiB/s)
> libfdt fdt_check_header(): FDT_ERR_BADMAGIC
> ## Starting EFI application at 6200 ...
> Scanning disk dw...@10214000.blk...
> Scanning disk usb_mass_storage.lun0...
> Found 2 disks
> >> OpenBSD/armv7 BOOTARM 0.8
> boot>
> booting sd0a:/bsd: 3887248+166476+499544 [281845+90+515968+242685]=0x558c3c
>
> OpenBSD/armv7 booting ...
> arg0 0xc0858c3c arg1 0x0 arg2 0x6800
> Allocating page tables
> freestart = 0x60859000, free_pages = 522151 (0x0007f7a7)
> IRQ stack: p0x60887000 v0xc0887000
> ABT stack: p0x60888000 v0xc0888000
> UND st
Re: ospfd: add IMSG_IFADDRADD to deal with "sh /etc/netstart if"
On 08/21/17 18:57, Remi Locherer wrote: On Mon, Jul 24, 2017 at 04:59:46PM +0200, Remi Locherer wrote: On Fri, Jul 21, 2017 at 06:24:06PM +0200, Remi Locherer wrote: On Fri, Jul 21, 2017 at 02:45:03PM +0200, Florian Riehm wrote: On 06/25/17 23:47, Remi Locherer wrote: Hi, ospfd does not react nicely when running "sh /etc/netstart if". This is because adding the same address again do an interface results in RTM_DELADDR and RTM_NEWADDR. ospfd handles the former but the later. If this happens ospfd says "interface vether0:192.168.250.1 gone". Adjacencies on that interface are down and ospfd can not recover. The below patch adds IMSG_IFADDRADD to deal with that. With it ospfd logs the following after "ifconfig vether0 192.168.250.1/24" (same address as active before): Hi Remi, thanks for your report and your patch. I think it is the right approach, but unfortunately it doesn't work in my setup. If I run 'sh /etc/netstart vio1' vio1 goes down and stays down. Please have a look to my config / logs. What is the differece between your and my test? I tested with an interface connected to stub network. Your output shows an interface connected to a transit network. In my test setup I did not get the error message: "if_join_group: error IP_ADD_MEMBERSHIP" I'll look into it and try to fix this. I could reproduce the behaviour you see with my patch when testing with vmm and vio interfaces. It looks like in the IFADDRDEL case the interface can not leave the multicast group. I do not see this when testing with vether, pair or vlan (over ix) interfaces. Could this be a bug with multicast handling in vio? Today I did a test with an unpatched ospfd and a vio interface in vmm. I started ospfd and waited till adjacency was up. Then I did "ifconfig vio0 192.168.250.101/24" (same ip as set before). The debug output from ospfd: [...] if_leave_group: error IP_DROP_MEMBERSHIP, interface vio0 address 224.0.0.5: Can't assign requested address if_leave_group: error IP_DROP_MEMBERSHIP, interface vio0 address 224.0.0.6: Can't assign requested address orig_rtr_lsa: area 0.0.0.0 orig_rtr_lsa: stub net, interface vio0 orig_rtr_lsa: stub net, interface vether0 if_act_elect: interface vio0 old dr 192.168.250.1 new dr 192.168.250.101, old bdr 192.168.250.101 new bdr none orig_rtr_lsa: area 0.0.0.0 [...] Doing the same with a pair or vether interface does not produce the message "if_leave_group: error IP_DROP_MEMBERSHIP ". My conclusion of this is that the error is problem with the vio driver and not with my patch for ospfd. Could we proceed with the proposed ospfd patch and attack the vio problem separately? Actually I would be fine with that approach after I am sure it is a vio(4) problem. I think vether(4) and pair(4) are a bit too exotic to prove your patch works ;) Tomorrow I will test your change with em(4). My problem is not the multicast error message. I just can't 'see' your fix, since my interfaces stay down after netstart. Does your interface come up again, if the multicast error message occurs? friehm
fix an off by one in udl(4) bounds test
Fix an off by one in array bounds test, Coverity CID 1452930.
sys/dev/usb/usb.h contains:
/*
* Note: The length of the USB string descriptor is stored in a one byte
* value and can therefore be no longer than 255 bytes. Two bytes are
* used for the length itself and the descriptor type, a theoretical maximum
* of 253 bytes is left for the actual string data. Since the strings are
* encoded as 2-byte unicode characters, only 252 bytes or 126 two-byte
* characters can be used. USB_MAX_STRING_LEN is defined as 127, leaving
* space for the terminal '\0' character in C strings.
*/
struct usb_string_descriptor {
uByte bLength;
uByte bDescriptorType;
uWord bString[126];
} __packed;
typedef struct usb_string_descriptor usb_string_descriptor_t;
#define USB_MAX_STRING_LEN 127
Index: udl.c
===
RCS file: /cvs/src/sys/dev/usb/udl.c,v
retrieving revision 1.87
diff -u -p -r1.87 udl.c
--- udl.c 8 Apr 2017 02:57:25 - 1.87
+++ udl.c 23 Aug 2017 04:10:36 -
@@ -1312,7 +1312,7 @@ udl_select_chip(struct udl_softc *sc)
s = &serialnum[0];
n = len / 2 - 1;
- for (i = 0; i < n && i < USB_MAX_STRING_LEN; i++) {
+ for (i = 0; i < n && i < nitems(us.bString); i++) {
c = UGETW(us.bString[i]);
/* Convert from Unicode, handle buggy strings. */
if ((c & 0xff00) == 0)
