Subject: CVS commit: src/sys/net
To: None <source-changes@NetBSD.org>
From: Christos Zoulas <christos@netbsd.org>
List: source-changes
Date: 05/21/2006 05:09:13
Module Name:	src
Committed By:	christos
Date:		Sun May 21 05:09:13 UTC 2006

Modified Files:
	src/sys/net: if_spppsubr.c

Log Message:
Fixes from David Boggs; in his words:

	/sys/net/if_spppvar.h says:

	"Lower layer drivers that are always ready to communicate
	(like hardware HDLC) can shortcut pp_up from pp_tls,
	and pp_down from pp_tlf."

	When I follow those instructions, I get a kernel stack
	overflow as soon as I open the HDLC device.

	Here is the loop:
	 sppp_ioctl calls sppp_lcp_open
	 sppp_lcp_open calls sppp_open_event
	 sppp_open_event calls sppp_lcp_tls
	 sppp_lcp_tls calls pp_tls
	 pp_tls is the SHORTCUT to sppp_lcp_up
	 sppp_lcp_up calls spp_lcp_open
	 ...and around we go until the stack overflows.

	The fix is to reverse the order of the action (tls)
	and the state change (from INITIAL to STARTING) in
	sppp_open_event.

	There is a similar loop during closing:
	 sppp_ioctl calls sppp_lcp_close
	 sppp_lcp_close calls sppp_close_event
	 spp_close_event calls sppp_lcp_tlf
	 sppp_lcp_tlf calls pp_tlf
	 pp_tlf is the SHORTCUT to sppp_lcp_down
	 sppp_lcp_down calls sppp_lcp_close
	 ...and around we go until the stack overflows.

	The fix is to reverse the order of the action (tlf)
	and the state change (from STARTING to INITIAL) in
	sppp_close_event.

	Separately, while I was discovering this, I noticed
	that pp_tlf was being called unconditionally rather
	than first checking to see if it is NULL.  pp_tlf
	is a callout from sppp to the hdlc device driver.
	Elsewhere in sppp, this is always checked for NULL
	before calling it, and the comments in if_spppvar.h
	imply that filling it in is optional.

	From spppvar.h:
	"These functions need to be filled in by the lower layer
	(hardware) drivers if they request notification from the
	PPP layer whether the link is actually required."
	This clearly says that pp_tlf and pp_tls are optional
	and so sppp must check before calling them.


To generate a diff of this commit:
cvs rdiff -r1.90 -r1.91 src/sys/net/if_spppsubr.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.