kern/46780: tty TIOCSQSIZE can free clist buffers which drivers are still using for output

[dennis%mistimed.com@localhost]

>Number:         46780
>Category:       kern
>Synopsis:       tty TIOCSQSIZE can free clist buffers which drivers are still 
>using for output
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Aug 07 17:15:00 +0000 2012
>Originator:     Dennis Ferguson
>Release:        6.99.10
>Organization:
>Environment:
NetBSD ts1.to.mistimed.ca 6.99.10 NetBSD 6.99.10 (GENERIC) #0: Tue Jul 31 
13:49:09 EDT 2012  
dennis%ts1.to.mistimed.ca@localhost:/usr/obj/sys/arch/amd64/compile/GENERIC 
amd64
>Description:
It is common for serial device drivers to cache a pointer into the tty clist 
buffer along with a character count in their private softc structure for use in 
the hardware transmit interrupt routine.  Here is a typical example from 
sys/dev/ic/com.c, in comstart():

        {
                u_char *tba;
                int tbc;
  
                tba = tp->t_outq.c_cf;
                tbc = ndqb(&tp->t_outq, 0);
  
                mutex_spin_enter(&sc->sc_lock);
 
                sc->sc_tba = tba;
                sc->sc_tbc = tbc;
        }

Many other drivers have similar code.

The tty TIOCSQSIZE ioctl() allows the size of this clist buffer to be changed.  
It does this, in sys/kern/tty.c, by calling tty_set_qsize(), which ultimately 
kmem_zalloc()'s a buffer of the requested new size, points the tty outq at the 
new buffer and then kmem_free()'s the old buffer.

The problem with this seems to be that if there is data present in old buffer 
when it is freed it will often be the case that the device driver will still 
have cached a pointer into the old buffer and will continue to transmit data 
from the now-free buffer until it exhausts the character count.  In addition, 
when the operation does complete, drivers seem to signal that the characters 
have now been consumed by calling ndflush() with a character count computed by 
pointer math which is now going to be a bit off.
>How-To-Repeat:
Do an output-buffer-sized write() to a (slower-is-better) serial port, then 
immediately make a TIOCSQSIZE call to change the buffer.
>Fix:
I don't think this can be fixed in the serial device drivers, they have good 
reasons to work the way they do.

I think what TIOCSQSIZE does with the input clists is okay, only the output has 
a problem.  An approach to fixing this might be to wait until the character 
count in the old output clist goes to zero before swapping in the new one.