From owner-freebsd-current@FreeBSD.ORG Tue Jul 23 12:04:15 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx2.freebsd.org (mx2.freebsd.org [8.8.178.116]) by hub.freebsd.org (Postfix) with ESMTP id 0F6E866F; Tue, 23 Jul 2013 12:04:15 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from butcher-nb.yandex.net (hub.freebsd.org [IPv6:2001:1900:2254:206c::16:88]) by mx2.freebsd.org (Postfix) with ESMTP id 2B30F2255; Tue, 23 Jul 2013 12:04:12 +0000 (UTC) Message-ID: <51EE7066.9030403@FreeBSD.org> Date: Tue, 23 Jul 2013 16:00:38 +0400 From: "Andrey V. Elsukov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Andre Oppermann Subject: Re: IPSEC crashes after r253088 References: <20130721054323.915f865769e6042c7dc62d08@tackymt.homeip.net> <51EE309D.2030106@FreeBSD.org> <51EE68D8.9020603@freebsd.org> In-Reply-To: <51EE68D8.9020603@freebsd.org> X-Enigmail-Version: 1.4.6 Content-Type: multipart/mixed; boundary="------------060701060904000402090606" Cc: George Neville-Neil , Taku YAMAMOTO , freebsd-current@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Jul 2013 12:04:15 -0000 This is a multi-part message in MIME format. --------------060701060904000402090606 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 23.07.2013 15:28, Andre Oppermann wrote: > On 23.07.2013 09:28, Andrey V. Elsukov wrote: >> On 21.07.2013 00:43, Taku YAMAMOTO wrote: >>> After r253088, systems with IPSEC and KSTACK_PAGES < 4 crashes on >>> booting into multi-user mode. >>> >>> The crash is due to sysctl -a in /etc/rc.d/initrandom ended up with >>> kernel stack overflow. >> >>> where type is struct ipsecstat which is 12560 bytes of size (larger than >>> 3 pages) of size when processing net.inet.ipsec.ipsecstats. >> >> Hi, >> >> Only few fields of struct ipsecstat is used, the rest fields are never >> updated. We can split it to several structures, or just remove unused >> fields. What is better? > > Not storing it on the stack? Also, I already prepared patch to test. -- WBR, Andrey V. Elsukov --------------060701060904000402090606 Content-Type: text/plain; charset=UTF-8; name="ipsec.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="ipsec.diff" Index: sys/netinet/sctp_input.c =================================================================== --- sys/netinet/sctp_input.c (revision 253562) +++ sys/netinet/sctp_input.c (working copy) @@ -5705,7 +5705,7 @@ sctp_common_input_processing(struct mbuf **mm, int #ifdef INET case AF_INET: if (ipsec4_in_reject(m, &inp->ip_inp.inp)) { - IPSECSTAT_INC(in_polvio); + IPSECSTAT_INC(ips_in_polvio); SCTP_STAT_INCR(sctps_hdrops); goto out; } @@ -5714,7 +5714,7 @@ sctp_common_input_processing(struct mbuf **mm, int #ifdef INET6 case AF_INET6: if (ipsec6_in_reject(m, &inp->ip_inp.inp)) { - IPSEC6STAT_INC(in_polvio); + IPSEC6STAT_INC(ips_in_polvio); SCTP_STAT_INCR(sctps_hdrops); goto out; } Index: sys/netinet/tcp_input.c =================================================================== --- sys/netinet/tcp_input.c (revision 253562) +++ sys/netinet/tcp_input.c (working copy) @@ -899,12 +899,12 @@ findpcb: #ifdef IPSEC #ifdef INET6 if (isipv6 && ipsec6_in_reject(m, inp)) { - IPSEC6STAT_INC(in_polvio); + IPSEC6STAT_INC(ips_in_polvio); goto dropunlock; } else #endif /* INET6 */ if (ipsec4_in_reject(m, inp) != 0) { - IPSECSTAT_INC(in_polvio); + IPSECSTAT_INC(ips_in_polvio); goto dropunlock; } #endif /* IPSEC */ Index: sys/netinet/udp_usrreq.c =================================================================== --- sys/netinet/udp_usrreq.c (revision 253562) +++ sys/netinet/udp_usrreq.c (working copy) @@ -282,7 +282,7 @@ udp_append(struct inpcb *inp, struct ip *ip, struc /* Check AH/ESP integrity. */ if (ipsec4_in_reject(n, inp)) { m_freem(n); - IPSECSTAT_INC(in_polvio); + IPSECSTAT_INC(ips_in_polvio); return; } #ifdef IPSEC_NAT_T @@ -1294,7 +1294,7 @@ udp4_espdecap(struct inpcb *inp, struct mbuf *m, i if (minlen > m->m_pkthdr.len) minlen = m->m_pkthdr.len; if ((m = m_pullup(m, minlen)) == NULL) { - IPSECSTAT_INC(in_inval); + IPSECSTAT_INC(ips_in_inval); return (NULL); /* Bypass caller processing. */ } data = mtod(m, caddr_t); /* Points to ip header. */ @@ -1334,7 +1334,7 @@ udp4_espdecap(struct inpcb *inp, struct mbuf *m, i uint32_t spi; if (payload <= sizeof(struct esp)) { - IPSECSTAT_INC(in_inval); + IPSECSTAT_INC(ips_in_inval); m_freem(m); return (NULL); /* Discard. */ } @@ -1355,7 +1355,7 @@ udp4_espdecap(struct inpcb *inp, struct mbuf *m, i tag = m_tag_get(PACKET_TAG_IPSEC_NAT_T_PORTS, 2 * sizeof(uint16_t), M_NOWAIT); if (tag == NULL) { - IPSECSTAT_INC(in_nomem); + IPSECSTAT_INC(ips_in_nomem); m_freem(m); return (NULL); /* Discard. */ } Index: sys/netinet6/ip6_forward.c =================================================================== --- sys/netinet6/ip6_forward.c (revision 253562) +++ sys/netinet6/ip6_forward.c (working copy) @@ -120,7 +120,7 @@ ip6_forward(struct mbuf *m, int srcrt) * before forwarding packet actually. */ if (ipsec6_in_reject(m, NULL)) { - IPSEC6STAT_INC(in_polvio); + IPSEC6STAT_INC(ips_in_polvio); m_freem(m); return; } @@ -182,7 +182,7 @@ ip6_forward(struct mbuf *m, int srcrt) sp = ipsec_getpolicybyaddr(m, IPSEC_DIR_OUTBOUND, IP_FORWARDING, &error); if (sp == NULL) { - IPSEC6STAT_INC(out_inval); + IPSEC6STAT_INC(ips_out_inval); IP6STAT_INC(ip6s_cantforward); if (mcopy) { #if 0 @@ -203,7 +203,7 @@ ip6_forward(struct mbuf *m, int srcrt) /* * This packet is just discarded. */ - IPSEC6STAT_INC(out_polvio); + IPSEC6STAT_INC(ips_out_polvio); IP6STAT_INC(ip6s_cantforward); KEY_FREESP(&sp); if (mcopy) { Index: sys/netinet6/raw_ip6.c =================================================================== --- sys/netinet6/raw_ip6.c (revision 253562) +++ sys/netinet6/raw_ip6.c (working copy) @@ -269,7 +269,7 @@ rip6_input(struct mbuf **mp, int *offp, int proto) */ if (n && ipsec6_in_reject(n, last)) { m_freem(n); - IPSEC6STAT_INC(in_polvio); + IPSEC6STAT_INC(ips_in_polvio); /* Do not inject data into pcb. */ } else #endif /* IPSEC */ @@ -301,7 +301,7 @@ rip6_input(struct mbuf **mp, int *offp, int proto) */ if ((last != NULL) && ipsec6_in_reject(m, last)) { m_freem(m); - IPSEC6STAT_INC(in_polvio); + IPSEC6STAT_INC(ips_in_polvio); IP6STAT_DEC(ip6s_delivered); /* Do not inject data into pcb. */ INP_RUNLOCK(last); Index: sys/netinet6/udp6_usrreq.c =================================================================== --- sys/netinet6/udp6_usrreq.c (revision 253562) +++ sys/netinet6/udp6_usrreq.c (working copy) @@ -141,7 +141,7 @@ udp6_append(struct inpcb *inp, struct mbuf *n, int /* Check AH/ESP integrity. */ if (ipsec6_in_reject(n, inp)) { m_freem(n); - IPSEC6STAT_INC(in_polvio); + IPSEC6STAT_INC(ips_in_polvio); return; } #endif /* IPSEC */ Index: sys/netipsec/ipsec.h =================================================================== --- sys/netipsec/ipsec.h (revision 253562) +++ sys/netipsec/ipsec.h (working copy) @@ -217,43 +217,17 @@ struct secspacq { /* statistics for ipsec processing */ struct ipsecstat { - uint64_t in_success; /* succeeded inbound process */ - uint64_t in_polvio; - /* security policy violation for inbound process */ - uint64_t in_nosa; /* inbound SA is unavailable */ - uint64_t in_inval; /* inbound processing failed due to EINVAL */ - uint64_t in_nomem; /* inbound processing failed due to ENOBUFS */ - uint64_t in_badspi; /* failed getting a SPI */ - uint64_t in_ahreplay; /* AH replay check failed */ - uint64_t in_espreplay; /* ESP replay check failed */ - uint64_t in_ahauthsucc; /* AH authentication success */ - uint64_t in_ahauthfail; /* AH authentication failure */ - uint64_t in_espauthsucc; /* ESP authentication success */ - uint64_t in_espauthfail; /* ESP authentication failure */ - uint64_t in_esphist[256]; - uint64_t in_ahhist[256]; - uint64_t in_comphist[256]; - uint64_t out_success; /* succeeded outbound process */ - uint64_t out_polvio; - /* security policy violation for outbound process */ - uint64_t out_nosa; /* outbound SA is unavailable */ - uint64_t out_inval; /* outbound process failed due to EINVAL */ - uint64_t out_nomem; /* inbound processing failed due to ENOBUFS */ - uint64_t out_noroute; /* there is no route */ - uint64_t out_esphist[256]; - uint64_t out_ahhist[256]; - uint64_t out_comphist[256]; + uint64_t ips_in_polvio; /* input: sec policy violation */ + uint64_t ips_in_nomem; /* input: no memory available */ + uint64_t ips_in_inval; /* input: generic error */ - uint64_t spdcachelookup; - uint64_t spdcachemiss; - - uint64_t ips_in_polvio; /* input: sec policy violation */ uint64_t ips_out_polvio; /* output: sec policy violation */ uint64_t ips_out_nosa; /* output: SA unavailable */ uint64_t ips_out_nomem; /* output: no memory available */ uint64_t ips_out_noroute; /* output: no route available */ uint64_t ips_out_inval; /* output: generic error */ uint64_t ips_out_bundlesa; /* output: bundled SA processed */ + uint64_t ips_mbcoalesced; /* mbufs coalesced during clone */ uint64_t ips_clcoalesced; /* clusters coalesced during clone */ uint64_t ips_clcopied; /* clusters copied during clone */ Index: usr.bin/netstat/ipsec.c =================================================================== --- usr.bin/netstat/ipsec.c (revision 253562) +++ usr.bin/netstat/ipsec.c (working copy) @@ -166,84 +166,18 @@ static struct val2str ipsec_compnames[] = { { -1, NULL }, }; -static void ipsec_hist(const u_quad_t *hist, size_t histmax, - const struct val2str *name, const char *title); static void print_ipsecstats(const struct ipsecstat *ipsecstat); - -/* - * Dump IPSEC statistics structure. - */ static void -ipsec_hist(const u_quad_t *hist, size_t histmax, const struct val2str *name, - const char *title) -{ - int first; - size_t proto; - const struct val2str *p; - - first = 1; - for (proto = 0; proto < histmax; proto++) { - if (hist[proto] <= 0) - continue; - if (first) { - printf("\t%s histogram:\n", title); - first = 0; - } - for (p = name; p && p->str; p++) { - if (p->val == (int)proto) - break; - } - if (p && p->str) { - printf("\t\t%s: %ju\n", p->str, (uintmax_t)hist[proto]); - } else { - printf("\t\t#%ld: %ju\n", (long)proto, - (uintmax_t)hist[proto]); - } - } -} - -static void print_ipsecstats(const struct ipsecstat *ipsecstat) { #define p(f, m) if (ipsecstat->f || sflag <= 1) \ printf(m, (uintmax_t)ipsecstat->f, plural(ipsecstat->f)) -#define pes(f, m) if (ipsecstat->f || sflag <= 1) \ - printf(m, (uintmax_t)ipsecstat->f, plurales(ipsecstat->f)) -#define hist(f, n, t) \ - ipsec_hist((f), sizeof(f)/sizeof(f[0]), (n), (t)); - - p(in_success, "\t%ju inbound packet%s processed successfully\n"); - p(in_polvio, "\t%ju inbound packet%s violated process security " - "policy\n"); - p(in_nosa, "\t%ju inbound packet%s with no SA available\n"); - p(in_inval, "\t%ju invalid inbound packet%s\n"); - p(in_nomem, "\t%ju inbound packet%s failed due to insufficient memory\n"); - p(in_badspi, "\t%ju inbound packet%s failed getting SPI\n"); - p(in_ahreplay, "\t%ju inbound packet%s failed on AH replay check\n"); - p(in_espreplay, "\t%ju inbound packet%s failed on ESP replay check\n"); - p(in_ahauthsucc, "\t%ju inbound packet%s considered authentic\n"); - p(in_ahauthfail, "\t%ju inbound packet%s failed on authentication\n"); - hist(ipsecstat->in_ahhist, ipsec_ahnames, "AH input"); - hist(ipsecstat->in_esphist, ipsec_espnames, "ESP input"); - hist(ipsecstat->in_comphist, ipsec_compnames, "IPComp input"); - - p(out_success, "\t%ju outbound packet%s processed successfully\n"); - p(out_polvio, "\t%ju outbound packet%s violated process security " - "policy\n"); - p(out_nosa, "\t%ju outbound packet%s with no SA available\n"); - p(out_inval, "\t%ju invalid outbound packet%s\n"); - p(out_nomem, "\t%ju outbound packet%s failed due to insufficient memory\n"); - p(out_noroute, "\t%ju outbound packet%s with no route\n"); - hist(ipsecstat->out_ahhist, ipsec_ahnames, "AH output"); - hist(ipsecstat->out_esphist, ipsec_espnames, "ESP output"); - hist(ipsecstat->out_comphist, ipsec_compnames, "IPComp output"); - p(spdcachelookup, "\t%ju SPD cache lookup%s\n"); - pes(spdcachemiss, "\t%ju SPD cache miss%s\n"); -#undef pes -#undef hist p(ips_in_polvio, "\t%ju inbound packet%s violated process " "security policy\n"); + p(ips_in_nomem, "\t%ju inbound packet%s failed due to " + "insufficient memory\n"); + p(ips_in_inval, "\t%ju invalid inbound packet%s\n"); p(ips_out_polvio, "\t%ju outbound packet%s violated process " "security policy\n"); p(ips_out_nosa, "\t%ju outbound packet%s with no SA available\n"); --------------060701060904000402090606--