kern/85123: Improper serialization in iir_ioctl() allows iir_intr() to reference freed memory - CRASH

[Dave Baukus]

>Number:         85123
>Category:       kern
>Synopsis:       Improper serialization in iir_ioctl() allows iir_intr() to reference freed memory - CRASH
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Aug 19 14:50:10 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dave Baukus
>Release:        bsd4.10 --> Bug still exists in TOB
>Organization:
Chiaro Networks
>Environment:
FreeBSD krakatoa.chiaro.com 4.10-RELEASE FreeBSD 4.10-RELEASE #0: Wed Jul 13 16:46:34 CDT 2005     root at krakatoa.chiaro.com:/usr/src/sys/compile/KRAKATOA  i386

>Description:
In sys/dev/iir/iir_ctrl.c::iir_ioctl() there is the following code:
switch (cmd) {
  case GDT_IOCTL_GENERAL:
  {
       gdt_ucmd_t *ucmd;
       struct gdt_softc *gdt;
       int lock;
       ucmd = (gdt_ucmd_t *)cmdarg;
       gdt = gdt_minor2softc(ucmd->io_node);
       if (gdt == NULL)
            return (ENXIO);
       lock = splcam();
       TAILQ_INSERT_TAIL(&gdt->sc_ucmd_queue, ucmd, links);
       ucmd->complete_flag = FALSE;
       splx(lock);
       gdt_next(gdt);
       if (!ucmd->complete_flag)
           (void) tsleep((void *)ucmd, PCATCH | PRIBIO, "iirucw", 0);
       break;
        }

If the command is not complete and tsleep() fails (by a pending signal)
before iir_intr() can process the request then the memory malloced by
the ioctl() system call will be freed, but the request is 
still referenced by the driver. 
Therefore, iir_intr() will process the request by accessing freed
memory.  If INVARIANTS are enabled (as we have here) then iir_intr()
ends up passing a length of 0xdeadcode to bcopy:

>From iir_intr():
      case GDT_GCF_IOCTL:
..
..
..
            } else {
                cnt = ucmd->u.raw.sdlen;
                if (cnt != 0)
                    bcopy(gccb->gc_scratch, ucmd->data, cnt);
                if (ucmd->u.raw.sense_len != 0)
                    bcopy(gccb->gc_scratch, ucmd->data, cnt);
            }

>From one of my crashes:
(kgdb) set $UCMD=(gdt_ucmd_t *)$SCBS->gc_ucmd
(kgdb) p *$UCMD
$219 = {io_node = 0xc0de, service = 0xdead, timeout = 0xc05076a0, status = 0x1, info = 0x0, BoardNode = 0xc0ded8b2,CommandIndex = 0xc0dedead, pCode = 0xdead, u = {cache = {DeviceNo = 0xc0de, BlockNo = 0xc0dedead, BlockCnt = 0xc0dedead, DestAddr = 0xc0dedead}, ioctl = {param_size = 0xc0de, subfunc = 0xc0dedead, channel = 0xc0dedead, p_param = 0xc0dedead}, raw = {reserved = 0xc0de, direction = 0xc0dedead,   mdisc_time = 0xc0dedead, mcon_time = 0xc0dedead, sdata = 0xc0dedead, sdlen = 0xc0dedead, clen = 0xc0dedead, cmd = "­ÞÞÀ­ÞÞÀ­ÞÞÀ", target = 0xad, lun = 0xde, bus = 0x1, priority = 0x0, sense_len = 0x0, sense_data = 0x0,   link_p = 0x10}}, data = "\001\000\0013", '\000' <repeats 3067 times>, complete_flag = 0xcb16c400, links = { tqe_next = 0xcb9f9000, tqe_prev = 0x0}}

===============
So the system crashes. I may be wrong on the entire sequence of events.
Clearly, however, the tsleep() code is broken in iir_ioctl(); this
is the only way I can explain the 2 system crashes I have seen in 
iir_intr() where bcopy() is called with some permutation of
0xdeadcode as the length.

     
>How-To-Repeat:
We have a raid status program that periodically polls the raid controler.
Perhaps under load the hole I explained above can be exploited.
2 crashes in a year - its a small hole but its there.

>Fix:
Check the return of tsleep() and cleanup if it fails.
Or loop until complete (ucmd->complete_flag)

>Release-Note:
>Audit-Trail:
>Unformatted: