kern/42312: Range problem in clrbit()s in tty_subr.c

[tlambert%apple.com@localhost]

>Number:         42312
>Category:       kern
>Synopsis:       Range problem in clrbit()s in tty_subr.c
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Nov 13 18:40:01 +0000 2009
>Originator:     Terry Lambert
>Release:        Release 5.0.1
>Organization:
Apple
>Environment:
N/A
>Description:
When the following code is run on a count of bytes or an offset + len that 
pushes it to exactly a byte boundary, it will zero one byte beyond the end of 
the buffer.  If the buffer is up against a page boundary, the system will 
panic, and if not, it will result in touching the first byte of the buffer 
following.  Since tty buffers are typically allocated in 1K chunks, there is a 
1:4 probabiliy of this being in unmapped space on a system with 4K pages.  This 
is the code:

[...]
        eby = (off+len) / NBBY;
        ebi = (off+len) % NBBY;
        if (sby == eby) {
                mask = ((1 << (ebi - sbi)) - 1) << sbi;
                cp[sby] &= ~mask;
        } else {
                mask = (1<<sbi) - 1;
                cp[sby++] &= mask;

                mask = (1<<ebi) - 1;
                cp[eby] &= ~mask;

                for (i = sby; i < eby; i++)
                        cp[i] = 0x00;
        }

...consider an 'off' value of 0x3fc and a 'len' value of 4; NBBY is 8, so:

(gdb) p/x (0x3fc + 4) % 8
$16 = 0x0 (ebi)
(gdb) p/x (0x3fc + 4) / 8
$17 = 0x80 (eby)

For this code:

                mask = (1<<ebi) - 1;
                cp[eby] &= ~mask;

                for (i = sby; i < eby; i++)

mask = (1<<0) - 1          /* == 0 */
cp[0x80] &= ~0;               /* promoted to lval type; unsigned char == 0xff */

...range should be per the 'for' loop, which is the interval [sby..eby).
>How-To-Repeat:
This is hard to reproduce, and is theoretical on the basis of problems observed 
in Mac OS X, which incorporates identical code.
>Fix:
-               mask = (1<<ebi) - 1;
-               cp[eby] &= ~mask;
---
+               if ((mask = (1<<ebi) - 1) != 0);
+                       cp[eby] &= ~mask;