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

[Guy Yur]

Hi,

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.

> G>
> G> (gdb) bt
> G> #0  print_host (addr=0x2085a11a, port=7660, af=2 '\002', opts=1024) at
> G> /usr/src/sbin/pfctl/pf_print_state.c:178
> G> #1  0x00021c4c in print_state (s=0x2085a0f2, opts=1024) at
> G> /usr/src/sbin/pfctl/pf_print_state.c:236
> G> #2  0x0000c664 in pfctl_show_states (dev=<value optimized out>,
> G> iface=0x0, opts=1024) at /usr/src/sbin/pfctl/pfctl.c:1095
> G>
> G> sizeof(struct pfsync_state_key) is 36
> G> sizeof(struct pfsync_state_peer) is 32
> G> sizeof(struct pf_addr) is 16
> G> sizeof(struct pfsync_state) is 242
> G>

>
> I will try to fix this making new structure for the ioctl. That will mean
> moving slowly towards divorcing internal structures and ioctl ones.
>
> I'd appreciate if you file a PR on that, so that problem won't leave forgotten
> in the mailing list. You can even code the bugfix :)
>
> Thanks!
>
> --
> Totus tuus, Glebius.

I filled arm/185617 with some updated information.

After further looking at why the kernel doesn't crash when filling
the pfsync_state array and only the userspace pfctl is crashing I
see that pfsync_state has the __packed attribute which means on arm
unaligned access is used so there is no problem handling an unaligned
pfsync_state.

The reason pfctl crashes is because it passes a structure field
as a pf_addr pointer.  struct pf_addr is not __packed so on arm
word access will be used, triggering the unaligned fault.

So there is indeed no need to break the pfsync protocol.

In if_pfsync.c I think all the accesses to pfsync_state are done using
a pfsync_state pointer, there is no passing of struct fields as
separate pointers and since the struct is covered by __packed
there won't be an unaligned access.


Thanks,
Guy