HomeFreeBSD

pf: Fix source node locking

Description

pf: Fix source node locking

Source nodes are created quite early in pf_create_state(), even before
the state is allocated, locked and inserted into its hash row. They are
prone to being freed by source node killing or clearing ioctl while
pf_create_state() is still running.

The function pf_map_addr_sn() can be called in two very different paths.

One is for filter rules where it is called from
pf_create_state() after pf_insert_src_node(). In this case it is called
with a given source node and does not perform its own search and must
return the source node.

The other one is for NAT rules where it is called from
pf_get_translation() or its descendants. In this case it is called with
no known source node and performs its own search for source nodes. This
source node is then passed back to pf_create_state() without locking.

The states property of source node is increased in pf_find_src_node()
which allows for the counter to increase when a packet matches the NAT
rule but not a pass keep state rule.

The function pf_map_addr() operates on unlocked source node.

Modify pf_find_src_node() to return locked on source node found, so
that any subsequent operations can access the source node safely.

Move sn->states++ counter increase to pf_insert_src_node() to ensure
that it's called only from pf_create_state() and not from NAT ruleset
path, and have it increased only if the source node has really been
inserted or found, simplifying the cleanup.

Add locking in pf_src_connlimit() and pf_map_addr(). Sprinkle mutex
assertions in pf_map_addr().

Add a function pf_src_node_exists() to check a known source node is
still valid. Use it in pf_create_state() where it's impossible to hold
locks from pf_insert_src_node() because that would cause LoR (nodes
first, then state) against pf_src_connlimit() (state first, then node).

Don't propagate the source node found while parsing the NAT ruleset to
pf_create_state() because it must be found again and locked or created.

Reviewed by: kp
Approved by: kp (mentor)
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D47770

Details

Provenance
vegeta_tuxpowered.netAuthored on Sat, Nov 23, 9:21 PM
Reviewer
kp
Differential Revision
D47770: pf: Fix source node locking
Parents
rGde852d78ff58: hwpmc: Restore line lost in previous commit
Branches
Unknown
Tags
Unknown