10.0-RC1, armv6: "pfctl -s state" crashes on BeagleBone Black due to unaligned access

[John-Mark Gurney]

Guy Yur wrote this message on Fri, Jan 10, 2014 at 01:04 +0200:
> On Fri, Jan 10, 2014 at 12:26 AM, John-Mark Gurney <jmg at funkthat.com> wrote:
> > Guy Yur wrote this message on Fri, Jan 10, 2014 at 00:17 +0200:
> >> On Thu, Jan 9, 2014 at 12:42 PM, Gleb Smirnoff <glebius at freebsd.org> wrote:
> >> >   Guy,
> >> >
> >> > On Sat, Jan 04, 2014 at 03:06:02PM +0200, Guy Yur wrote:
> >> > G> I am running 10.0-RC1 arm.armv6 on the BeagleBone Black.
> >> > G> The "pfctl -s state" command is crashing when trying to print the
> >> > G> second entry.
> >>
> 
> > Ok, that makes sense...  so, either we mark struct pf_addr as __packed,
> > or we do some nasty stuff, like the following in print_host:
> > struct {
> >         struct pf_addr a
> > } *uaddr __packed;
> >
> > uaddr = addr;
> > aw.v.a.addr = uaddr->a;
> >
> > it's not pretty, but I believe it would work...
> 
> For performance reasons, I don't think pf_addr should be marked as __packed.
> 
> I attached the changes I am now using in print_state() since there is
> no need to copy
> the full pfsync_state, only pf_addr.
> I converted sk and nk from pointers to structs on the stack and using
> struct copy.
> pf_addr is 16 bytes.

Did you look at using the above trick?

Since we are iterating over a list, that'll be a lot of copies, plus,
I'm not sure that your fix will be guaranteed to work for ever.. since
there isn't a requirement that the copy happens w/ bcopy/memcpy or some
other copy routine that assumes things might not be aligned...

Specificly these:
-               sk = &s->key[PF_SK_STACK];
-               nk = &s->key[PF_SK_WIRE];
+               sk = s->key[PF_SK_STACK];
+               nk = s->key[PF_SK_WIRE];

since s->key is already assumed to be aligned, a future compiler could
be smart enough to say, I'm not going to use the stack..  That
would/could happen if print_host's addr arg grew a const which it
could...

Also, I just realized that some of the lines modify sk (setting port),
but you don't write those modifications back to s->key[PF_SK_STACK]...

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."