pfsync: Avoid zeroing the state export union

[markj]

Summary

pfsync_state_export() takes a pointer to a union that is in reality a
pointer to one of the three state formats (1301, 1400, 1500), and zeros
the union. The three formats do not have the same size, so zeroing is
wrong when the format isn't that which has the largest size.

Refactor a bit so that the zeroing happens at the layer where we know
which format we're dealing with.

Reported by: CHERI

[jrtc27]

sys/netpfil/pf/pf_ioctl.c

5802

These dereferences are still UB as far as I know, if you're dereferencing a union it must be a pointer to that actual union (with maybe some exceptions based on the layouts of all members?). Perhaps it doesn't trip up CHERI subobject bounds enforcement, but it's still dodgy...

[jrtc27]

sys/netpfil/pf/pf_ioctl.c

5802

(The way you're supposed to do this is have a struct type that is the same as the common prefix, and use that as the type to dereference, then cast to the more specialised types when known to deal with their specific members)

[markj]

sys/netpfil/pf/pf_ioctl.c

5802

Yes, though that's somewhat less severe a bug.

These structures are visible to userspace so it'd be a bit painful to factor out common fields into a separate struct without breaking builds. I wonder if it'd be acceptable to use a macro to copy common fields, and just get rid of the common pfsync_state_export()?

[jrtc27]

sys/netpfil/pf/pf_ioctl.c

5802

You don't actually need to use that common structs in the specific structs. There's a carve-out so:

struct foo_header {
  A a;
  B b;
  C c;
}

struct foo_1 {
  A a;
  B b;
  C c;
  D d;
}


struct foo_2 {
  A a;
  B b;
  C c;
  E e;
}

let you treat foo_1 and foo_2 as a foo_header.