Re: iked(8) and GCM
Hi, On Fri, May 17, 2013 at 12:55:15PM -0700, Aaron Stellman wrote: > Before I proceed, I realize that iked is not yet finished and is missing > some important security features. I am just pointing out something that > may not be known, and perhaps should be addressed. > ... > ikev2 esp from 10.92.19.85 to 10.92.20.160 \ > ikesa enc aes-256 prf hmac-sha2-512 group ecp521 \ > childsa enc aes-256-gmac group ecp521 \ > srcid openbsd00.foo.net dstid openbsd01.foo.net > ... > ikev2 active esp from 10.92.20.160 to 10.92.19.85 \ > ikesa enc aes-256 prf hmac-sha2-512 group ecp521 \ > childsa enc aes-256-gmac group ecp521 \ > srcid openbsd01.foo.net dstid openbsd00.foo.net ... > esp tunnel from 10.92.20.160 to 10.92.19.85 spi 0xdf0fbe53 enc aes-256-gmac > esp tunnel from 10.92.19.85 to 10.92.20.160 spi 0xffd4c5fd enc aes-256-gmac ... > esp tunnel from 10.92.20.160 to 10.92.19.85 spi 0xdf0fbe53 enc aes-256-gmac > esp tunnel from 10.92.19.85 to 10.92.20.160 spi 0xffd4c5fd enc aes-256-gmac > > If I do ipsecctl -sa -k, I see that authkey and enckey are the same, which, I > assume is "a good thing" for GCM. > > Now, here is the curious part. If openbsd01 pings openbsd00, the payload of > the > icmp packet appears to go in the clear: > You're mixing up GCM and GMAC. You have to update your config to use aes-256-gcm instead of aes-256-gmac! The GMAC is actually only the authentication part and it is not encrypting the payload. You can see it as "childsa enc null auth aes-256-gmac", but it is technically the encryption part that is calculating the GMAC. But I have to admit that the iked.conf(5) manpage is not describing it and is just listing the aes-???-gmac as encryption ciphers, so it is also our fault. I even see the point that a cipher AES-something is misleading in a way that most people will asume that it is encrypting the payload. We will think about a way to improve this. reyk
iked(8) and GCM
Before I proceed, I realize that iked is not yet finished and is missing some important security features. I am just pointing out something that may not be known, and perhaps should be addressed. I have a very simple instance of 2 qemu machines, running same snapshot of 5.3-current: OpenBSD openbsd00.foo.net 5.3 GENERIC#103 i386 openbsd00: # cat /etc/iked.conf # $OpenBSD: iked.conf,v 1.1 2010/06/07 10:09:05 reyk Exp $ # # See iked.conf(5) for syntax and examples. ikev2 esp from 10.92.19.85 to 10.92.20.160 \ ikesa enc aes-256 prf hmac-sha2-512 group ecp521 \ childsa enc aes-256-gmac group ecp521 \ srcid openbsd00.foo.net dstid openbsd01.foo.net openbsd01: # cat /etc/iked.conf # $OpenBSD: iked.conf,v 1.1 2010/06/07 10:09:05 reyk Exp $ # # See iked.conf(5) for syntax and examples. ikev2 active esp from 10.92.20.160 to 10.92.19.85 \ ikesa enc aes-256 prf hmac-sha2-512 group ecp521 \ childsa enc aes-256-gmac group ecp521 \ srcid openbsd01.foo.net dstid openbsd00.foo.net Both hosts start iked(8) as follows: # iked Both hosts are configured to use X509 certificates. I could provide more x509 details if necessary. IPsec tunnels come properly come up: openbsd00: # ipsecctl -sa FLOWS: flow esp in from 10.92.20.160 to 10.92.19.85 peer 10.92.20.160 srcid FQDN/openbsd00.foo.net dstid FQDN/openbsd01.foo.net type use flow esp out from 10.92.19.85 to 10.92.20.160 peer 10.92.20.160 srcid FQDN/openbsd00.foo.net dstid FQDN/openbsd01.foo.net type require flow esp out from ::/0 to ::/0 type deny SAD: esp tunnel from 10.92.20.160 to 10.92.19.85 spi 0xdf0fbe53 enc aes-256-gmac esp tunnel from 10.92.19.85 to 10.92.20.160 spi 0xffd4c5fd enc aes-256-gmac #openbsd01: # ipsecctl -sa FLOWS: flow esp in from 10.92.19.85 to 10.92.20.160 peer 10.92.19.85 srcid FQDN/openbsd01.foo.net dstid FQDN/openbsd00.foo.net type use flow esp out from 10.92.20.160 to 10.92.19.85 peer 10.92.19.85 srcid FQDN/openbsd01.foo.net dstid FQDN/openbsd00.foo.net type require flow esp out from ::/0 to ::/0 type deny SAD: esp tunnel from 10.92.20.160 to 10.92.19.85 spi 0xdf0fbe53 enc aes-256-gmac esp tunnel from 10.92.19.85 to 10.92.20.160 spi 0xffd4c5fd enc aes-256-gmac If I do ipsecctl -sa -k, I see that authkey and enckey are the same, which, I assume is "a good thing" for GCM. Now, here is the curious part. If openbsd01 pings openbsd00, the payload of the icmp packet appears to go in the clear: #openbsd01 # ping -c 2 10.92.19.85 PING 10.92.19.85 (10.92.19.85): 56 data bytes 64 bytes from 10.92.19.85: icmp_seq=0 ttl=255 time=0.880 ms 64 bytes from 10.92.19.85: icmp_seq=1 ttl=255 time=0.737 ms --- 10.92.19.85 ping statistics --- 2 packets transmitted, 2 packets received, 0.0% packet loss round-trip min/avg/max/std-dev = 0.737/0.808/0.880/0.076 ms #openbsd00: # tcpdump -s1515 -X -netti em0 esp tcpdump: listening on em0, link-type EN10MB 1368820214.387366 56:68:29:70:53:b2 56:68:29:70:4c:41 0800 154: esp 10.92.20.160 > 10.92.19.85 sf0fbe53 seq 4 len 120 : 5668 2970 4c41 5668 2970 53b2 0800 4500 Vh)pLAVh)pS...E. 0010: 008c 574a 4032 e649 0a5c 14a0 0a5c ..WJ..@2.I.\...\ 0020: 1355 df0f be53 0004 6640 e4d3 b1e9 .U...Sf@ 0030: 31c6 4500 0054 c9ab ff01 b550 0a5c 1.E..T...P.\ 0040: 14a0 0a5c 1355 0800 ecbe c837 5196 ...\.U.7..Q. 0050: 89f6 000e 7c6b 0809 0a0b 0c0d 0e0f 1011 |k.. 0060: 1213 1415 1617 1819 1a1b 1c1d 1e1f 2021 .. ! 0070: 2223 2425 2627 2829 2a2b 2c2d 2e2f 3031 "#$%&'()*+,-./01 0080: 3233 3435 3637 0102 0204 f729 d083 8bb5 234567.) 0090: 5af1 c042 bd14 278d 4b5e Z..B..'.K^ 1368820214.387653 56:68:29:70:4c:41 56:68:29:70:53:b2 0800 154: esp 10.92.19.85 > 10.92.20.160 sfd4c5fd seq 4 len 120 : 5668 2970 53b2 5668 2970 4c41 0800 4500 Vh)pS.Vh)pLA..E. 0010: 008c 52b1 4032 eae2 0a5c 1355 0a5c ..R...@2...\.U.\ 0020: 14a0 ffd4 c5fd 0004 dbb5 8e74 b8c8 .t.. 0030: 9ddd 4500 0054 4dd5 ff01 3127 0a5c ..E..TM.1'.\ 0040: 1355 0a5c 14a0 f4be c837 5196 .U.\...7..Q. 0050: 89f6 000e 7c6b 0809 0a0b 0c0d 0e0f 1011 |k.. 0060: 1213 1415 1617 1819 1a1b 1c1d 1e1f 2021 .. ! 0070: 2223 2425 2627 2829 2a2b 2c2d 2e2f 3031 "#$%&'()*+,-./01 0080: 3233 3435 3637 0102 0204 6c2b 9159 b678 234567l+.Y.x 0090: 2662 2db5 01f3 d14e ee2d &b-N.- 1368820215.399056 56:68:29:70:53:b2 56:68:29:70:4c:41 0800 154: esp 10.92.20.160 > 10.92.19.85 sf0fbe53 seq 5 len 120 : 5668 2970 4c41 5668 2970 53b2 0800 4500 Vh)pLAVh)pS...E. 0010: 008c a751 4032 9642 0a5c 14a0 0a5c ...Q..@2.B.\...\ 0020: 1355 df0f be53 0005 ee9f a3cb d2a6 .U...S.. 0030: c1bf 4500 0054 1c78 ff01 6284 0a5c ..E..T.xb..\ 0040: 14a0 0a5c 1355 0800 d290 c837 0001
Re: Unify and document usbd_transfer(9)
On 14/05/13(Tue) 16:09, Marcus Glocker wrote:
> On Fri, May 10, 2013 at 09:44:53AM +0200, Martin Pieuchot wrote:
>
> > So this is is one more step in my effort to merge the various code paths
> > to submit an USB transfer.
> >
> > This diff gets rid of the two badly named functions: usbd_bulk_transfer()
> > & usbd_intr_transfer() and makes use of the usbd_setup_xfer(9) +
> > usbd_transfer(9) combination. These functions were badly named because
> > they are identical wrappers to submit a synchronous transfer. There are
> > however two small functional differences with this diff:
> >
> > - previously a custom name for the wait channel was given to tlseep(9)
> >while sleeping for I/O. Now it will be "usbsyn" for all USB
> >synchronous transfers.
> >
> > - previously the priority given to tlseep(9) was PZERO. Now it will be
> >PRIBIO like for all USB synchronous transfers. But this shouldn't make
> >a difference in practice, because the only priority between the two,
> >PVFS, is unused.
> >
> > This diff also includes a new manual for the above mentioned functions.
> > I'd like to document as much functions of our USB stack as possible to
> > make sure driver porter/writer understand how it works.
Updated diff with two minors tweaks to make sure that:
- no information is leaked to userland when USBD_SHORT_XFER_OK is set
for ugen(4) reads
- do not bother checking for the actual transfered length for reads
in urio(4), USBD_SHORT_XFER_OK being not set, an error is returned
if actlen < len.
Plus includes some tweaks for the manpage by jmc@
Index: sys/dev/usb/ugen.c
===
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.72
diff -u -p -r1.72 ugen.c
--- sys/dev/usb/ugen.c 17 May 2013 09:09:11 - 1.72
+++ sys/dev/usb/ugen.c 17 May 2013 17:49:09 -
@@ -478,7 +478,7 @@ ugen_do_read(struct ugen_softc *sc, int
struct usbd_xfer *xfer;
usbd_status err;
int s;
- int error = 0;
+ int flags, error = 0;
u_char buffer[UGEN_CHUNK];
DPRINTFN(5, ("%s: ugenread: %d\n", sc->sc_dev.dv_xname, endpt));
@@ -546,14 +546,16 @@ ugen_do_read(struct ugen_softc *sc, int
xfer = usbd_alloc_xfer(sc->sc_udev);
if (xfer == 0)
return (ENOMEM);
+ flags = USBD_SYNCHRONOUS;
+ if (sce->state & UGEN_SHORT_OK)
+ flags |= USBD_SHORT_XFER_OK;
+ if (sce->timeout == 0)
+ flags |= USBD_CATCH;
while ((n = min(UGEN_BBSIZE, uio->uio_resid)) != 0) {
DPRINTFN(1, ("ugenread: start transfer %d bytes\n",n));
- tn = n;
- err = usbd_bulk_transfer(
- xfer, sce->pipeh,
- sce->state & UGEN_SHORT_OK ?
- USBD_SHORT_XFER_OK : 0,
- sce->timeout, buf, &tn, "ugenrb");
+ usbd_setup_xfer(xfer, sce->pipeh, 0, buf, n,
+ flags, sce->timeout, NULL);
+ err = usbd_transfer(xfer);
if (err) {
if (err == USBD_INTERRUPTED)
error = EINTR;
@@ -563,6 +565,7 @@ ugen_do_read(struct ugen_softc *sc, int
error = EIO;
break;
}
+ usbd_get_xfer_status(xfer, NULL, NULL, &tn, NULL);
DPRINTFN(1, ("ugenread: got %d bytes\n", tn));
error = uiomove(buf, tn, uio);
if (error || tn < n)
@@ -640,7 +643,7 @@ ugen_do_write(struct ugen_softc *sc, int
{
struct ugen_endpoint *sce = &sc->sc_endpoints[endpt][OUT];
u_int32_t n;
- int error = 0;
+ int flags, error = 0;
char buf[UGEN_BBSIZE];
struct usbd_xfer *xfer;
usbd_status err;
@@ -663,6 +666,9 @@ ugen_do_write(struct ugen_softc *sc, int
return (EIO);
}
#endif
+ flags = USBD_SYNCHRONOUS;
+ if (sce->timeout == 0)
+ flags |= USBD_CATCH;
switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
case UE_BULK:
@@ -674,8 +680,9 @@ ugen_do_write(struct ugen_softc *sc, int
if (error)
break;
DPRINTFN(1, ("ugenwrite: transfer %d bytes\n", n));
- err = usbd_bulk_transfer(xfer, sce->pipeh, 0,
- sce->timeout, buf, &n,"ugenwb");
+ usbd_setup_xfer(xfer, sce->pipeh, 0, buf, n,
+ flags, sce->timeout, NULL);
+ err = usbd_transfer(xfer);
if (err) {
"mpsafe" mutexes
So now that we have a mechanism to install interrupt handlers, it is
time to look at our mutexes again. To prevent lock ordering problems
with the kernel lock, we need to make sure we block all interrupts
that can grab the kernel lock. The simplest way to achieve this is to
make sure mutexes always raise the ipl to the highest level that has
interrupts that grab the kernel lock. On amd64 that currently is
IPL_AUDIO, but this will soon be IPL_VM or IPL_TTY. In theory this
increases interrupt latency. In practice it seems that this increase
is just noise compared to the latency we get due to kernel lock
contention.
I'd like to keep mutexes as lightweight as possible, and preferably
not add any code to the mtx_enter() and mtx_leave(). So the idea is
to adjust the ipl for the mutex when it gets initialized. The diff
below implements this strategy for amd64.
An earlier version of this diff has been tested by a couple of
devlopers already. IIRC mikeb@ even did some network benchmarking
with it and didn't notice any decreased performance. There are ways
to make this a bit smarter, but let's leave that until we've gained a
little bit more experience with running code that doesn't grab the
kernel lock. So I'd like to move forward with this, doing at least
i386 and sparc64 as soon as we we're done bikeshedding over this amd64
version ;).
ok?
Index: amd64/mutex.S
===
RCS file: /cvs/src/sys/arch/amd64/amd64/mutex.S,v
retrieving revision 1.8
diff -u -p -r1.8 mutex.S
--- amd64/mutex.S 24 Sep 2010 13:21:30 - 1.8
+++ amd64/mutex.S 17 May 2013 17:51:56 -
@@ -38,7 +38,7 @@
* Yeah, we don't really need to implement mtx_init here, but let's keep
* all the functions in the same place.
*/
-ENTRY(mtx_init)
+ENTRY(__mtx_init)
movl%esi, MTX_WANTIPL(%rdi)
movl$0, MTX_OLDIPL(%rdi)
movq$0, MTX_OWNER(%rdi)
Index: include/mutex.h
===
RCS file: /cvs/src/sys/arch/amd64/include/mutex.h,v
retrieving revision 1.4
diff -u -p -r1.4 mutex.h
--- include/mutex.h 23 Mar 2011 16:54:34 - 1.4
+++ include/mutex.h 17 May 2013 17:51:56 -
@@ -33,7 +33,17 @@ struct mutex {
__volatile void *mtx_owner;
};
-#define MUTEX_INITIALIZER(ipl) { (ipl), 0, NULL }
+#ifdef MULTIPROCESSOR
+#define __MUTEX_IPL(ipl) \
+(((ipl) > IPL_NONE && (ipl) < IPL_AUDIO) ? IPL_AUDIO : (ipl))
+#else
+#define __MUTEX_IPL(ipl) (ipl)
+#endif
+
+#define MUTEX_INITIALIZER(ipl) { __MUTEX_IPL((ipl)), 0, NULL }
+
+void __mtx_init(struct mutex *, int);
+#define mtx_init(mtx, ipl) __mtx_init((mtx), __MUTEX_IPL((ipl)))
#define MUTEX_ASSERT_LOCKED(mtx) do { \
if ((mtx)->mtx_owner != curcpu()) \
Re: Wake from zzz causes panic on thinkpad x60
Hi,
On Fri, 17 May 2013 14:06:49 +0200
Martin Pieuchot wrote:
> On 17/05/13(Fri) 19:25, YASUOKA Masahiko wrote:
>> On Fri, 1 Mar 2013 13:43:00 +
>> "Wade, Daniel" wrote:
>> > -Original Message-
>> > From: owner-t...@openbsd.org [mailto:owner-t...@openbsd.org] On Behalf Of
>> > Stefan Sperling
>> > Sent: Thursday, February 28, 2013 12:16 PM
>> > To: Edd Barrett
>> > Cc: tech@openbsd.org
>> > Subject: Re: Wake from zzz causes panic on thinkpad x60
>> >
>> > On Thu, Feb 28, 2013 at 04:59:12PM +, Edd Barrett wrote:
>> >> Went to run some TESTS for release and I am seeing panics when waking
>> >> my thinkpad x60 from zzz.
>> >>
>> >> I didn't have a serial line attached to get trace and ps, so I have
>> >> taken pictures of the kernel debugger. Sorry about that.
>> >>
>> >> http://farm9.staticflickr.com/8505/8516467142_1f3580e87a_c.jpg
>> >
>> > I've seen this panic in usb_abort_task_thread() on an x60s before.
>> > It doesn't happen often. It's not a new issue in recent snaps.
>>
>> Same problem happens on my sony vaio vgn-sz94s.
>> Attached diff will fix the problem.
>>
>> Remove `abort_task' from usb task queue before recycling a `struct
>> usbd_xfer object' which includes the `abort_task'. Otherwise
>> usb_abort_task_thread() may try to dequeue the recycled task then it
>> causes panic with page fault.
>
> Good analysis, but what about the less intrusive diff below from FreeBSD?
>
> It looks like when isochronous support has been imported, task
> cancellation were forgotten from the abort path.
Your diff seem to fix the problem more exactly.
> Does this also fix your panic?
Yes, it fixed the panic.
But I needed to apply same changes to uhci.c like attached diff.
Index: sys/dev/usb/ehci.c
===
RCS file: /cvs/openbsd/src/sys/dev/usb/ehci.c,v
retrieving revision 1.131
diff -u -p -r1.131 ehci.c
--- sys/dev/usb/ehci.c 19 Apr 2013 08:58:53 - 1.131
+++ sys/dev/usb/ehci.c 17 May 2013 15:56:30 -
@@ -800,6 +800,7 @@ ehci_check_itd_intr(struct ehci_softc *s
done:
DPRINTFN(12, ("ehci_check_itd_intr: ex=%p done\n", ex));
timeout_del(&ex->xfer.timeout_handle);
+ usb_rem_task(ex->xfer.pipe->device, &ex->abort_task);
ehci_idone(ex);
}
@@ -2859,6 +2860,7 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x
s = splusb();
xfer->status = status;
timeout_del(&xfer->timeout_handle);
+ usb_rem_task(epipe->pipe.device, &exfer->abort_task);
usb_transfer_complete(xfer);
splx(s);
return;
@@ -2883,6 +2885,7 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x
xfer->status = status;
timeout_del(&xfer->timeout_handle);
+ usb_rem_task(epipe->pipe.device, &exfer->abort_task);
s = splusb();
for (itd = exfer->itdstart; itd != NULL; itd = itd->xfer_next) {
Index: sys/dev/usb/uhci.c
===
RCS file: /cvs/openbsd/src/sys/dev/usb/uhci.c,v
retrieving revision 1.96
diff -u -p -r1.96 uhci.c
--- sys/dev/usb/uhci.c 19 Apr 2013 08:58:53 - 1.96
+++ sys/dev/usb/uhci.c 17 May 2013 15:57:26 -
@@ -1264,6 +1264,7 @@ uhci_check_intr(struct uhci_softc *sc, s
done:
DPRINTFN(12, ("uhci_check_intr: ii=%p done\n", ii));
timeout_del(&ii->xfer->timeout_handle);
+ usb_rem_task(ii->xfer->pipe->device, &UXFER(ii->xfer)->abort_task);
uhci_idone(ii);
}
@@ -1856,6 +1857,7 @@ uhci_abort_xfer(struct usbd_xfer *xfer,
s = splusb();
xfer->status = status; /* make software ignore it */
timeout_del(&xfer->timeout_handle);
+ usb_rem_task(xfer->pipe->device, &UXFER(xfer)->abort_task);
usb_transfer_complete(xfer);
splx(s);
return;
@@ -1870,6 +1872,7 @@ uhci_abort_xfer(struct usbd_xfer *xfer,
s = splusb();
xfer->status = status; /* make software ignore it */
timeout_del(&xfer->timeout_handle);
+ usb_rem_task(xfer->pipe->device, &UXFER(xfer)->abort_task);
DPRINTFN(1,("uhci_abort_xfer: stop ii=%p\n", ii));
for (std = ii->stdstart; std != NULL; std = std->link.std)
std->td.td_status &= htole32(~(UHCI_TD_ACTIVE | UHCI_TD_IOC));
Re: Wake from zzz causes panic on thinkpad x60
On 17/05/13(Fri) 19:25, YASUOKA Masahiko wrote:
> Hi,
>
> On Fri, 1 Mar 2013 13:43:00 +
> "Wade, Daniel" wrote:
> > -Original Message-
> > From: owner-t...@openbsd.org [mailto:owner-t...@openbsd.org] On Behalf Of
> > Stefan Sperling
> > Sent: Thursday, February 28, 2013 12:16 PM
> > To: Edd Barrett
> > Cc: tech@openbsd.org
> > Subject: Re: Wake from zzz causes panic on thinkpad x60
> >
> > On Thu, Feb 28, 2013 at 04:59:12PM +, Edd Barrett wrote:
> >> Went to run some TESTS for release and I am seeing panics when waking
> >> my thinkpad x60 from zzz.
> >>
> >> I didn't have a serial line attached to get trace and ps, so I have
> >> taken pictures of the kernel debugger. Sorry about that.
> >>
> >> http://farm9.staticflickr.com/8505/8516467142_1f3580e87a_c.jpg
> >
> > I've seen this panic in usb_abort_task_thread() on an x60s before.
> > It doesn't happen often. It's not a new issue in recent snaps.
>
> Same problem happens on my sony vaio vgn-sz94s.
> Attached diff will fix the problem.
>
> Remove `abort_task' from usb task queue before recycling a `struct
> usbd_xfer object' which includes the `abort_task'. Otherwise
> usb_abort_task_thread() may try to dequeue the recycled task then it
> causes panic with page fault.
Good analysis, but what about the less intrusive diff below from FreeBSD?
It looks like when isochronous support has been imported, task
cancellation were forgotten from the abort path.
Does this also fix your panic?
Index: ehci.c
===
RCS file: /home/ncvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.131
diff -u -p -r1.131 ehci.c
--- ehci.c 19 Apr 2013 08:58:53 - 1.131
+++ ehci.c 17 May 2013 12:05:37 -
@@ -800,6 +800,7 @@ ehci_check_itd_intr(struct ehci_softc *s
done:
DPRINTFN(12, ("ehci_check_itd_intr: ex=%p done\n", ex));
timeout_del(&ex->xfer.timeout_handle);
+ usb_rem_task(ex->xfer.pipe->device, &ex->abort_task);
ehci_idone(ex);
}
@@ -2859,6 +2860,7 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x
s = splusb();
xfer->status = status;
timeout_del(&xfer->timeout_handle);
+ usb_rem_task(epipe->pipe.device, &exfer->abort_task);
usb_transfer_complete(xfer);
splx(s);
return;
@@ -2883,6 +2885,7 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x
xfer->status = status;
timeout_del(&xfer->timeout_handle);
+ usb_rem_task(epipe->pipe.device, &exfer->abort_task);
s = splusb();
for (itd = exfer->itdstart; itd != NULL; itd = itd->xfer_next) {
Re: Compile time assert macro
> Date: Fri, 17 May 2013 13:24:45 +0200 (CEST) > From: Stefan Fritsch > > On Fri, 3 May 2013, Stefan Fritsch wrote: > > I think a CTASSERT macro like in FreeBSD would be nice. It could be used > > to verify assumptions about type and struct sizes. > > Ariane suggested making the syntax compatible to the C11 variant. This makes switching between KASSERT and CTASSERT painful. And you have to be creative and add a message string, which then isn't used... NetBSD has CTASSERT as well. I like CTASSERT better. Our kernel is not written in C11. > --- a/sys/lib/libkern/libkern.h > +++ b/sys/lib/libkern/libkern.h > @@ -138,6 +138,12 @@ abs(int j) > #endif > #endif > > +#if __STDC_VERSION__ < 201112L > +#define _Static_assert(predicate, msg) \ > + extern char __static_assert[(predicate) ? 1 : -1 ] \ > + __attribute__((__unused__)) > +#endif > + > /* Prototypes for non-quad routines. */ > void __assert(const char *, const char *, int, const char *) > __attribute__ ((__noreturn__)); > --- a/share/man/man9/kern.9 > +++ b/share/man/man9/kern.9 > @@ -108,6 +108,20 @@ enabled. > tests are only included if the kernel has > .Dv DEBUG > enabled. > +.Pp > +.nr nS 1 > +.Fn _Static_assert "CONDITION" "MESSAGE" > +.nr nS 0 > +.Pp > +This is a standard feature in C11 and is implemented as a macro if compiling > +with a pre-C11 compiler. > +It causes a compile time error if the given condition evaluates to > +false. > +Its main purpose is to verify assertions about type and struct sizes that > +would otherwise make the code fail at run time. > +.Fn _Static_assert > +can be used in global scope or at the start of blocks, where variable > +declarations are allowed. > .Sh BYTE STRINGS > .nr nS 1 > .Ft int > >
Re: Compile time assert macro
On Fri, 3 May 2013, Stefan Fritsch wrote: > I think a CTASSERT macro like in FreeBSD would be nice. It could be used > to verify assumptions about type and struct sizes. Ariane suggested making the syntax compatible to the C11 variant. OKs for this version? --- a/sys/lib/libkern/libkern.h +++ b/sys/lib/libkern/libkern.h @@ -138,6 +138,12 @@ abs(int j) #endif #endif +#if __STDC_VERSION__ < 201112L +#define_Static_assert(predicate, msg) \ + extern char __static_assert[(predicate) ? 1 : -1 ] \ + __attribute__((__unused__)) +#endif + /* Prototypes for non-quad routines. */ void__assert(const char *, const char *, int, const char *) __attribute__ ((__noreturn__)); --- a/share/man/man9/kern.9 +++ b/share/man/man9/kern.9 @@ -108,6 +108,20 @@ enabled. tests are only included if the kernel has .Dv DEBUG enabled. +.Pp +.nr nS 1 +.Fn _Static_assert "CONDITION" "MESSAGE" +.nr nS 0 +.Pp +This is a standard feature in C11 and is implemented as a macro if compiling +with a pre-C11 compiler. +It causes a compile time error if the given condition evaluates to +false. +Its main purpose is to verify assertions about type and struct sizes that +would otherwise make the code fail at run time. +.Fn _Static_assert +can be used in global scope or at the start of blocks, where variable +declarations are allowed. .Sh BYTE STRINGS .nr nS 1 .Ft int
Re: Possible relayd memory leak analysis
recent snaps don't have above mentioned problem. no sure what was the
cause, but leak is gone.
On Tue, Apr 9, 2013 at 1:47 AM, Alexey Suslikov
wrote:
> hi tech@
>
> tools used:
> * ps auxwww | grep relayd
> * httperf --hog --server=192.168.5.201 --wsess=25,1000,0.1 --rate=50
> --timeout=5
>
> target machine:
> OpenBSD 5.3-current (GENERIC.MP) #0: Sun Apr 7 15:14:10 EEST 2013
> *@*:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> /etc/relayd.conf:
>
> ext_addr="192.168.5.201"
> webhost1="192.168.5.202"
> webhost2="192.168.5.203"
>
> prefork 2
>
> table { $webhost1 $webhost2 }
>
> http protocol proto_pool_http {
> header append "$REMOTE_ADDR" to "X-Forwarded-For"
> header append "$SERVER_ADDR:$SERVER_PORT" to "X-Forwarded-By"
> header change "Connection" to "close"
> }
>
> relay cluster_pool_http {
> listen on $ext_addr port www
> protocol proto_pool_http
> forward to port www mode roundrobin check http "/index.html"
> host "test.local" code 200
> }
>
> cold ps auxwww:
>
> root 31403 0.0 0.1 1160 1916 ?? Ss12:21AM0:00.03
> relayd: parent (relayd)
> _relayd 18684 0.0 0.1 1044 2056 ?? S 12:21AM0:00.01
> relayd: pfe (relayd)
> _relayd 29554 0.0 0.1 912 1948 ?? S 12:21AM0:00.01
> relayd: hce (relayd)
> _relayd 7937 0.0 0.1 1108 2020 ?? S 12:21AM0:00.02
> relayd: relay (relayd)
> _relayd 28352 0.0 0.1 1108 2036 ?? S 12:21AM0:00.00
> relayd: relay (relayd)
>
> ps auxwww after 1st httperf run:
>
> _relayd 28352 4.1 0.6 10280 11672 ?? S 12:21AM0:08.83
> relayd: relay (relayd)
> _relayd 7937 4.8 0.6 10620 12004 ?? S 12:21AM0:09.17
> relayd: relay (relayd)
> root 31403 0.0 0.1 1160 1916 ?? Is12:21AM0:00.03
> relayd: parent (relayd)
> _relayd 18684 0.0 0.1 1044 2056 ?? S 12:21AM0:00.02
> relayd: pfe (relayd)
> _relayd 29554 0.0 0.1 912 1948 ?? S 12:21AM0:00.03
> relayd: hce (relayd)
>
> ps auxwww after 2nd httperf run:
>
> _relayd 28352 1.5 1.0 19424 20816 ?? S 12:21AM0:17.77
> relayd: relay (relayd)
> _relayd 7937 1.4 1.0 19724 21108 ?? S 12:21AM0:18.11
> relayd: relay (relayd)
> root 31403 0.0 0.1 1160 1916 ?? Is12:21AM0:00.03
> relayd: parent (relayd)
> _relayd 18684 0.0 0.1 1044 2056 ?? S 12:21AM0:00.02
> relayd: pfe (relayd)
> _relayd 29554 0.0 0.1 912 1952 ?? S 12:21AM0:00.05
> relayd: hce (relayd)
>
> on busy production setup relayd continuously leaks and eventually crashes.
Re: Wake from zzz causes panic on thinkpad x60
Hi,
On Fri, 1 Mar 2013 13:43:00 +
"Wade, Daniel" wrote:
> -Original Message-
> From: owner-t...@openbsd.org [mailto:owner-t...@openbsd.org] On Behalf Of
> Stefan Sperling
> Sent: Thursday, February 28, 2013 12:16 PM
> To: Edd Barrett
> Cc: tech@openbsd.org
> Subject: Re: Wake from zzz causes panic on thinkpad x60
>
> On Thu, Feb 28, 2013 at 04:59:12PM +, Edd Barrett wrote:
>> Went to run some TESTS for release and I am seeing panics when waking
>> my thinkpad x60 from zzz.
>>
>> I didn't have a serial line attached to get trace and ps, so I have
>> taken pictures of the kernel debugger. Sorry about that.
>>
>> http://farm9.staticflickr.com/8505/8516467142_1f3580e87a_c.jpg
>
> I've seen this panic in usb_abort_task_thread() on an x60s before.
> It doesn't happen often. It's not a new issue in recent snaps.
Same problem happens on my sony vaio vgn-sz94s.
Attached diff will fix the problem.
Remove `abort_task' from usb task queue before recycling a `struct
usbd_xfer object' which includes the `abort_task'. Otherwise
usb_abort_task_thread() may try to dequeue the recycled task then it
causes panic with page fault.
test, comment or ok?
Index: sys/dev/usb/ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.131
diff -u -p -r1.131 ehci.c
--- sys/dev/usb/ehci.c 19 Apr 2013 08:58:53 - 1.131
+++ sys/dev/usb/ehci.c 17 May 2013 08:37:12 -
@@ -1204,6 +1204,7 @@ void
ehci_freex(struct usbd_bus *bus, struct usbd_xfer *xfer)
{
struct ehci_softc *sc = (struct ehci_softc *)bus;
+ struct usb_task *task = &EXFER(xfer)->abort_task;
#ifdef DIAGNOSTIC
if (xfer->busy_free != XFER_BUSY) {
@@ -1217,6 +1218,8 @@ ehci_freex(struct usbd_bus *bus, struct
return;
}
#endif
+ if ((task->state & USB_TASK_STATE_ONQ) != 0)
+ usb_rem_task(xfer->pipe->device, task);
SIMPLEQ_INSERT_HEAD(&sc->sc_free_xfers, xfer, next);
}
Index: sys/dev/usb/ohci.c
===
RCS file: /cvs/src/sys/dev/usb/ohci.c,v
retrieving revision 1.111
diff -u -p -r1.111 ohci.c
--- sys/dev/usb/ohci.c 19 Apr 2013 08:58:53 - 1.111
+++ sys/dev/usb/ohci.c 17 May 2013 08:37:13 -
@@ -974,6 +974,7 @@ void
ohci_freex(struct usbd_bus *bus, struct usbd_xfer *xfer)
{
struct ohci_softc *sc = (struct ohci_softc *)bus;
+ struct usb_task *task = &((struct ohci_xfer *)xfer)->abort_task;
#ifdef DIAGNOSTIC
if (xfer->busy_free != XFER_BUSY) {
@@ -983,6 +984,8 @@ ohci_freex(struct usbd_bus *bus, struct
}
xfer->busy_free = XFER_FREE;
#endif
+ if ((task->state & USB_TASK_STATE_ONQ) != 0)
+ usb_rem_task(xfer->pipe->device, task);
SIMPLEQ_INSERT_HEAD(&sc->sc_free_xfers, xfer, next);
}
Index: sys/dev/usb/uhci.c
===
RCS file: /cvs/src/sys/dev/usb/uhci.c,v
retrieving revision 1.96
diff -u -p -r1.96 uhci.c
--- sys/dev/usb/uhci.c 19 Apr 2013 08:58:53 - 1.96
+++ sys/dev/usb/uhci.c 17 May 2013 08:37:14 -
@@ -653,6 +653,7 @@ void
uhci_freex(struct usbd_bus *bus, struct usbd_xfer *xfer)
{
struct uhci_softc *sc = (struct uhci_softc *)bus;
+ struct usb_task *task = &UXFER(xfer)->abort_task;
#ifdef DIAGNOSTIC
if (xfer->busy_free != XFER_BUSY) {
@@ -666,6 +667,8 @@ uhci_freex(struct usbd_bus *bus, struct
return;
}
#endif
+ if ((task->state & USB_TASK_STATE_ONQ) != 0)
+ usb_rem_task(xfer->pipe->device, task);
SIMPLEQ_INSERT_HEAD(&sc->sc_free_xfers, xfer, next);
}
