svn commit: r328274 - in stable: 10/sys/contrib/ipfilter/netinet 11/sys/contrib/ipfilter/netinet

[Cy Schubert]

Author: cy
Date: Tue Jan 23 04:01:48 2018
New Revision: 328274
URL: https://svnweb.freebsd.org/changeset/base/328274

Log:
  MFC r327718:
  
  When growing the state, also grow the seed array. Otherwise memory
  that was not allocated will be accessed.
  
  This necessitated refactoring state seed allocation from
  ipf_state_soft_init() into a new common ipf_state_seed_alloc() function
  as it is now also used by ipf_state_rehash() when changing the size of
  the state hash table in addition to by ipf_state_soft_init() during
  initialization.
  
  According to Christos Zoulas <christos at NetBSD.org>:
  
  The bug was encountered by a NetBSD vendor who's customer machines had
  large ipfilter states. The bug was reliably triggered by resizing the
  state variables using "ipf -T".
  
  Submitted by:	Christos Zoulas <christos at NetBSD.org>
  Reviewed by:	delphij, rgrimes
  Obtained from:	NetBSD ip_state.c CVS revs r1.9 and r1.10
  Differential Revision:	https://reviews.freebsd.org/D13755

Modified:
  stable/11/sys/contrib/ipfilter/netinet/ip_state.c
Directory Properties:
  stable/11/   (props changed)

Changes in other areas also in this revision:
Modified:
  stable/10/sys/contrib/ipfilter/netinet/ip_state.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/11/sys/contrib/ipfilter/netinet/ip_state.c
==============================================================================
--- stable/11/sys/contrib/ipfilter/netinet/ip_state.c	Tue Jan 23 03:36:49 2018	(r328273)
+++ stable/11/sys/contrib/ipfilter/netinet/ip_state.c	Tue Jan 23 04:01:48 2018	(r328274)
@@ -301,7 +301,33 @@ ipf_state_soft_destroy(softc, arg)
 	KFREE(softs);
 }
 
+static void *
+ipf_state_seed_alloc(u_int state_size, u_int state_max)
+{
+	u_int i;
+	u_long *state_seed;
+	KMALLOCS(state_seed, u_long *, state_size * sizeof(*state_seed));
+	if (state_seed == NULL)
+		return NULL;
 
+	for (i = 0; i < state_size; i++) {
+		/*
+		 * XXX - ipf_state_seed[X] should be a random number of sorts.
+		 */
+#if  FREEBSD_GE_REV(400000)
+		state_seed[i] = arc4random();
+#else
+		state_seed[i] = ((u_long)state_seed + i) * state_size;
+		state_seed[i] ^= 0xa5a55a5a;
+		state_seed[i] *= (u_long)state_seed;
+		state_seed[i] ^= 0x5a5aa5a5;
+		state_seed[i] *= state_max;
+#endif
+	}
+	return state_seed;
+}
+
+
 /* ------------------------------------------------------------------------ */
 /* Function:    ipf_state_soft_init                                         */
 /* Returns:     int      - 0 == success, -1 == failure                      */
@@ -333,27 +359,11 @@ ipf_state_soft_init(softc, arg)
 	bzero((char *)softs->ipf_state_table,
 	      softs->ipf_state_size * sizeof(ipstate_t *));
 
-	KMALLOCS(softs->ipf_state_seed, u_long *,
-		 softs->ipf_state_size * sizeof(*softs->ipf_state_seed));
+	softs->ipf_state_seed = ipf_state_seed_alloc(softs->ipf_state_size,
+	    softs->ipf_state_max);
 	if (softs->ipf_state_seed == NULL)
 		return -2;
 
-	for (i = 0; i < softs->ipf_state_size; i++) {
-		/*
-		 * XXX - ipf_state_seed[X] should be a random number of sorts.
-		 */
-#if  FREEBSD_GE_REV(400000)
-		softs->ipf_state_seed[i] = arc4random();
-#else
-		softs->ipf_state_seed[i] = ((u_long)softs->ipf_state_seed + i) *
-				    softs->ipf_state_size;
-		softs->ipf_state_seed[i] ^= 0xa5a55a5a;
-		softs->ipf_state_seed[i] *= (u_long)softs->ipf_state_seed;
-		softs->ipf_state_seed[i] ^= 0x5a5aa5a5;
-		softs->ipf_state_seed[i] *= softs->ipf_state_max;
-#endif
-	}
-
 	KMALLOCS(softs->ipf_state_stats.iss_bucketlen, u_int *,
 		 softs->ipf_state_size * sizeof(u_int));
 	if (softs->ipf_state_stats.iss_bucketlen == NULL)
@@ -5259,6 +5269,7 @@ ipf_state_rehash(softc, t, p)
 {
 	ipf_state_softc_t *softs = softc->ipf_state_soft;
 	ipstate_t **newtab, *is;
+	u_long *newseed;
 	u_int *bucketlens;
 	u_int maxbucket;
 	u_int newsize;
@@ -5285,6 +5296,14 @@ ipf_state_rehash(softc, t, p)
 		return ENOMEM;
 	}
 
+	newseed = ipf_state_seed_alloc(newsize, softs->ipf_state_max);
+	if (newseed == NULL) {
+		KFREES(bucketlens, newsize * sizeof(*bucketlens));
+		KFREES(newtab, newsize * sizeof(*newtab));
+		IPFERROR(100037);
+		return ENOMEM;
+	}
+
 	for (maxbucket = 0, i = newsize; i > 0; i >>= 1)
 		maxbucket++;
 	maxbucket *= 2;
@@ -5299,6 +5318,12 @@ ipf_state_rehash(softc, t, p)
 		       softs->ipf_state_size * sizeof(*softs->ipf_state_table));
 	}
 	softs->ipf_state_table = newtab;
+
+	if (softs->ipf_state_seed != NULL) {
+		KFREES(softs->ipf_state_seed,
+		       softs->ipf_state_size * sizeof(*softs->ipf_state_seed));
+	}
+	softs->ipf_state_seed = newseed;
 
 	if (softs->ipf_state_stats.iss_bucketlen != NULL) {
 		KFREES(softs->ipf_state_stats.iss_bucketlen,