Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 25 Jan 2019 01:06:06 +0000 (UTC)
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r343418 - head/sys/netpfil/pf
Message-ID:  <201901250106.x0P1668p093321@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kp
Date: Fri Jan 25 01:06:06 2019
New Revision: 343418
URL: https://svnweb.freebsd.org/changeset/base/343418

Log:
  pf: Fix use-after-free of counters
  
  When cleaning up a vnet we free the counters in V_pf_default_rule and
  V_pf_status from shutdown_pf(), but we can still use them later, for example
  through pf_purge_expired_src_nodes().
  
  Free them as the very last operation, as they rely on nothing else themselves.
  
  PR:		235097
  MFC after:	1 week

Modified:
  head/sys/netpfil/pf/pf_ioctl.c

Modified: head/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- head/sys/netpfil/pf/pf_ioctl.c	Fri Jan 25 01:05:18 2019	(r343417)
+++ head/sys/netpfil/pf/pf_ioctl.c	Fri Jan 25 01:06:06 2019	(r343418)
@@ -3989,20 +3989,6 @@ shutdown_pf(void)
 
 		/* status does not use malloced mem so no need to cleanup */
 		/* fingerprints and interfaces have their own cleanup code */
-
-		/* Free counters last as we updated them during shutdown. */
-		counter_u64_free(V_pf_default_rule.states_cur);
-		counter_u64_free(V_pf_default_rule.states_tot);
-		counter_u64_free(V_pf_default_rule.src_nodes);
-
-		for (int i = 0; i < PFRES_MAX; i++)
-			counter_u64_free(V_pf_status.counters[i]);
-		for (int i = 0; i < LCNT_MAX; i++)
-			counter_u64_free(V_pf_status.lcounters[i]);
-		for (int i = 0; i < FCNT_MAX; i++)
-			counter_u64_free(V_pf_status.fcounters[i]);
-		for (int i = 0; i < SCNT_MAX; i++)
-			counter_u64_free(V_pf_status.scounters[i]);
 	} while(0);
 
 	return (error);
@@ -4232,6 +4218,20 @@ pf_unload_vnet(void)
 	pf_cleanup();
 	if (IS_DEFAULT_VNET(curvnet))
 		pf_mtag_cleanup();
+
+	/* Free counters last as we updated them during shutdown. */
+	counter_u64_free(V_pf_default_rule.states_cur);
+	counter_u64_free(V_pf_default_rule.states_tot);
+	counter_u64_free(V_pf_default_rule.src_nodes);
+
+	for (int i = 0; i < PFRES_MAX; i++)
+		counter_u64_free(V_pf_status.counters[i]);
+	for (int i = 0; i < LCNT_MAX; i++)
+		counter_u64_free(V_pf_status.lcounters[i]);
+	for (int i = 0; i < FCNT_MAX; i++)
+		counter_u64_free(V_pf_status.fcounters[i]);
+	for (int i = 0; i < SCNT_MAX; i++)
+		counter_u64_free(V_pf_status.scounters[i]);
 }
 
 static void



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