kern/118455: Panic with MD5 options on a TCP socket

[Tom Parker]

>Number:         118455
>Category:       kern
>Synopsis:       Panic with MD5 options on a TCP socket
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Dec 05 23:40:01 UTC 2007
>Closed-Date:
>Last-Modified:
>Originator:     Tom Parker
>Release:        N/A
>Organization:
>Environment:
>Description:
We're used a ported version of the BSD stack in a system in which we've just seen reproducible panics with MD5 options where the window size is shrunk.

I guess this might be able to be remotely exploited, not sure.

Referring to 1.145 of tcp_output.cpp

	if (len + optlen + ipoptlen > tp->t_maxopd) {
		flags &= ~TH_FIN;
		if (tso) {
			if (len > TCP_MAXWIN - hdrlen - optlen) {
				len = TCP_MAXWIN - hdrlen - optlen;
				len = len - (len % (tp->t_maxopd - optlen));
				sendalot = 1;
			} else if (tp->t_flags & TF_NEEDFIN)
				sendalot = 1;
		} else {
			len = tp->t_maxopd - optlen - ipoptlen;
			sendalot = 1;
		}
	}

In the above code we saw that without tso maxopd (=4) had shrunk to less than the optlen (=20 with MD5).  This gave a negative len which paniced in m_copym.  

		/*
		 * Start the m_copy functions from the closest mbuf
		 * to the offset in the socket buffer chain.
		 */
		mb = sbsndptr(&so->so_snd, off, len, &moff);

		if (len <= MHLEN - hdrlen - max_linkhdr) {
			m_copydata(mb, moff, (int)len,
			    mtod(m, caddr_t) + hdrlen);
			m->m_len += len;
		} else {
			m->m_next = m_copy(mb, moff, (int)len);
			if (m->m_next == NULL) {
				SOCKBUF_UNLOCK(&so->so_snd);
				(void) m_free(m);
				error = ENOBUFS;
				goto out;
			}
		}
We are using an older version but I believe the code above would panic given a negative len.


>How-To-Repeat:
Not sure, should be fixable by code inspection.
>Fix:
Our solution was to switch the test below from ==0 to >0, this causes no data to be sent.

	/*
	 * Grab a header mbuf, attaching a copy of data to
	 * be transmitted, and initialize the header from
	 * the template for sends on this connection.
	 */
	if (len) {

The real problem might be maxopd falling less than the mss.  The fix above makes the code safer in the mean time. 



>Release-Note:
>Audit-Trail:
>Unformatted: