>Number:         36851
>Category:       kern
>Synopsis:       large packets will crash FAST_IPSEC-kernel
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Aug 27 14:25:01 +0000 2007
>Originator:     Wolfgang Stukenbrock
>Release:        NetBSD 3.1
>Organization:
Dr. Nagler & Company GmbH
	
>Environment:
	
	
System: e010 3.1 NetBSD 3.1 (NSW-svc-ISDN) #13: Mon Aug 27 16:03:20 CEST 2007  ncadmin@e010:/usr/src/sys/arch/i386/compile/NSW-svc-ISDN i386
Architecture: i386
Machine: i386
>Description:
	When sending large packets though an ipsec tunnel, the m_makespace() routine in netipsec/ipsec_mbuf.c
	will fail in an assertion.
	The problem is that when sending a large icmp-ping packet (e.g. ping -s 2048 <host>) the data is located
	in exactly one mbuf and there is not enougth trailing space to add the ESP header.
	the ASSERTION, that the remaining data in the mbuf will fit into an new allocated mbuf will break.

        On the other hand the code to get the space is much too complicate.
	If the new header does not fit into the current mbuf, all remaining data must be moved.
	So we can get a new mbuf and move min(MLEN, remain) data to it. The rest can be handled like the
	case that there ist enougth roome inside of the mbuf, because hlen is forced to be less MLEN above.
	This will be done by the fix below.
>How-To-Repeat:
	Setup an esp tunnel like this:
	  spdadd 1.2.3.4 10.11.0.0/16 any -P out ipsec esp/tunnel/1.2.3.4-2.3.4.5/requrire;
	  add 1.2.3.4 2.3.4.5 esp 1234 -E blowfish-cbc "1234567890";

	When sending a packet trough the tunnel with "ping -s 2048 10.11.1.2" the system will crash.
>Fix:

*** ipsec_mbuf.c	2007/08/16 16:38:49	1.2
--- ipsec_mbuf.c	2007/08/27 14:13:21
***************
*** 240,248 ****
  	if (hlen > M_TRAILINGSPACE(m)) {
  		struct mbuf *n;
  
- 		/* XXX code doesn't handle clusters XXX */
- 		IPSEC_ASSERT(remain < MLEN,
- 			("m_makespace: remainder too big: %u", remain));
  		/*
  		 * Not enough space in m, split the contents
  		 * of m, inserting new mbufs as required.
--- 240,245 ----
***************
*** 255,315 ****
  		n->m_next = m->m_next;		/* splice new mbuf */
  		m->m_next = n;
  		newipsecstat.ips_mbinserted++;
! 		if (hlen <= M_TRAILINGSPACE(m) + remain) {
! 			/*
! 			 * New header fits in the old mbuf if we copy
! 			 * the remainder; just do the copy to the new
! 			 * mbuf and we're good to go.
! 			 */
! 			memcpy(mtod(n, caddr_t),
! 			       mtod(m, caddr_t) + skip, remain);
! 			n->m_len = remain;
! 			m->m_len = skip + hlen;
! 			*off = skip;
! 		} else {
! 			/*
! 			 * No space in the old mbuf for the new header.
! 			 * Make space in the new mbuf and check the
! 			 * remainder'd data fits too.  If not then we
! 			 * must allocate an additional mbuf (yech).
! 			 */
! 			n->m_len = 0;
! 			if (remain + hlen > M_TRAILINGSPACE(n)) {
! 				struct mbuf *n2;
! 
! 				MGET(n2, M_DONTWAIT, MT_DATA);
! 				/* NB: new mbuf is on chain, let caller free */
! 				if (n2 == NULL)
! 					return (NULL);
! 				n2->m_len = 0;
! 				memcpy(mtod(n2, caddr_t),
! 				       mtod(m, caddr_t) + skip, remain);
! 				n2->m_len = remain;
! 				/* splice in second mbuf */
! 				n2->m_next = n->m_next;
! 				n->m_next = n2;
! 				newipsecstat.ips_mbinserted++;
! 			} else {
! 				memcpy(mtod(n, caddr_t) + hlen,
! 				       mtod(m, caddr_t) + skip, remain);
! 				n->m_len += remain;
! 			}
! 			m->m_len -= remain;
! 			n->m_len += hlen;
! 			m = n;			/* header is at front ... */
! 			*off = 0;		/* ... of new mbuf */
! 		}
! 	} else {
! 		/*
! 		 * Copy the remainder to the back of the mbuf
! 		 * so there's space to write the new header.
! 		 */
! 		/* XXX can this be memcpy? does it handle overlap? */
! 		ovbcopy(mtod(m, caddr_t) + skip,
! 			mtod(m, caddr_t) + skip + hlen, remain);
! 		m->m_len += hlen;
! 		*off = skip;
! 	}
  	m0->m_pkthdr.len += hlen;		/* adjust packet length */
  	return m;
  }
--- 252,274 ----
  		n->m_next = m->m_next;		/* splice new mbuf */
  		m->m_next = n;
  		newipsecstat.ips_mbinserted++;
! 		/* copy somespace of mbuf into new one ... */
! 		if (remain < MLEN)
! 		  { memcpy(mtod(n, caddr_t), mtod(m, caddr_t) + m->m_len - remain, remain); n->m_len = remain; }
! 		else
! 		  { memcpy(mtod(n, caddr_t), mtod(m, caddr_t) + m->m_len - MLEN, MLEN); n->m_len = MLEN; }
! 		remain -= n->m_len;
! 		m->m_len -= n->m_len;
! 	}
! 	/*
! 	 * Copy the remainder to the back of the mbuf
! 	 * so there's space to write the new header.
! 	 */
! 	/* XXX can this be memcpy? does it handle overlap? */
! 	if (remain > 0)
! 	  ovbcopy(mtod(m, caddr_t) + skip, mtod(m, caddr_t) + skip + hlen, remain);
! 	m->m_len += hlen;
! 	*off = skip;
  	m0->m_pkthdr.len += hlen;		/* adjust packet length */
  	return m;
  }

>Unformatted: