Date: Fri, 19 Aug 2005 14:33:53 GMT From: Dave Baukus <dbaukus@chiaro.com> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/85123: Improper serialization in iir_ioctl() allows iir_intr() to reference freed memory - CRASH Message-ID: <200508191433.j7JEXrtI095180@www.freebsd.org> Resent-Message-ID: <200508191450.j7JEoBvx008595@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200508191433.j7JEXrtI095180>