Subject: kern/319: bug in if_ppp.c
To: None <gnats-admin>
From: Alasdair Baird <alasdair@wildcat.demon.co.uk>
List: netbsd-bugs
Date: 07/02/1994 21:50:11
>Number:         319
>Category:       kern
>Synopsis:       if_ppp.c causes machine to crash
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    gnats-admin (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jul  2 21:50:07 1994
>Originator:     Alasdair Baird
>Organization:
absolutely none whatsoever
>Release:        current as of July 2nd
>Environment:
System: NetBSD wildcat.demon.co.uk 0.9C NetBSD 0.9C (WILDCAT) #14: Sun Jul 3 03:40:45 BST 1994 alasdair@wildcat.demon.co.uk:/usr/src/sys/arch/i386/compile/WILDCAT i386


>Description:
	On starting a ppp connection the machine crashes in pppfcs()
	while calculating a checksum; the crash is due to pppfcs()
	being passed an invalid length field for data in an mbuf.

	The mbuf is initially allocated by calling MGET() in the
	pppoutput() routine.  Subsequently a call will be made to
	M_TRAILINGSPACE(); now if the outgoing data is small enough
	to fit in the initial mbuf (i.e. no call to MCLGET() need be
	made) the M_TRAILINGSPACE() macro will attempt to use the
	m_len field of the mbuf.  Unfortunately this field is not
	initialised in MGET() so some quite arbitrary result gets
	returned and eventually used to initialise the m_len field.
	Subsequently pppoutput() calls pppasyncstart() which calls
	pppstart() which in turn calls pppfcs() to checksum the data
	in the mbuf.  Though the data is correct the length field is
	wholly bogus and will at some point cause a nasty vm_fault as
	a pointer runs off into never never land.
>How-To-Repeat:
	Start up a ppp connection and try sending some packets.
>Fix:
	There are I guess two ways to fix this problem, one being to
	patch the MGET() macro to initialise the m_len field to zero,
	and the other being to initialise it in the if_ppp.c code as
	needed.  I've chosen to do the latter as I have no proof that
	this is a problem in any other part of the code.  The context
	diff for this change is as follows:


*** if_ppp.c.old	Wed Jun 29 11:30:27 1994
--- if_ppp.c	Sun Jul  3 03:39:56 1994
***************
*** 423,428 ****
--- 423,429 ----
  	    m_freem(m0);
  	    return (ENOBUFS);
  	}
+ 	m->m_len = 0;
  	if (uio->uio_resid >= MCLBYTES / 2)
  	    MCLGET(m, M_DONTWAIT);
  	len = M_TRAILINGSPACE(m);


	This fix certainly seems to work, otherwise this mail message
	would never have arrived.

	   Alasdair.
>Audit-Trail:
>Unformatted:


------------------------------------------------------------------------------