From: jhood@banana.ithaca.ny.us (John Hood)
Subject: FAS 2.09.1 bugs found and fixed
Organization: Pick a banana, any banana
Date: Thu, 23 Jul 1992 02:54:00 GMT


Several months ago, I started having problems with FAS 2.09.1 on my
busy modem lines.  After a generous amount of headscratching, far
too much study of the source code, and much time wasted waiting for
the modem lines to hang, I finally found the bugs.  I fixed them, and
reported them to Uwe, who will incorporate them into the next
version of FAS.  Meanwhile, I'm releasing a minimal patch for the
other suffering users of FAS.

I've had this patch installed on my system for nearly two months now,
and I have not had the modem lines hang since.  (I have had a *modem*
hang, though.  Telebit, fix your code :)

Sorry for not getting this out sooner.  Think of it this way-- it's
well tested.

This is a minimal set of patches that fixes the bugs and does nothing
else.  Apply to a standard fas 2.09.1 distribution.

--jh

John Hood, CU student, CU employee, and sometime BananaOp
jhood@albert.mannlib.cornell.edu,jhood@banana.ithaca.ny.us,jeh@crnlvax5.bitnet
By any reasonable computer's standard, I'm a virus.  So are you.


--buglist--*--buglist--*--buglist--*--buglist--*--buglist--*--buglist--

bug 1:

ports occasionally get stuck with a getty sleeping on them forever.
Usually, echo is on and the ports are running in cooked mode.  This
can be cured by killing the getty.

repeat-by:

  from a FAS tty, type:

$ exec cat /etc/passwd

  <interrupt cat near the end of the file, while FAS is draining its
  internal buffer; note absences of modem hangup and login: prompt>

In the real world, this is probably caused by line noise (frequently
including '^?') from a non-error-correcting modem when it is hung up
on. 

problem:

The kernel calls fasclose() upon closing the last open of a FAS
device.  fasclose() can be interrupted.  This leaves the kernel
thinking the device has been closed, and fas thinking the device is
still open (DF_DEVICE_OPEN).  When the next fasopen() occurs, the
driver hangs on a sleep.  Either it sleeps on ttyp->t_canq or it
becomes a special case of bug #2.  (I don't remember now exactly where
it got stuck; I just remember the central problem, and the fix)

fix:

change sleeps at low priority in fasclose() to a higher priority, so
they are uninterruptable.  My AIX 2 documentation (this is the best
driver documentation I have) says that an xx_close() routine should
never return an error; I conservatively interpret that to mean that it
should also always return, instead of allowing itself to be exited via
a signal.  Certainly, even if a fasclose exits by some route other
than returning, it should make sure it does something resembling the
normal close cleanup.

bug 2:

ports occasionally get stuck with a getty sleeping on them forever.
Usually, they are in a no-echo mode.  This can be cured by killing the
getty.

repeat-by:

  On a FAS tty set up to pay attention to DTR, do the following:

$ (trap "" 1; sleep 30) &
  <drop dtr for a short time, but long enough for a new getty to start>
  <raise dtr; note absence of login: prompt>

In the real world, this is probably caused by a sleeping inews in the
background keeping the port open (this is one unfortunate feature of
Cnews-- it'll sit there in the background if there's no disk space,
and it's SIGHUP-proofed), or with Waffle, Waffle not waiting for its
children when it gets SIGHUP and allowing another getty to start
before all its children die.  (Waffle is usually run as a shell.)

problem:

fasopen() will sleep if it is doing a getty open, there is a
preexisting getty open, and there is no carrier.  The only thing that
will wake this sleep is a fasclose().  But since the kernel will
consider the driver open because of the second fasopen(), it will
not call fasclose when the preexisting getty open is closed (actually,
it will never call fasclose).

fix:

replace if conditional for the first sleep in fasopen() with the code
used in fas 2.09.0.


--patches--*--patches--*--patches--*--patches--*--patches--*--patches--*

*** ../fas-dist/fas.c	Wed Jul 22 22:28:36 1992
--- fas.c	Sun May 24 20:08:05 1992
***************
*** 629,635
  	for (unit = 0; unit < fas_physical_units; unit++)
  		(void) printcfg ("fas", (uint) ((ushort) (fas_port [unit])), 7,
  					fas_vec [unit], -1,
! 					"unit=%d type=%c release=2.09.1",
  					unit, port_stat [unit]);
  #else
  	port_stat [unit] = '\0';

--- 629,635 -----
  	for (unit = 0; unit < fas_physical_units; unit++)
  		(void) printcfg ("fas", (uint) ((ushort) (fas_port [unit])), 7,
  					fas_vec [unit], -1,
! 					"unit=%d type=%c release=2.09.1.jh.1",
  					unit, port_stat [unit]);
  #else
  	port_stat [unit] = '\0';
***************
*** 633,639
  					unit, port_stat [unit]);
  #else
  	port_stat [unit] = '\0';
! 	(void) printf ("\nFAS 2.09.1 async driver: Unit 0-%d init state is [%s]\n\n",
  			unit - 1,
  			port_stat);
  #endif

--- 633,639 -----
  					unit, port_stat [unit]);
  #else
  	port_stat [unit] = '\0';
! 	(void) printf ("\nFAS 2.09.1.jh.1 async driver: Unit 0-%d init state is [%s]\n\n",
  			unit - 1,
  			port_stat);
  #endif
***************
*** 683,691
  	*/
  	for (;;)
  	{
! 		/* If this is a getty open, the FNDELAY flag is not set,
! 		   and the device is already open for dialout or it is
! 		   open and there is no carrier, wait until device
  		   is closed.
  		*/
  		while ((open_mode & OS_OPEN_FOR_GETTY) && !(flag & FNDELAY)

--- 683,690 -----
  	*/
  	for (;;)
  	{
! 		/* If this is a getty open, the device is already open for
! 		   dialout and the FNDELAY flag is not set, wait until device
  		   is closed.
  		*/
  		while ((open_mode & OS_OPEN_FOR_GETTY)
***************
*** 688,698
  		   open and there is no carrier, wait until device
  		   is closed.
  		*/
! 		while ((open_mode & OS_OPEN_FOR_GETTY) && !(flag & FNDELAY)
! 				&& ((fip->o_state & OS_OPEN_FOR_DIALOUT)
! 					|| ((fip->o_state & OS_OPEN_FOR_GETTY)
! 						&& !(fip->tty->t_state
! 								& CARR_ON))))
  		{
  			if (have_lock)
  			{

--- 687,695 -----
  		   dialout and the FNDELAY flag is not set, wait until device
  		   is closed.
  		*/
! 		while ((open_mode & OS_OPEN_FOR_GETTY)
! 					&& (fip->o_state & OS_OPEN_FOR_DIALOUT)
! 					&& !(flag & FNDELAY))
  		{
  			if (have_lock)
  			{
***************
*** 850,856
  		: fas_tty_ptr [physical_unit];
  	
  	old_level = SPLINT ();
! 	get_device_lock (fip, TTIPRI);
  
  	/* wait for output buffer drain only if device was open */
  	if (ttyp->t_state & ISOPEN)

--- 847,853 -----
  		: fas_tty_ptr [physical_unit];
  	
  	old_level = SPLINT ();
! 	get_device_lock (fip, PZERO - 1);
  
  	/* wait for output buffer drain only if device was open */
  	if (ttyp->t_state & ISOPEN)
***************
*** 855,861
  	/* wait for output buffer drain only if device was open */
  	if (ttyp->t_state & ISOPEN)
  	{
! 		/* wait for buffer drain and catch interrupts */
  		while (ttyp->t_outq.c_cc || (ttyp->t_state & (BUSY | TIMEOUT)))
  		{
  			ttyp->t_state |= TTIOW;

--- 852,858 -----
  	/* wait for output buffer drain only if device was open */
  	if (ttyp->t_state & ISOPEN)
  	{
! 		/* wait for buffer drain */
  		while (ttyp->t_outq.c_cc || (ttyp->t_state & (BUSY | TIMEOUT)))
  		{
  			ttyp->t_state |= TTIOW;
***************
*** 859,872
  		while (ttyp->t_outq.c_cc || (ttyp->t_state & (BUSY | TIMEOUT)))
  		{
  			ttyp->t_state |= TTIOW;
! 			if (sleep ((caddr_t) &ttyp->t_oflag, TTOPRI | PCATCH))
! 			{
! 				/* caught signal */
! 				ttyp->t_state &= ~TTIOW;
! 				release_device_lock (fip);
! 				(void) splx (old_level);
! 				longjmp (u.u_qsav);
! 			}
  		}
  		/* block transmitter and wait until it is
  		   empty

--- 856,862 -----
  		while (ttyp->t_outq.c_cc || (ttyp->t_state & (BUSY | TIMEOUT)))
  		{
  			ttyp->t_state |= TTIOW;
! 			sleep ((caddr_t) &ttyp->t_oflag, PZERO - 1);
  		}
  		/* block transmitter and wait until it is
  		   empty
-- 
John Hood, CU student, CU employee, and sometime BananaOp
jhood@albert.mannlib.cornell.edu,jhood@banana.ithaca.ny.us,jeh@crnlvax5.bitnet
By any reasonable computer's standard, I'm a virus.  So are you.