Stefan Sperling writes:
> On Sun, Mar 03, 2019 at 01:14:06PM +0100, Stefan Sperling wrote: >> On Thu, Feb 28, 2019 at 10:53:35PM +0100, Stefan Sperling wrote: >> > This problem is still present in the latest snapshot: >> > kern.version=OpenBSD 6.5-beta (GENERIC) #1248: Thu Feb 28 09:57:20 MST 2019 >> > >> > Will anyone else have time to look at this soon or is this >> > sitting on my plate? >> >> struct sadb_x_counter violates an implicit assumption made by ipsecctl >> which assumes that all SADB extension have a size cleanly divisible by 8. >> Padding struct sadb_x_counter from 68 bytes to 72 bytes fixes the problem. >> >> OK? >> > > visa@ points out that according to RFC 2367 Section 2.2 the padding > belongs elsewhere: https://tools.ietf.org/html/rfc2367#section-2.2 > > I've verified that this also fixes the segfault. > Hi Stefan, > diff 716b4707a904aa2012c47dd657b6db0fd4417862 /usr/src > blob - 7c5d9a62eafaa49b574f517fb4296488fc2c4ad0 > file + sys/net/pfkeyv2.h > --- sys/net/pfkeyv2.h > +++ sys/net/pfkeyv2.h > @@ -221,6 +221,7 @@ struct sadb_x_tap { > struct sadb_x_counter { > uint16_t sadb_x_counter_len; > uint16_t sadb_x_counter_exttype; > + uint32_t pad; Perhaps call it sadb_x_counter_pad? There are reserved fields with names like sadb_x_foo_reserved so sadb_x_counter_pad would be consistent in my opinion. This brought me here in the first place (: > uint64_t sadb_x_counter_ipackets; /* Input IPsec packets */ > uint64_t sadb_x_counter_opackets; /* Output IPsec packets */ > uint64_t sadb_x_counter_ibytes; /* Input bytes */ > blob - 732a39d7004f8f55ee36a86fe40bebcde37da947 > file + sys/net/pfkeyv2_convert.c > --- sys/net/pfkeyv2_convert.c > +++ sys/net/pfkeyv2_convert.c > @@ -906,6 +906,7 @@ export_counter(void **p, struct tdb *tdb) > > scnt->sadb_x_counter_len = sizeof(struct sadb_x_counter) / > sizeof(uint64_t); > + scnt->pad = 0; > scnt->sadb_x_counter_ipackets = tdb->tdb_ipackets; > scnt->sadb_x_counter_opackets = tdb->tdb_opackets; > scnt->sadb_x_counter_ibytes = tdb->tdb_ibytes; It's hard to argue whether or not this is needed since nothing calls this function in the tree (: Looks like other PFKEYv2 structures avoid this problem (uint64_t is aligned to the 64 byte boundary). Ok mikeb regardless whether or not you'll be renaming the pad.
