Date: Tue, 23 Jan 2018 04:01:49 +0000 (UTC) From: Cy Schubert <cy@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r328274 - in stable: 10/sys/contrib/ipfilter/netinet 11/sys/contrib/ipfilter/netinet Message-ID: <201801230401.w0N41nfE027898@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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@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@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/10/sys/contrib/ipfilter/netinet/ip_state.c Directory Properties: stable/10/ (props changed) Changes in other areas also in this revision: Modified: stable/11/sys/contrib/ipfilter/netinet/ip_state.c Directory Properties: stable/11/ (props changed) Modified: stable/10/sys/contrib/ipfilter/netinet/ip_state.c ============================================================================== --- stable/10/sys/contrib/ipfilter/netinet/ip_state.c Tue Jan 23 03:36:49 2018 (r328273) +++ stable/10/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) @@ -5253,6 +5263,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; @@ -5279,6 +5290,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; @@ -5293,6 +5312,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,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201801230401.w0N41nfE027898>