git: 482f4dc272cc - stable/14 - pf: improve pf_state_key_attach() error handling
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 21 Apr 2025 21:15:43 UTC
The branch stable/14 has been updated by kp:
URL: https://cgit.FreeBSD.org/src/commit/?id=482f4dc272ccb73f80f07e838fe53d0ab2e85931
commit 482f4dc272ccb73f80f07e838fe53d0ab2e85931
Author: Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-03-27 14:21:41 +0000
Commit: Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-04-21 20:25:45 +0000
pf: improve pf_state_key_attach() error handling
If we fail to attach the stack key that means we've already attached the wire
key. That means the state could be found by other cores, and given that we then
free it, be used after free.
Fix this by not releasing the ID hashrow lock and key locks until after we've
removed the inserted key again, ensuring the state cannot be found by other
cores.
Reported by: markj
Submitted by: glebius
Reviewed by: glebius, markj
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D49550
(cherry picked from commit 8efd2acf07bc0e1c3ea1f7390e0f1cfb7cf6f86c)
---
sys/netpfil/pf/pf.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 06761060b583..c962821b8acd 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -1365,15 +1365,28 @@ keyattach:
printf("\n");
}
s->timeout = PFTM_UNLINKED;
+ if (idx == PF_SK_STACK)
+ /*
+ * Remove the wire key from
+ * the hash. Other threads
+ * can't be referencing it
+ * because we still hold the
+ * hash lock.
+ */
+ pf_state_key_detach(s,
+ PF_SK_WIRE);
PF_HASHROW_UNLOCK(ih);
KEYS_UNLOCK();
- if (idx == PF_SK_WIRE) {
+ if (idx == PF_SK_WIRE)
+ /*
+ * We've not inserted either key.
+ * Free both.
+ */
uma_zfree(V_pf_state_key_z, skw);
- if (skw != sks)
- uma_zfree(V_pf_state_key_z, sks);
- } else {
- pf_detach_state(s);
- }
+ if (skw != sks)
+ uma_zfree(
+ V_pf_state_key_z,
+ sks);
return (EEXIST); /* collision! */
}
}