git: d6381193a3e8 - stable/13 - pf: improve pf_state_key_attach() error handling

The branch stable/13 has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=d6381193a3e8bf4663863510fd8af8396f4fdb07

commit d6381193a3e8bf4663863510fd8af8396f4fdb07
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-03-27 14:21:41 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-04-21 21:14:16 +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 b5f872d40b02..363e678cbe24 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -1300,15 +1300,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! */
 				}
 			}