pf: fix potential state key leak
ClosedPublic
Actions

Authored by kp on Nov 9 2024, 10:57 AM.

Details

Reviewers

None

Group Reviewers

network
pfsense

Commits

rGb4b2e420195a: pf: fix potential state key leak
rG93c80b79ad65: pf: fix potential state key leak
rG371bd29d4b22: pf: fix potential state key leak

Summary

If we fail to attach the new state keys in pf_state_key_attach() we are
expected to free both keys. Do so.
Note that in some scenarios both key are the same, so check for that
before freeing.

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

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kp created this revision.Nov 9 2024, 10:57 AM

Herald added subscribers: vegeta_tuxpowered.net, glebius, melifaro and 2 others. · View Herald TranscriptNov 9 2024, 10:57 AM

kp requested review of this revision.Nov 9 2024, 10:57 AM

Harbormaster completed remote builds in B60455: Diff 146195.Nov 9 2024, 10:57 AM

This revision was not accepted when it landed; it landed in state Needs Review.Nov 13 2024, 10:34 AM

Closed by commit rG371bd29d4b22: pf: fix potential state key leak (authored by kp). · Explain Why

This revision was automatically updated to reflect the committed changes.

kp added a commit: rG371bd29d4b22: pf: fix potential state key leak.

kp added a commit: rG93c80b79ad65: pf: fix potential state key leak.Nov 27 2024, 1:24 PM

kp added a commit: rGb4b2e420195a: pf: fix potential state key leak.

This commit likely causes the following panic reported by multiple users:

db:0:kdb.enter.default>  show pcpu
cpuid        = 2
dynamic pcpu = 0xfffffe0090458080
curthread    = 0xfffff800019ad000: pid 7 tid 100104 critnest 1 "pf purge"
curpcb       = 0xfffff800019ad520
fpcurthread  = none
idlethread   = 0xfffff800016d0740: tid 100005 "idle: cpu2"
self         = 0xffffffff83a12000
curpmap      = 0xffffffff81b81670
tssp         = 0xffffffff83a12384
rsp0         = 0xfffffe00853e9000
kcr3         = 0x74480000
ucr3         = 0xffffffffffffffff
scr3         = 0x2286f0000
gs32p        = 0xffffffff83a12404
ldt          = 0xffffffff83a12444
tss          = 0xffffffff83a12434
curvnet      = 0xfffff800011afd80
db:0:kdb.enter.default>  bt
Tracing pid 7 tid 100104 td 0xfffff800019ad000
kdb_enter() at kdb_enter+0x33/frame 0xfffffe00853e8c20
panic() at panic+0x43/frame 0xfffffe00853e8c80
trap_fatal() at trap_fatal+0x40b/frame 0xfffffe00853e8ce0
trap_pfault() at trap_pfault+0x46/frame 0xfffffe00853e8d30
calltrap() at calltrap+0x8/frame 0xfffffe00853e8d30
--- trap 0xc, rip = 0xffffffff82197d9c, rsp = 0xfffffe00853e8e00, rbp = 0xfffffe00853e8e30 ---
pf_detach_state() at pf_detach_state+0x5fc/frame 0xfffffe00853e8e30
pf_unlink_state() at pf_unlink_state+0x290/frame 0xfffffe00853e8e70
pf_purge_expired_states() at pf_purge_expired_states+0x188/frame 0xfffffe00853e8ec0
pf_purge_thread() at pf_purge_thread+0x13b/frame 0xfffffe00853e8ef0
fork_exit() at fork_exit+0x7f/frame 0xfffffe00853e8f30
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe00853e8f30
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---

I don't have a core dump at this point and I would appreciate a reasonable reaction to this report.

thanks Franco for the heads-up. The correct place for bugs is bugzilla, please report it there. As usual, it helps if you can provide a kgdb/backtrace with line numbers, and more details on the scenario, ideally with a reproducible test case on stock FreeBSD.

Done, for reference: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=283137