always keep pf_state_keys attached to pf_states.
authordlg <dlg@openbsd.org>
Fri, 16 Dec 2022 02:05:44 +0000 (02:05 +0000)
committerdlg <dlg@openbsd.org>
Fri, 16 Dec 2022 02:05:44 +0000 (02:05 +0000)
commit437c6c3f437b1cb182e10d7cb7a6528e67c89fe2
treee878966889c483be6da7948c88e808d1867a6b55
parent1c6718c9e640f01a022c20c6f7fe907a4acd87c3
always keep pf_state_keys attached to pf_states.

pf_state structures don't contain ip addresses, protocols, ports,
etc. that information is stored in a pf_state_key struct, which is
used to wire a state into the state table. when things like pfsync
or the pf state ioctls want to export information about a state,
particularly the addresses on it, they needs the pf_state_key struct
to read from.

before this diff the code assumed that when a state was removed
from the state tables it could throw the pf_state_key structs away
as part of that removal. this code changes it so once pf_state_insert
succeeds, a pf_state will keep its references to the pf_state_key
structs until the pf_state struct itself is being destroyed.

this allows anything that holds a reference to a pf_state to also
look at the pf_state_key structs because they're now effectively
an immutable part of the pf_state struct.

this is by far the simplest and most straightforward fix for pfsync
crashing on pf_state_key dereferences we've come up with so far.
it has been made possible by the addition of reference counts to
pf_state and pf_state_key structs, which allows us to properly
account for this adjusted lifecycle for pf_state_keys on pf_state
structs.

sashan@ and i have been kicking this diff around for a couple of
weeks now.
ok sashan@ jmatthew@
sys/net/pf.c
sys/net/pfvar.h
sys/net/pfvar_priv.h