Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 10 Sep 2023 20:40:34 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: b6ce41118bb1 - main - pf: fix state leak
Message-ID:  <202309102040.38AKeYjw067299@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
The branch main has been updated by kp:

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

commit b6ce41118bb11d3db86eae8fbebc8c198e8b330d
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2023-09-06 07:58:07 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-09-10 20:40:22 +0000

    pf: fix state leak
    
    If we hit the csfailed case in pf_create_state() we may have allocated
    a state, so we must also free it. While here reduce the amount of
    duplicated cleanup code.
    
    MFC after:      2 weeks
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D41772
---
 sys/netpfil/pf/pf.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 21d2e16e83dc..df4bd47c35d5 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -4885,13 +4885,8 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a,
 	if (r->rt) {
 		/* pf_map_addr increases the reason counters */
 		if ((reason = pf_map_addr(pd->af, r, pd->src, &s->rt_addr,
-		    &s->rt_kif, NULL, &sn)) != 0) {
-			pf_src_tree_remove_state(s);
-			s->timeout = PFTM_UNLINKED;
-			STATE_DEC_COUNTERS(s);
-			pf_free_state(s);
+		    &s->rt_kif, NULL, &sn)) != 0)
 			goto csfailed;
-		}
 		s->rt = r->rt;
 	}
 
@@ -4947,11 +4942,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a,
 	    (pd->dir == PF_IN) ? sk : nk,
 	    (pd->dir == PF_IN) ? nk : sk, s)) {
 		REASON_SET(&reason, PFRES_STATEINS);
-		pf_src_tree_remove_state(s);
-		s->timeout = PFTM_UNLINKED;
-		STATE_DEC_COUNTERS(s);
-		pf_free_state(s);
-		return (PF_DROP);
+		goto drop;
 	} else
 		*sm = s;
 
@@ -5020,13 +5011,14 @@ csfailed:
 		PF_SRC_NODE_UNLOCK(nsn);
 	}
 
-	return (PF_DROP);
-
 drop:
-	pf_src_tree_remove_state(s);
-	s->timeout = PFTM_UNLINKED;
-	STATE_DEC_COUNTERS(s);
-	pf_free_state(s);
+	if (s != NULL) {
+		pf_src_tree_remove_state(s);
+		s->timeout = PFTM_UNLINKED;
+		STATE_DEC_COUNTERS(s);
+		pf_free_state(s);
+	}
+
 	return (PF_DROP);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202309102040.38AKeYjw067299>