Re: athn(4) hostap: mem leak
On Wed, Dec 05, 2018 at 07:55:07PM +0100, Benjamin Baier wrote: > Finally got a usb athn device. I can confirm that this codepath is hit > in hostap mode and the device still works with the patch. > > athn0 at uhub4 port 2 configuration 1 interface 0 "ATHEROS USB2.0 WLAN" rev > 2.00/1.08 addr 3 > athn0: AR9271 rev 1 (1T1R), ROM rev 13, address c4:e9:84:dc:27:11 > > Full dmesg below. Committed, thanks!
sed(1) Add support for "-" file
According to POSIX[0]:
The standard input shall be used if no file operands are specified, and
shall be used if a file operand is '-' and the implementation treats the
'-' as meaning standard input. Otherwise, the standard input shall not
be used. See the INPUT FILES section.
Which we currently don't support:
$ cat /tmp/test
test
$ printf "foo\ntest\nbaz\n" | sed "s/test/bar/" - /tmp/test
sed: 0: -: No such file or directory
bar
So let's fix that while reducing the LoC.
I choose too accept stdin with the -i flag, because:
1) It allows us to combine it as an inplace editor and filter in a
single operation.
2) It further simplifies the code.
So now we can do fancy stuff like:
$ printf "foo\ntest\nbaz\n" | ./sed "s/test/bar/" /tmp/test -
bar
foo
bar
baz
and
$ printf "foo\ntest\nbaz\n" | ./sed -i "s/test/bar/" /tmp/test -
foo
bar
baz
$ cat /tmp/test
bar
Note that gsed does support the "-" filename, but treats - in
combination with -i as a filename, instead of stdin:
$ echo test > -
$ gsed -i s/test/bar/ -
$ cat ./-
bar
OK?
martijn@
Index: main.c
===
RCS file: /cvs/src/usr.bin/sed/main.c,v
retrieving revision 1.38
diff -u -p -r1.38 main.c
--- main.c 14 Nov 2018 10:59:33 - 1.38
+++ main.c 6 Dec 2018 07:28:15 -
@@ -345,32 +345,13 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
size_t len;
char *p;
int c, fd;
- static int firstfile;
-
- if (infile == NULL) {
- /* stdin? */
- if (files->fname == NULL) {
- if (inplace != NULL)
- error(FATAL, "-i may not be used with stdin");
- infile = stdin;
- fname = "stdin";
- outfile = stdout;
- outfname = "stdout";
- }
-
- firstfile = 1;
- }
+ static int firstfile = 1;
for (;;) {
if (infile != NULL && (c = getc(infile)) != EOF) {
(void)ungetc(c, infile);
break;
}
- /* If we are here then either eof or no files are open yet */
- if (infile == stdin) {
- sp->len = 0;
- return (0);
- }
finish_file();
if (firstfile == 0)
files = files->next;
@@ -381,6 +362,13 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
return (0);
}
fname = files->fname;
+ if (fname == NULL || strcmp(fname, "-") == 0) {
+ infile = stdin;
+ fname = "stdin";
+ outfile = stdout;
+ outfname = "stdout";
+ break;
+ }
if (inplace != NULL) {
if (lstat(fname, &sb) != 0)
error(FATAL, "%s: %s", fname,
Re: relayd and TLS client cert verification
It's been a week or so, so bumping. (Benno was kind enough to offer a review but was time-poor recently.) Here's a diff for the manpage too. Ashe Index: usr.sbin/relayd/relayd.conf.5 === RCS file: /home/kivikakk/cvsync/root/src/usr.sbin/relayd/relayd.conf.5,v retrieving revision 1.187 retrieving revision 1.187.6.1 diff -u -p -r1.187 -r1.187.6.1 --- usr.sbin/relayd/relayd.conf.5 6 Aug 2018 18:26:29 - 1.187 +++ usr.sbin/relayd/relayd.conf.5 30 Nov 2018 21:10:06 - 1.187.6.1 @@ -939,6 +939,10 @@ will be used (strong crypto cipher suite See the CIPHERS section of .Xr openssl 1 for information about SSL/TLS cipher suites and preference lists. +.It Ic client ca Ar path +Require TLS client certificates whose authenticity can be verified +against the CA certificate(s) in the specified file in order to +proceed beyond the TLS handshake. .It Ic client-renegotiation Allow client-initiated renegotiation. To mitigate a potential DoS risk,
find -not
Seen in the wild. Alias for ! that's friendlier to the shell.
Index: find.1
===
RCS file: /cvs/src/usr.bin/find/find.1,v
retrieving revision 1.95
diff -u -p -r1.95 find.1
--- find.1 1 Aug 2018 07:09:15 - 1.95
+++ find.1 6 Dec 2018 03:50:10 -
@@ -524,6 +524,7 @@ This evaluates to true if the parenthesi
true.
.Pp
.It Cm \&! Ar expression
+.It Cm -not Ar expression
This is the unary NOT operator.
It evaluates to true if the expression is false.
.Pp
@@ -624,9 +625,10 @@ primaries
and
.Ic -print0 ,
and operators
-.Fl or
-and
.Fl and ,
+.Fl not ,
+and
+.Fl or ,
are extensions to that specification.
.Pp
Historically, the
Index: option.c
===
RCS file: /cvs/src/usr.bin/find/option.c,v
retrieving revision 1.20
diff -u -p -r1.20 option.c
--- option.c3 Jan 2017 21:31:16 - 1.20
+++ option.c6 Dec 2018 03:44:12 -
@@ -80,6 +80,7 @@ static OPTION options[] = {
{ "-name", N_NAME, c_name, O_ARGV },
{ "-newer", N_NEWER,c_newer,O_ARGV },
{ "-nogroup", N_NOGROUP, c_nogroup, O_ZERO },
+ { "-not", N_NOT, c_not, O_ZERO },
{ "-nouser",N_NOUSER, c_nouser, O_ZERO },
{ "-o", N_OR, c_or, O_ZERO },
{ "-ok",N_OK, c_exec, O_ARGVP },
Re: be more strict when parsing netmasks for IPv6
On Wed, Dec 05, 2018 at 09:22:22AM +0100, Claudio Jeker wrote:
> When parsing a network mask into prefixlen be more paranoid and make sure
> no value bigger then 128 is returned. In general this should never happen
> but if it does the result can be bad.
>
> This is for bgpd but there are other users in the tree. I will adjust them
> if we dicide to go this way.
> --
> :wq Claudio
>
makes sense to me.
OK remi@
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.225
> diff -u -p -r1.225 kroute.c
> --- kroute.c 5 Nov 2018 07:01:15 - 1.225
> +++ kroute.c 19 Nov 2018 12:46:23 -
> @@ -2406,7 +2406,8 @@ mask2prefixlen(in_addr_t ina)
> u_int8_t
> mask2prefixlen6(struct sockaddr_in6 *sa_in6)
> {
> - u_int8_t l = 0, *ap, *ep;
> + u_int8_t*ap, *ep;
> + u_intl = 0;
>
> /*
>* sin6_len is the size of the sockaddr so substract the offset of
> @@ -2422,32 +2423,35 @@ mask2prefixlen6(struct sockaddr_in6 *sa_
> break;
> case 0xfe:
> l += 7;
> - return (l);
> + goto done;
> case 0xfc:
> l += 6;
> - return (l);
> + goto done;
> case 0xf8:
> l += 5;
> - return (l);
> + goto done;
> case 0xf0:
> l += 4;
> - return (l);
> + goto done;
> case 0xe0:
> l += 3;
> - return (l);
> + goto done;
> case 0xc0:
> l += 2;
> - return (l);
> + goto done;
> case 0x80:
> l += 1;
> - return (l);
> + goto done;
> case 0x00:
> - return (l);
> + goto done;
> default:
> fatalx("non contiguous inet6 netmask");
> }
> }
>
> + done:
> + if (l > sizeof(struct in6_addr) * 8)
> + fatalx("%s: prefixlen %d out of bound", __func__, l);
> return (l);
> }
>
>
Re: option kcov + GENERIC.MP -> silent crash
On Sat, Dec 01, 2018 at 04:34:57PM +0100, Anton Lindqvist wrote:
> On Tue, Nov 27, 2018 at 05:52:15PM -0800, Greg Steuck wrote:
> > I booted the patched kernel and it seems to have gone farther and I believe
> > reached init before crashing.
>
> By performing a semi-automated bisect I was able to identify the source
> files that are incompatible with tracing. Common for all source files
> seems to be that they define routines called early on in the boot
> process before curcpu() is usable.
>
> I do not have any plans on committing the diff below but please give it
> a try. Instead, I'm working on extending the files.conf(5) grammar in
> order to infer a different make target for the files.
>
> Index: arch/amd64/conf/Makefile.amd64
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/Makefile.amd64,v
> retrieving revision 1.106
> diff -u -p -r1.106 Makefile.amd64
> --- arch/amd64/conf/Makefile.amd6430 Oct 2018 11:08:30 - 1.106
> +++ arch/amd64/conf/Makefile.amd641 Dec 2018 15:32:58 -
> @@ -151,7 +151,31 @@ vers.o: ${SYSTEM_DEP:Ngap.o}
> ${CC} ${CFLAGS} ${CPPFLAGS} ${PROF} -c vers.c
>
> .if ${SYSTEM_OBJ:Mkcov.o} && ${COMPILER_VERSION:Mclang}
> +amd64_mem.o: $S/arch/amd64/amd64/amd64_mem.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +cpu.o: $S/arch/amd64/amd64/cpu.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +fpu.o: $S/arch/amd64/amd64/fpu.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +gdt.o: $S/arch/amd64/amd64/gdt.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +intr.o: $S/arch/amd64/amd64/intr.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +lapic.o: $S/arch/amd64/amd64/lapic.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +machdep.o: $S/arch/amd64/amd64/machdep.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +tsc.o: $S/arch/amd64/amd64/tsc.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> kcov.o: $S/dev/kcov.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +pvbus.o: $S/dev/pv/pvbus.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +kern_lock.o: $S/kern/kern_lock.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +kern_sched.o: $S/kern/kern_sched.c
> + ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> +kern_tc.o: $S/kern/kern_tc.c
> ${NORMAL_C} -fno-sanitize-coverage=trace-pc
> .endif
>
Here's a new diff taking a different approach. Keeping tracing off until
all secondary CPUs have booted solves the issue of accessing curcpu()
too early. Another issue was then discovered, curproc can be NULL before
the idle thread tied the current CPU has started. Currently running with
this diff applied on my laptop (MP) and positive results from Greg. The
diff will be further exercised in the actual syzkaller setup before
committing.
Comments? OK?
diff --git sys/dev/kcov.c sys/dev/kcov.c
index 8e36bc8b8ef..c97aae4ed5d 100644
--- sys/dev/kcov.c
+++ sys/dev/kcov.c
@@ -58,6 +58,8 @@ static inline int inintr(void);
TAILQ_HEAD(, kcov_dev) kd_list = TAILQ_HEAD_INITIALIZER(kd_list);
+int kcov_cold = 1;
+
#ifdef KCOV_DEBUG
int kcov_debug = 1;
#endif
@@ -76,19 +78,31 @@ int kcov_debug = 1;
void
__sanitizer_cov_trace_pc(void)
{
- extern int cold;
struct kcov_dev *kd;
+ struct proc *p;
uint64_t idx;
- /* Do not trace during boot. */
- if (cold)
+ /*
+* Do not trace before all secondary CPUs have booted.
+* Accessing the current CPU during boot causes a subtle crash since its
+* GSBASE register has not yet been written.
+*/
+ if (kcov_cold)
return;
/* Do not trace in interrupts to prevent noisy coverage. */
if (inintr())
return;
- kd = curproc->p_kd;
+ /*
+* Protect against when the idle thread for the current CPU has not yet
+* started and curproc is absent.
+*/
+ p = curproc;
+ if (p == NULL)
+ return;
+
+ kd = p->p_kd;
if (kd == NULL || kd->kd_mode != KCOV_MODE_TRACE_PC)
return;
@@ -226,6 +240,12 @@ kcov_exit(struct proc *p)
p->p_kd = NULL;
}
+void
+kcov_init(void)
+{
+ kcov_cold = 0;
+}
+
struct kcov_dev *
kd_lookup(int unit)
{
diff --git sys/kern/init_main.c sys/kern/init_main.c
index 91070090bb1..25b71fd89ce 100644
--- sys/kern/init_main.c
+++ sys/kern/init_main.c
@@ -103,6 +103,11 @@ extern void nfs_init(void);
#include "vscsi.h"
#include "softraid.h"
+#include "kcov.h"
+#if NKCOV > 0
+#include
+#endif
+
const char copyright[] =
"Copyright (c) 1982, 1986, 1989, 1991, 1993\n"
"\tThe Regents of the University of California. All rights reserved.\n"
@@ -555,6 +560,10 @@ main(void *framep)
config_process_deferred_mountroot();
+#if NKCOV > 0
+ kcov_init();
+#endif
+
/*
* Okay, now we can let init(8) exec! It's off to userland!
*/
bridge cleanup
Stop passing `sc' when it is not necessary. Also prefer passing the
brigde's ifp rather than `sc' when it's enough. This makes auditing
easier. Different structures need different locking.
As a bonus is removes an horrible cast in net/if_switch.c.
ok?
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.313
diff -u -p -r1.313 if_bridge.c
--- net/if_bridge.c 14 Nov 2018 17:07:44 - 1.313
+++ net/if_bridge.c 5 Dec 2018 19:27:18 -
@@ -113,8 +113,8 @@ voidbridge_process(struct ifnet *, stru
void bridgeintr_frame(struct bridge_softc *, struct ifnet *, struct mbuf *);
void bridge_broadcast(struct bridge_softc *, struct ifnet *,
struct ether_header *, struct mbuf *);
-void bridge_localbroadcast(struct bridge_softc *, struct ifnet *,
-struct ether_header *, struct mbuf *);
+intbridge_localbroadcast(struct ifnet *, struct ether_header *,
+struct mbuf *);
void bridge_span(struct bridge_softc *, struct mbuf *);
void bridge_stop(struct bridge_softc *);
void bridge_init(struct bridge_softc *);
@@ -123,13 +123,17 @@ int bridge_blocknonip(struct ether_heade
void bridge_ifinput(struct ifnet *, struct mbuf *);
intbridge_dummy_output(struct ifnet *, struct mbuf *, struct sockaddr *,
struct rtentry *);
+void bridge_send_icmp_err(struct ifnet *, struct ether_header *,
+struct mbuf *, int, struct llc *, int, int, int);
+intbridge_ifenqueue(struct ifnet *, struct ifnet *, struct mbuf *);
+struct mbuf *bridge_ip(struct ifnet *, int, struct ifnet *,
+struct ether_header *, struct mbuf *);
#ifdef IPSEC
-int bridge_ipsec(struct bridge_softc *, struct ifnet *,
-struct ether_header *, int, struct llc *,
+int bridge_ipsec(struct ifnet *, struct ether_header *, int, struct llc *,
int, int, int, struct mbuf *);
#endif
int bridge_clone_create(struct if_clone *, int);
-intbridge_clone_destroy(struct ifnet *ifp);
+intbridge_clone_destroy(struct ifnet *);
intbridge_delete(struct bridge_softc *, struct bridge_iflist *);
#defineETHERADDR_IS_IP_MCAST(a) \
@@ -813,7 +817,7 @@ bridge_output(struct ifnet *ifp, struct
BRL_ACTION_BLOCK)
continue;
- error = bridge_ifenqueue(sc, dst_if, mc);
+ error = bridge_ifenqueue(&sc->sc_if, dst_if, mc);
if (error)
continue;
}
@@ -833,7 +837,7 @@ sendunicast:
m_freem(m);
return (ENETDOWN);
}
- bridge_ifenqueue(sc, dst_if, m);
+ bridge_ifenqueue(&sc->sc_if, dst_if, m);
return (0);
}
@@ -979,7 +983,7 @@ bridgeintr_frame(struct bridge_softc *sc
m_freem(m);
return;
}
- m = bridge_ip(sc, BRIDGE_IN, src_if, &eh, m);
+ m = bridge_ip(&sc->sc_if, BRIDGE_IN, src_if, &eh, m);
if (m == NULL)
return;
/*
@@ -1019,7 +1023,7 @@ bridgeintr_frame(struct bridge_softc *sc
m_freem(m);
return;
}
- m = bridge_ip(sc, BRIDGE_OUT, dst_if, &eh, m);
+ m = bridge_ip(&sc->sc_if, BRIDGE_OUT, dst_if, &eh, m);
if (m == NULL)
return;
@@ -1030,9 +1034,9 @@ bridgeintr_frame(struct bridge_softc *sc
len += ETHER_VLAN_ENCAP_LEN;
#endif
if ((len - ETHER_HDR_LEN) > dst_if->if_mtu)
- bridge_fragment(sc, dst_if, &eh, m);
+ bridge_fragment(&sc->sc_if, dst_if, &eh, m);
else {
- bridge_ifenqueue(sc, dst_if, m);
+ bridge_ifenqueue(&sc->sc_if, dst_if, m);
}
}
@@ -1252,7 +1256,8 @@ bridge_broadcast(struct bridge_softc *sc
if (dst_if->if_index == ifp->if_index)
continue;
- bridge_localbroadcast(sc, dst_if, eh, m);
+ if (bridge_localbroadcast(dst_if, eh, m))
+ sc->sc_if.if_oerrors++;
#if NMPW > 0
/*
@@ -1276,7 +1281,7 @@ bridge_broadcast(struct bridge_softc *sc
}
}
- mc = bridge_ip(sc, BRIDGE_OUT, dst_if, eh, mc);
+ mc = bridge_ip(&sc->sc_if, BRIDGE_OUT, dst_if, eh, mc);
if (mc == NULL)
continue;
@@ -1287,9 +1292,9 @@ bridge_broadcast(struct bridge_softc *sc
len += ETHER_VLAN_ENCAP_LEN;
#endif
if ((len - ETHER_HDR_LEN) > dst_if->if_mtu)
- bridge_fragment(sc, dst_if, eh, mc);
+ bridge_fragment(&sc->sc_if, dst_if, eh, mc);
else {
- bridge_ifenqueue(sc, dst_if, mc);
+ bridge_ifenqueue(&sc->sc_if, dst_if, mc);
}
}
@@ -1297,9 +1302,9 @@ bridge_broadcast(s
Re: athn(4) hostap: mem leak
Finally got a usb athn device. I can confirm that this codepath is hit in hostap mode and the device still works with the patch. athn0 at uhub4 port 2 configuration 1 interface 0 "ATHEROS USB2.0 WLAN" rev 2.00/1.08 addr 3 athn0: AR9271 rev 1 (1T1R), ROM rev 13, address c4:e9:84:dc:27:11 Full dmesg below. On Sun, 2 Dec 2018 10:15:44 +0100 Benjamin Baier wrote: > On Sat, 1 Dec 2018 15:48:13 -0200 > Martin Pieuchot wrote: > > > On 30/11/18(Fri) 13:49, Benjamin Baier wrote: > > > Hi > > > > > > There is a leak of *arg in > > > dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263 > > > since Rev. 1.49 > > > Because athn_usb_do_async() memcpy's the argument anyway. > > > > > > Found with llvm/scan-build. > > > > > > Instead of adding free(arg) I opted to make this function > > > more like the other ones which call athn_usb_do_async. > > > > > > Only compile tested... looking for tests. > > > > You should also remove the free(arg...) in athn_usb_newauth_cb(). > Indeed, new patch attached. Index: if_athn_usb.c === RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v retrieving revision 1.51 diff -u -p -r1.51 if_athn_usb.c --- if_athn_usb.c 6 Sep 2018 11:50:54 - 1.51 +++ if_athn_usb.c 2 Dec 2018 09:09:29 - @@ -1202,8 +1202,6 @@ athn_usb_newauth_cb(struct athn_usb_soft struct athn_node *an = (struct athn_node *)ni; int s, error = 0; - free(arg, M_DEVBUF, sizeof(*arg)); - if (ic->ic_state != IEEE80211_S_RUN) return; @@ -1231,7 +1229,7 @@ athn_usb_newauth(struct ieee80211com *ic struct ifnet *ifp = &ic->ic_if; struct athn_node *an = (struct athn_node *)ni; int nsta; - struct athn_usb_newauth_cb_arg *arg; + struct athn_usb_newauth_cb_arg arg; if (ic->ic_opmode != IEEE80211_M_HOSTAP) return 0; @@ -1254,12 +1252,9 @@ athn_usb_newauth(struct ieee80211com *ic * In a process context, try to add this node to the * firmware table and confirm the AUTH request. */ - arg = malloc(sizeof(*arg), M_DEVBUF, M_NOWAIT); - if (arg == NULL) - return ENOMEM; - arg->ni = ieee80211_ref_node(ni); - arg->seq = seq; - athn_usb_do_async(usc, athn_usb_newauth_cb, arg, sizeof(*arg)); + arg.ni = ieee80211_ref_node(ni); + arg.seq = seq; + athn_usb_do_async(usc, athn_usb_newauth_cb, &arg, sizeof(arg)); return EBUSY; #else return 0; OpenBSD 6.4-current (GENERIC.MP) #492: Mon Dec 3 21:37:10 MST 2018 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 8451125248 (8059MB) avail mem = 8185712640 (7806MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xdae9c000 (64 entries) bios0: vendor LENOVO version "8DET69WW (1.39 )" date 07/18/2013 bios0: LENOVO 4287CTO acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SLIC SSDT SSDT SSDT HPET APIC MCFG ECDT ASF! TCPA SSDT SSDT DMAR UEFI UEFI UEFI acpi0: wakeup devices LID_(S3) SLPB(S3) IGBE(S4) EXP4(S4) EXP7(S4) EHC1(S3) EHC2(S3) HDEF(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2492.26 MHz, 06-2a-07 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,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,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 99MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz, 06-2a-07 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,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 1, core 0, package 0 cpu2 at mainbus0: apid 2 (application processor) cpu2: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz, 06-2a-07 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,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cp
Useless splnet()
bstp_create() is called on by bridge/switch_clone_create(). Nothing it
touches is reachable, so there's no need for splnet().
ok?
Index: net/bridgestp.c
===
RCS file: /cvs/src/sys/net/bridgestp.c,v
retrieving revision 1.66
diff -u -p -r1.66 bridgestp.c
--- net/bridgestp.c 22 Oct 2018 13:18:23 - 1.66
+++ net/bridgestp.c 5 Dec 2018 17:53:51 -
@@ -1897,9 +1897,7 @@ struct bstp_state *
bstp_create(struct ifnet *ifp)
{
struct bstp_state *bs;
- int s;
- s = splnet();
bs = malloc(sizeof(*bs), M_DEVBUF, M_WAITOK|M_ZERO);
LIST_INIT(&bs->bs_bplist);
@@ -1914,8 +1912,6 @@ bstp_create(struct ifnet *ifp)
bs->bs_protover = BSTP_PROTO_RSTP; /* STP instead of RSTP? */
getmicrotime(&bs->bs_last_tc_time);
-
- splx(s);
return (bs);
}
Re: malloc.3: remove left-over from malloc_usable_size commit
Hi Hiltjo, Hiltjo Posthuma wrote on Wed, Dec 05, 2018 at 05:46:53PM +0100: > In the malloc_usable_size() revert commit it was forgotten > to remove one line. > > The below patch fixes this: Committed, thanks. Ingo > diff --git a/lib/libc/stdlib/malloc.3 b/lib/libc/stdlib/malloc.3 > index 49a5b993f44..cc71560739a 100644 > --- a/lib/libc/stdlib/malloc.3 > +++ b/lib/libc/stdlib/malloc.3 > @@ -63,7 +63,6 @@ > .Fn freezero "void *ptr" "size_t size" > .Ft void * > .Fn aligned_alloc "size_t alignment" "size_t size" > -.Ft size_t > .Vt char *malloc_options ; > .Sh DESCRIPTION > The standard functions
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
Le 2018-12-05 13:03, Stuart Henderson a écrit : On 2018/12/05 12:40, Arnaud BRAND wrote: Or is there something else I missed ? > > + if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr); > > + if (src == NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr; if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr); if (src == NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr; Of course, that's quite me. The more obvious, the less I spot it. I could have spent hours reading style(9) and writing code to compare the size of structs. Thanks a lot Stuart ! Here is an updated diff : Index: sys/netinet6/icmp6.c === RCS file: /cvs/src/sys/netinet6/icmp6.c,v retrieving revision 1.227 diff -u -p -u -p -r1.227 icmp6.c --- sys/netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227 +++ sys/netinet6/icmp6.c5 Dec 2018 13:31:02 - @@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off struct icmp6_hdr *icmp6; struct in6_addr t, *src = NULL; struct sockaddr_in6 sa6_src, sa6_dst; + struct in6_ifaddr *ifa; u_int rtableid; CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <= MHLEN); @@ -1141,7 +1142,11 @@ icmp6_reflect(struct mbuf *m, size_t off rtfree(rt); goto bad; } - src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr; + ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid); + if (ifa != NULL) + src = &(ifa->ia_addr.sin6_addr); + if (src == NULL) + src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr; } ip6->ip6_src = *src;
malloc.3: remove left-over from malloc_usable_size commit
Hi, In the malloc_usable_size() revert commit it was forgotten to remove one line. The below patch fixes this: diff --git a/lib/libc/stdlib/malloc.3 b/lib/libc/stdlib/malloc.3 index 49a5b993f44..cc71560739a 100644 --- a/lib/libc/stdlib/malloc.3 +++ b/lib/libc/stdlib/malloc.3 @@ -63,7 +63,6 @@ .Fn freezero "void *ptr" "size_t size" .Ft void * .Fn aligned_alloc "size_t alignment" "size_t size" -.Ft size_t .Vt char *malloc_options ; .Sh DESCRIPTION The standard functions -- Kind regards, Hiltjo
netcred free(9) sizes
Keep track of the allocated size and use it later :)
ok?
Index: kern/vfs_subr.c
===
RCS file: /cvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.282
diff -u -p -r1.282 vfs_subr.c
--- kern/vfs_subr.c 29 Sep 2018 04:29:48 - 1.282
+++ kern/vfs_subr.c 18 Nov 2018 16:05:25 -
@@ -1424,6 +1424,7 @@ vfs_hang_addrlist(struct mount *mp, stru
return (EINVAL);
nplen = sizeof(struct netcred) + argp->ex_addrlen + argp->ex_masklen;
np = (struct netcred *)malloc(nplen, M_NETADDR, M_WAITOK|M_ZERO);
+ np->netc_len = nplen;
saddr = (struct sockaddr *)(np + 1);
error = copyin(argp->ex_addr, saddr, argp->ex_addrlen);
if (error)
@@ -1466,7 +1467,7 @@ finish:
np->netc_exflags = argp->ex_flags;
return (0);
out:
- free(np, M_NETADDR, nplen);
+ free(np, M_NETADDR, np->netc_len);
return (error);
}
@@ -1474,9 +1475,10 @@ int
vfs_free_netcred(struct radix_node *rn, void *w, u_int id)
{
struct radix_node_head *rnh = (struct radix_node_head *)w;
+ struct netcred * np = (struct netcred *)rn;
rn_delete(rn->rn_key, rn->rn_mask, rnh, NULL);
- free(rn, M_NETADDR, 0);
+ free(np, M_NETADDR, np->netc_len);
return (0);
}
@@ -1490,7 +1492,7 @@ vfs_free_addrlist(struct netexport *nep)
if ((rnh = nep->ne_rtable_inet) != NULL) {
rn_walktree(rnh, vfs_free_netcred, rnh);
- free(rnh, M_RTABLE, 0);
+ free(rnh, M_RTABLE, sizeof(*rnh));
nep->ne_rtable_inet = NULL;
}
}
Index: sys/mount.h
===
RCS file: /cvs/src/sys/sys/mount.h,v
retrieving revision 1.142
diff -u -p -r1.142 mount.h
--- sys/mount.h 29 Sep 2018 04:29:48 - 1.142
+++ sys/mount.h 18 Nov 2018 16:00:22 -
@@ -552,6 +552,7 @@ struct vfsops {
struct netcred {
struct radix_node netc_rnodes[2];
int netc_exflags;
+ int netc_len; /* size of the allocation */
struct ucred netc_anon;
};
Re: cvsintro.7: patch to fix .Bx invocation
Hi, Fabio Scotoni wrote on Wed, Dec 05, 2018 at 02:23:58PM +0100: > The cvsintro(7) man page uses ".Bx -licensed", > which is rendered as "-licensedBSD". You are right, that is incorrect usage. > This patch fixes this to correctly read "BSD-licensed" > in both mandoc and groff. > I'm not entirely sure if .Bx should even be used here, You are right again. The .Bx macro refers to releases made by the Berkeley Computer Systems Research Group, not to a licensing style. > though apparently that decision has been made deliberately. Not all authors are perfectly familiar with the mdoc(7) language, so glitches happen, just like in code - only that in the docs, consequences are on average less dire. :) See below for what i committed. I guess i should make more use of dbm_dump, it is quite handy. Thanks, Ingo CVSROOT:/cvs Module name:src Changes by: schwa...@cvs.openbsd.org2018/12/05 08:34:52 Modified files: share/man/man4 : multicast.4 share/man/man9 : style.9 usr.bin/cvs: cvsintro.7 Log message: fix incorrect usage of the .Bx macro; one case reported by Fabio Scotoni , the rest found with regress/usr.bin/mandoc/db/dbm_dump Index: share/man/man4/multicast.4 === RCS file: /cvs/src/share/man/man4/multicast.4,v retrieving revision 1.13 diff -u -r1.13 multicast.4 --- share/man/man4/multicast.4 20 Oct 2018 20:09:18 - 1.13 +++ share/man/man4/multicast.4 5 Dec 2018 15:30:39 - @@ -122,7 +122,7 @@ sockets should be used for sending and receiving respectively IGMP or MLD messages. In the case of -.Bx -derived +.Bx Ns -derived kernels, it may be possible to open separate sockets for IGMP or MLD messages only. Index: share/man/man9/style.9 === RCS file: /cvs/src/share/man/man9/style.9,v retrieving revision 1.72 diff -u -r1.72 style.9 --- share/man/man9/style.9 5 Oct 2018 12:11:21 - 1.72 +++ share/man/man9/style.9 5 Dec 2018 15:30:39 - @@ -615,7 +615,7 @@ .Xr mdoc 7 .Sh HISTORY This man page is largely based on the src/admin/style/style file from the -.Bx 4.4-Lite2 +.Bx 4.4 Lite2 release, with updates to reflect the current practice and desire of the .Ox Index: usr.bin/cvs/cvsintro.7 === RCS file: /cvs/src/usr.bin/cvs/cvsintro.7,v retrieving revision 1.15 diff -u -r1.15 cvsintro.7 --- usr.bin/cvs/cvsintro.7 14 Aug 2013 08:46:07 - 1.15 +++ usr.bin/cvs/cvsintro.7 5 Dec 2018 15:30:39 - @@ -222,9 +222,7 @@ .Xr cvs 5 , .Xr sshd 8 .Sh HISTORY -The OpenCVS project is a -.Bx -licensed -rewrite of the original +The OpenCVS project is a BSD-licensed rewrite of the original Concurrent Versioning System written by Jean-Francois Brousseau. The original CVS code was written in large parts by Dick Grune, Brian Berliner, and Jeff Polk.
cvsintro.7: patch to fix .Bx invocation
The cvsintro(7) man page uses ".Bx -licensed", which is rendered as "-licensedBSD". This patch fixes this to correctly read "BSD-licensed" in both mandoc and groff. I'm not entirely sure if .Bx should even be used here, though apparently that decision has been made deliberately. Index: cvsintro.7 === RCS file: /cvs/src/usr.bin/cvs/cvsintro.7,v retrieving revision 1.15 diff -u -p -r1.15 cvsintro.7 --- cvsintro.7 14 Aug 2013 08:46:07 - 1.15 +++ cvsintro.7 29 Nov 2018 19:18:34 - @@ -223,7 +223,7 @@ unless the development network is local. .Xr sshd 8 .Sh HISTORY The OpenCVS project is a -.Bx -licensed +.Bx Ns -licensed rewrite of the original Concurrent Versioning System written by Jean-Francois Brousseau. The original CVS code was written in large parts by Dick Grune,
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
On 2018/12/05 12:40, Arnaud BRAND wrote: > Or is there something else I missed ? > > > + if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr); > > > + if (src == NULL) src = > > > &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr; if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr); if (src == NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
Le 2018-12-05 12:02, Denis Fondras a écrit :
On Wed, Dec 05, 2018 at 11:36:14AM +0100, Arnaud BRAND wrote:
Any feedback on this patch ?
I'm running it without problems since the 30th November.
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.227
diff -u -p -u -p -r1.227 icmp6.c
--- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
+++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
@@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
struct icmp6_hdr *icmp6;
struct in6_addr t, *src = NULL;
struct sockaddr_in6 sa6_src, sa6_dst;
+ struct in6_ifaddr *ifa;
u_int rtableid;
CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <=
MHLEN);
@@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
rtfree(rt);
goto bad;
}
- src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
+ ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t,
rtableid);
+ if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
+ if (src == NULL) src =
&ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
}
ip6->ip6_src = *src;
style(9) :)
OK, I thought I had this read this one carefully enough but it seems not
to be the case.
Are you referring to the new variable position ?
Rereading the examples it looks like I had not understood this rule
properly
(When declaring variables in functions, declare them sorted by size
(largest to smallest), then in alphabetical order;)
Or is there something else I missed ?
Le 2018-11-29 00:50, Arnaud BRAND a écrit :
> I have setup a test environment where I can reproduce the issue with
> GENERIC#481.
> I can confirm that the issue is fixed by the proposed patch.
> traceroute6 to/from/through the patched machine got expected result
> and did not crash the machine.
>
> Anyone else would like to try ?
> Or propose improvements ?
>
> Le 2018-11-29 00:02, Arnaud BRAND a écrit :
> > Le 2018-11-28 22:46, Arnaud BRAND a écrit :
> > > Le 2018-10-05 23:42, Stuart Henderson a écrit :
> > > > On 2018/10/05 18:38, Alexander Bluhm wrote:
> > > > > IPv6 Source selection is a mess!
> > > > >
> > > > > > ICMP6 messages
> > > > > > are generated with a source of, I think, the local address
associated with
> > > > > > the route to the recipient,
> > > > >
> > > > > It is not that simple. Look at in6_ifawithscope() in
> > > > > sys/netinet6/in6.c.
> > > >
> > > > I know that's used for newly generated packets, but I'm not
> > > > sure it's the
> > > > case for icmp6, I haven't tried modifying the kernel to
> > > > confirm for sure
> > > > that this is the code generating the address in this case,
> > > > but it seems
> > > > likely, in icmp6.c:
> > > >
> > > > /*
> > > > 1112 * If the incoming packet was addressed directly to us
> > > > (i.e. unicast),
> > > > 1113 * use dst as the src for the reply.
> > > > 1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED
> > > > case would be
> > > > VERY rare,
> > > > 1115 * but is possible (for example) when we
> > > > encounter an error while
> > > > 1116 * forwarding procedure destined to a
> > > > duplicated address of ours.
> > > > 1117 */
> > > > 1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
> > > > 1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
> > > > 1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
> > > > 1121
> > > > IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
> > > > 1122 src = &t;
> > > > 1123 }
> > > >
> > >
> > > I don't think this is the proper code extract because the
> > > traceroute6 packet
> > > is not addressed directly to us.
> > > It's addressed to another global unicast address and appears to
> > > time out
> > > because the TTL gets decremented.
> > >
> > > So I think the code that gets executed in this case is :
> > > 1127 if (src == NULL) {
> > > 1128 /*
> > > 1129 * This case matches to multicasts, our anycast,
> > > or unicasts
> > > 1130 * that we do not own. Select a source address
> > > based on the
> > > 1131 * source address of the erroneous packet.
> > > 1132 */
> > > 1133 rt = rtalloc(sin6tosa(&sa6_src),
> > > RT_RESOLVE, rtableid);
> > > 1134 if (!rtisvalid(rt)) {
> > > 1135 char addr[INET6_ADDRSTRLEN];
> > > 1136
> > > 1137 nd6log((LOG_DEBUG,
> > > 1138 "%s: source can't be
> > > determined: dst=%s\n",
> > > 1139 __func__, inet_ntop(AF_INET6,
> > > &sa6_src.sin6_addr,
> > > 1140 addr, sizeof(addr;
> > > 1141 rtfree(rt);
> >
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
On Wed, Dec 05, 2018 at 12:02:25PM +0100, Denis Fondras wrote:
> On Wed, Dec 05, 2018 at 11:36:14AM +0100, Arnaud BRAND wrote:
> > Any feedback on this patch ?
> > I'm running it without problems since the 30th November.
> >
> >
> > Index: netinet6/icmp6.c
> > ===
> > RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> > retrieving revision 1.227
> > diff -u -p -u -p -r1.227 icmp6.c
> > --- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
> > +++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
> > @@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
> > struct icmp6_hdr *icmp6;
> > struct in6_addr t, *src = NULL;
> > struct sockaddr_in6 sa6_src, sa6_dst;
> > + struct in6_ifaddr *ifa;
> > u_int rtableid;
> >
> > CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <=
> > MHLEN);
> > @@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
> > rtfree(rt);
> > goto bad;
> > }
> > - src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> > + ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
> > + if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
> > + if (src == NULL) src =
> > &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> > }
> >
> > ip6->ip6_src = *src;
> >
>
> style(9) :)
But apart from that I think this is OK claudio@.
> >
> >
> > Le 2018-11-29 00:50, Arnaud BRAND a écrit :
> > > I have setup a test environment where I can reproduce the issue with
> > > GENERIC#481.
> > > I can confirm that the issue is fixed by the proposed patch.
> > > traceroute6 to/from/through the patched machine got expected result
> > > and did not crash the machine.
> > >
> > > Anyone else would like to try ?
> > > Or propose improvements ?
> > >
> > > Le 2018-11-29 00:02, Arnaud BRAND a écrit :
> > > > Le 2018-11-28 22:46, Arnaud BRAND a écrit :
> > > > > Le 2018-10-05 23:42, Stuart Henderson a écrit :
> > > > > > On 2018/10/05 18:38, Alexander Bluhm wrote:
> > > > > > > IPv6 Source selection is a mess!
> > > > > > >
> > > > > > > > ICMP6 messages
> > > > > > > > are generated with a source of, I think, the local address
> > > > > > > > associated with
> > > > > > > > the route to the recipient,
> > > > > > >
> > > > > > > It is not that simple. Look at in6_ifawithscope() in
> > > > > > > sys/netinet6/in6.c.
> > > > > >
> > > > > > I know that's used for newly generated packets, but I'm not
> > > > > > sure it's the
> > > > > > case for icmp6, I haven't tried modifying the kernel to
> > > > > > confirm for sure
> > > > > > that this is the code generating the address in this case,
> > > > > > but it seems
> > > > > > likely, in icmp6.c:
> > > > > >
> > > > > > /*
> > > > > > 1112 * If the incoming packet was addressed directly to us
> > > > > > (i.e. unicast),
> > > > > > 1113 * use dst as the src for the reply.
> > > > > > 1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED
> > > > > > case would be
> > > > > > VERY rare,
> > > > > > 1115 * but is possible (for example) when we
> > > > > > encounter an error while
> > > > > > 1116 * forwarding procedure destined to a
> > > > > > duplicated address of ours.
> > > > > > 1117 */
> > > > > > 1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
> > > > > > 1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
> > > > > > 1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
> > > > > > 1121
> > > > > > IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
> > > > > > 1122 src = &t;
> > > > > > 1123 }
> > > > > >
> > > > >
> > > > > I don't think this is the proper code extract because the
> > > > > traceroute6 packet
> > > > > is not addressed directly to us.
> > > > > It's addressed to another global unicast address and appears to
> > > > > time out
> > > > > because the TTL gets decremented.
> > > > >
> > > > > So I think the code that gets executed in this case is :
> > > > > 1127 if (src == NULL) {
> > > > > 1128 /*
> > > > > 1129 * This case matches to multicasts, our anycast,
> > > > > or unicasts
> > > > > 1130 * that we do not own. Select a source address
> > > > > based on the
> > > > > 1131 * source address of the erroneous packet.
> > > > > 1132 */
> > > > > 1133 rt = rtalloc(sin6tosa(&sa6_src),
> > > > > RT_RESOLVE, rtableid);
> > > > > 1134 if (!rtisvalid(rt)) {
> > > > > 1135 char addr[INET6_ADDRSTRLEN];
> > > > > 1136
> > > > > 1137 nd6log((LOG_DEBUG,
> > > > > 1138 "%s: source can't be
> > > > > determined: dst=%s\n",
> > > > > 1139 __func__, inet
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
On Wed, Dec 05, 2018 at 11:36:14AM +0100, Arnaud BRAND wrote:
> Any feedback on this patch ?
> I'm running it without problems since the 30th November.
>
>
> Index: netinet6/icmp6.c
> ===
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.227
> diff -u -p -u -p -r1.227 icmp6.c
> --- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
> +++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
> @@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
> struct icmp6_hdr *icmp6;
> struct in6_addr t, *src = NULL;
> struct sockaddr_in6 sa6_src, sa6_dst;
> + struct in6_ifaddr *ifa;
> u_int rtableid;
>
> CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <=
> MHLEN);
> @@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
> rtfree(rt);
> goto bad;
> }
> - src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> + ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
> + if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
> + if (src == NULL) src =
> &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> }
>
> ip6->ip6_src = *src;
>
style(9) :)
>
>
> Le 2018-11-29 00:50, Arnaud BRAND a écrit :
> > I have setup a test environment where I can reproduce the issue with
> > GENERIC#481.
> > I can confirm that the issue is fixed by the proposed patch.
> > traceroute6 to/from/through the patched machine got expected result
> > and did not crash the machine.
> >
> > Anyone else would like to try ?
> > Or propose improvements ?
> >
> > Le 2018-11-29 00:02, Arnaud BRAND a écrit :
> > > Le 2018-11-28 22:46, Arnaud BRAND a écrit :
> > > > Le 2018-10-05 23:42, Stuart Henderson a écrit :
> > > > > On 2018/10/05 18:38, Alexander Bluhm wrote:
> > > > > > IPv6 Source selection is a mess!
> > > > > >
> > > > > > > ICMP6 messages
> > > > > > > are generated with a source of, I think, the local address
> > > > > > > associated with
> > > > > > > the route to the recipient,
> > > > > >
> > > > > > It is not that simple. Look at in6_ifawithscope() in
> > > > > > sys/netinet6/in6.c.
> > > > >
> > > > > I know that's used for newly generated packets, but I'm not
> > > > > sure it's the
> > > > > case for icmp6, I haven't tried modifying the kernel to
> > > > > confirm for sure
> > > > > that this is the code generating the address in this case,
> > > > > but it seems
> > > > > likely, in icmp6.c:
> > > > >
> > > > > /*
> > > > > 1112 * If the incoming packet was addressed directly to us
> > > > > (i.e. unicast),
> > > > > 1113 * use dst as the src for the reply.
> > > > > 1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED
> > > > > case would be
> > > > > VERY rare,
> > > > > 1115 * but is possible (for example) when we
> > > > > encounter an error while
> > > > > 1116 * forwarding procedure destined to a
> > > > > duplicated address of ours.
> > > > > 1117 */
> > > > > 1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
> > > > > 1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
> > > > > 1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
> > > > > 1121
> > > > > IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
> > > > > 1122 src = &t;
> > > > > 1123 }
> > > > >
> > > >
> > > > I don't think this is the proper code extract because the
> > > > traceroute6 packet
> > > > is not addressed directly to us.
> > > > It's addressed to another global unicast address and appears to
> > > > time out
> > > > because the TTL gets decremented.
> > > >
> > > > So I think the code that gets executed in this case is :
> > > > 1127 if (src == NULL) {
> > > > 1128 /*
> > > > 1129 * This case matches to multicasts, our anycast,
> > > > or unicasts
> > > > 1130 * that we do not own. Select a source address
> > > > based on the
> > > > 1131 * source address of the erroneous packet.
> > > > 1132 */
> > > > 1133 rt = rtalloc(sin6tosa(&sa6_src),
> > > > RT_RESOLVE, rtableid);
> > > > 1134 if (!rtisvalid(rt)) {
> > > > 1135 char addr[INET6_ADDRSTRLEN];
> > > > 1136
> > > > 1137 nd6log((LOG_DEBUG,
> > > > 1138 "%s: source can't be
> > > > determined: dst=%s\n",
> > > > 1139 __func__, inet_ntop(AF_INET6,
> > > > &sa6_src.sin6_addr,
> > > > 1140 addr, sizeof(addr;
> > > > 1141 rtfree(rt);
> > > > 1142 goto bad;
> > > > 1143 }
> > > > 1144 src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> > > > 1145
> > > > 1146
Bogus assertwaitok() in usb_block_allocmem()
usb_block_allocmem() does not sleep and is careful to always use the
BUS_DMA_NOWAIT flag. So why the assertwaitok()?
Gerhard
Index: sys/dev/usb/usb_mem.c
===
RCS file: /cvs/src/sys/dev/usb/usb_mem.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 usb_mem.c
--- sys/dev/usb/usb_mem.c 18 Nov 2018 16:33:26 - 1.31
+++ sys/dev/usb/usb_mem.c 5 Dec 2018 10:53:39 -
@@ -108,8 +108,6 @@ usb_block_allocmem(bus_dma_tag_t tag, si
}
splx(s);
- assertwaitok();
-
DPRINTFN(6, ("usb_block_allocmem: no free\n"));
p = malloc(sizeof *p, M_USB, M_NOWAIT);
if (p == NULL)
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
Any feedback on this patch ?
I'm running it without problems since the 30th November.
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.227
diff -u -p -u -p -r1.227 icmp6.c
--- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
+++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
@@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
struct icmp6_hdr *icmp6;
struct in6_addr t, *src = NULL;
struct sockaddr_in6 sa6_src, sa6_dst;
+ struct in6_ifaddr *ifa;
u_int rtableid;
CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <=
MHLEN);
@@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
rtfree(rt);
goto bad;
}
- src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
+ ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t,
rtableid);
+ if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
+ if (src == NULL) src =
&ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
}
ip6->ip6_src = *src;
Le 2018-11-29 00:50, Arnaud BRAND a écrit :
I have setup a test environment where I can reproduce the issue with
GENERIC#481.
I can confirm that the issue is fixed by the proposed patch.
traceroute6 to/from/through the patched machine got expected result
and did not crash the machine.
Anyone else would like to try ?
Or propose improvements ?
Le 2018-11-29 00:02, Arnaud BRAND a écrit :
Le 2018-11-28 22:46, Arnaud BRAND a écrit :
Le 2018-10-05 23:42, Stuart Henderson a écrit :
On 2018/10/05 18:38, Alexander Bluhm wrote:
IPv6 Source selection is a mess!
> ICMP6 messages
> are generated with a source of, I think, the local address associated with
> the route to the recipient,
It is not that simple. Look at in6_ifawithscope() in
sys/netinet6/in6.c.
I know that's used for newly generated packets, but I'm not sure
it's the
case for icmp6, I haven't tried modifying the kernel to confirm for
sure
that this is the code generating the address in this case, but it
seems
likely, in icmp6.c:
/*
1112 * If the incoming packet was addressed directly to us
(i.e. unicast),
1113 * use dst as the src for the reply.
1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would
be
VERY rare,
1115 * but is possible (for example) when we encounter an
error while
1116 * forwarding procedure destined to a duplicated
address of ours.
1117 */
1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
1121
IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
1122 src = &t;
1123 }
I don't think this is the proper code extract because the traceroute6
packet
is not addressed directly to us.
It's addressed to another global unicast address and appears to time
out
because the TTL gets decremented.
So I think the code that gets executed in this case is :
1127 if (src == NULL) {
1128 /*
1129 * This case matches to multicasts, our anycast,
or unicasts
1130 * that we do not own. Select a source address
based on the
1131 * source address of the erroneous packet.
1132 */
1133 rt = rtalloc(sin6tosa(&sa6_src), RT_RESOLVE,
rtableid);
1134 if (!rtisvalid(rt)) {
1135 char addr[INET6_ADDRSTRLEN];
1136
1137 nd6log((LOG_DEBUG,
1138 "%s: source can't be determined:
dst=%s\n",
1139 __func__, inet_ntop(AF_INET6,
&sa6_src.sin6_addr,
1140 addr, sizeof(addr;
1141 rtfree(rt);
1142 goto bad;
1143 }
1144 src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
1145
1146 }
I was thinking of replacing lines 1144 and 1145 with something along
the lines of
src = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
if (src=NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
Sorry if style is wrong, I'm not used to it, but you get the idea.
Currently building a test kernel with this diff.
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.227
diff -u -p -u -p -r1.227 icmp6.c
--- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
+++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
@@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
struct icmp6_hdr *icmp6;
struct in6_addr t, *src = NULL;
struct sockaddr_in6 sa6_src, sa6_dst;
+ struct in6_i
Re: pvclock(4)
On 2018 Dec 04 (Tue) at 15:14:51 +0100 (+0100), Reyk Floeter wrote:
:On Tue, Dec 04, 2018 at 05:43:48AM -0800, Chris Cappuccio wrote:
:> Of course printf instead of panic for testers
:>
:
:Oh, right, thanks!
:
:@john: Does this "slightly less simple" diff work for you?
:
:@phessler, Chris: Maybe we should get this fix tested and in, wait for
:reports, and I can use the time to think about my other option. What
:do you think?
:
Yea, I think this diff would help avoid panics for systems where that
bit is never set, and should go in.
OK
:Reyk
:
:Index: sys/dev/pv/pvclock.c
:===
:RCS file: /cvs/src/sys/dev/pv/pvclock.c,v
:retrieving revision 1.2
:diff -u -p -u -p -r1.2 pvclock.c
:--- sys/dev/pv/pvclock.c 24 Nov 2018 13:12:29 - 1.2
:+++ sys/dev/pv/pvclock.c 4 Dec 2018 14:03:57 -
:@@ -70,6 +70,11 @@ uint pvclock_get_timecount(struct timec
: void pvclock_read_time_info(struct pvclock_softc *,
: struct pvclock_time_info *);
:
:+static inline uint32_t
:+ pvclock_read_begin(const struct pvclock_time_info *);
:+static inline int
:+ pvclock_read_done(const struct pvclock_time_info *, uint32_t);
:+
: struct cfattach pvclock_ca = {
: sizeof(struct pvclock_softc),
: pvclock_match,
:@@ -127,8 +132,11 @@ pvclock_match(struct device *parent, voi
: void
: pvclock_attach(struct device *parent, struct device *self, void *aux)
: {
:- struct pvclock_softc*sc = (struct pvclock_softc *)self;
:- paddr_t pa;
:+ struct pvclock_softc*sc = (struct pvclock_softc *)self;
:+ struct pvclock_time_info*ti;
:+ paddr_t pa;
:+ uint32_t version;
:+ uint8_t flags;
:
: if ((sc->sc_time = km_alloc(PAGE_SIZE,
: &kv_any, &kp_zero, &kd_nowait)) == NULL) {
:@@ -143,6 +151,19 @@ pvclock_attach(struct device *parent, st
:
: wrmsr(KVM_MSR_SYSTEM_TIME, pa | PVCLOCK_SYSTEM_TIME_ENABLE);
: sc->sc_paddr = pa;
:+
:+ ti = sc->sc_time;
:+ do {
:+ version = pvclock_read_begin(ti);
:+ flags = ti->ti_flags;
:+ } while (!pvclock_read_done(ti, version));
:+
:+ if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0) {
:+ wrmsr(KVM_MSR_SYSTEM_TIME, pa & ~PVCLOCK_SYSTEM_TIME_ENABLE);
:+ km_free(sc->sc_time, PAGE_SIZE, &kv_any, &kp_zero);
:+ printf(": unstable clock\n");
:+ return;
:+ }
:
: sc->sc_tc = &pvclock_timecounter;
: sc->sc_tc->tc_name = DEVNAME(sc);
:
--
Reality is an obstacle to hallucination.
be more strict when parsing netmasks for IPv6
When parsing a network mask into prefixlen be more paranoid and make sure
no value bigger then 128 is returned. In general this should never happen
but if it does the result can be bad.
This is for bgpd but there are other users in the tree. I will adjust them
if we dicide to go this way.
--
:wq Claudio
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.225
diff -u -p -r1.225 kroute.c
--- kroute.c5 Nov 2018 07:01:15 - 1.225
+++ kroute.c19 Nov 2018 12:46:23 -
@@ -2406,7 +2406,8 @@ mask2prefixlen(in_addr_t ina)
u_int8_t
mask2prefixlen6(struct sockaddr_in6 *sa_in6)
{
- u_int8_t l = 0, *ap, *ep;
+ u_int8_t*ap, *ep;
+ u_intl = 0;
/*
* sin6_len is the size of the sockaddr so substract the offset of
@@ -2422,32 +2423,35 @@ mask2prefixlen6(struct sockaddr_in6 *sa_
break;
case 0xfe:
l += 7;
- return (l);
+ goto done;
case 0xfc:
l += 6;
- return (l);
+ goto done;
case 0xf8:
l += 5;
- return (l);
+ goto done;
case 0xf0:
l += 4;
- return (l);
+ goto done;
case 0xe0:
l += 3;
- return (l);
+ goto done;
case 0xc0:
l += 2;
- return (l);
+ goto done;
case 0x80:
l += 1;
- return (l);
+ goto done;
case 0x00:
- return (l);
+ goto done;
default:
fatalx("non contiguous inet6 netmask");
}
}
+ done:
+ if (l > sizeof(struct in6_addr) * 8)
+ fatalx("%s: prefixlen %d out of bound", __func__, l);
return (l);
}
