On Tue, May 04, 2021 at 07:29:20AM +0200, Peter J. Philipp wrote:
> > Index: print-wg.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/tcpdump/print-wg.c,v
> > retrieving revision 1.6
> > diff -u -p -u -r1.6 print-wg.c
> > --- print-wg.c 14 Apr 2021 19:34:56 -0000 1.6
> > +++ print-wg.c 3 May 2021 16:29:29 -0000
> > @@ -21,6 +21,7 @@
> >
> > #include <stdio.h>
> > #include <stddef.h>
> > +#include <string.h>
> >
> > #include "interface.h"
> > #include "extract.h"
> > @@ -104,6 +105,9 @@ wg_print(const u_char *bp, u_int length)
> > struct wg_data *data = (void *)bp;
> > u_int caplen;
> >
> > + uint32_t receiver;
> > + uint64_t nonce;
> > +
> > caplen = snapend - bp;
> > if (caplen < sizeof(type))
> > goto trunc;
> > @@ -142,8 +146,12 @@ wg_print(const u_char *bp, u_int length)
> > printf("[wg] keepalive ");
> > if (caplen < offsetof(struct wg_data, mac))
> > goto trunc;
> > +
> > + memcpy((void *)&receiver, (void *)&data->receiver,
> > sizeof(receiver));
> > + memcpy((void *)&nonce, (void *)&data->nonce, sizeof(nonce));
> > +
> > printf("to 0x%08x nonce %llu",
> > - letoh32(data->receiver), letoh64(data->nonce));
> > + letoh32(receiver), letoh64(nonce));
> > break;
> > }
> > return;
> >
> >
> > There may be other variables that need the same treatment... if that looks
> > ok
> > for you I'll work on that and submit the patch formally.
>
> I dumped with this patch for about an hour, and even restarted the wg tunnel
> on the device behind vlan6 so that other types get used (initialization) and
> I found that there was no more SIGBUS's for tcpdump on octeon, so I am unsure
> if other variables actually do need same treatment still.
data->nonce is the (most) offending variable because it needs 8-byte
alignment.
An alternative to memcpy() is to use tcpdump's EXTRACT_* macros that
handle unaligned data.
Yet another option is to declare the structs with the "packed" type
attribute. This makes the compiler emit machine code that is safe with
unaligned data. This also enables type checking that is better than with
EXTRACT_* because types are not overridden with explicit type casts.
However, "packed" makes it non obvious that unaligned accesses
may happen.
Index: print-wg.c
===================================================================
RCS file: src/usr.sbin/tcpdump/print-wg.c,v
retrieving revision 1.6
diff -u -p -r1.6 print-wg.c
--- print-wg.c 14 Apr 2021 19:34:56 -0000 1.6
+++ print-wg.c 14 Sep 2021 12:42:32 -0000
@@ -34,20 +34,20 @@ struct wg_initiation {
uint32_t type;
uint32_t sender;
uint8_t fill[140]; /* Includes ephemeral + MAC */
-};
+} __packed;
struct wg_response {
uint32_t type;
uint32_t sender;
uint32_t receiver;
uint8_t fill[80]; /* Includes ephemeral + MAC */
-};
+} __packed;
struct wg_cookie {
uint32_t type;
uint32_t receiver;
uint8_t fill[56]; /* Includes nonce + encrypted cookie */
-};
+} __packed;
struct wg_data {
uint32_t type;
@@ -55,7 +55,7 @@ struct wg_data {
uint64_t nonce;
/* uint8_t data[variable]; - Variable length data */
uint8_t mac[16];
-};
+} __packed;
/*
* Check if packet is a WireGuard packet, as WireGuard may run on any port.
