pf: don't use state keys after pf_state_insert()

[kp]

Summary

pf_state_insert() may free the state keys, it's not safe to access these
pointers after the call.

Introduce osrc/odst (similar to osport/odport) to store the original source and
destination addresses. This allows us to undo NAT transformations without having
to access the state keys.

MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

[glebius]

The pf_pdesc is always on the stack, is it?

[markj]

sys/netpfil/pf/pf.c

6266

Maybe it'd be nice to deduplicate this with the identical code in pf_return().

9927

This turns into a function call where we switch on af, even though we know its value here. It'd be nicer to have pf_addrcpy() be inlined, at least when the value of af is known at compile-time. Or have pf_addrcpy_inet()/pf_addrcpy_inet6().

[kp]

In D49551#1129918, @glebius wrote:

The pf_pdesc is always on the stack, is it?

Yes. In pf_test().
There's another instance in pf_icmp_state_lookup(), for the embedded packet in ICMP errors. That's on the stack too.

[kp]

sys/netpfil/pf/pf.c

6266

I'll propose a separate commit for that.

9927

I'm tempted to move pf_addrcpy() into pfvar.h so we can leave that to the compiler.
We basically always 'PF_ACPY()' to copy addresses, and I'd like to not have to think about why we sometimes do and sometimes don't.

[markj]

sys/netpfil/pf/pf.c

9927

That makes sense to me. I'd maybe also hardcode the value of af here, i.e., PF_ACPY(..., AF_INET6), just in case.