DF-5413 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 | --- a/sys/bus/firewire/fwdev.c +++ b/sys/bus/firewire/fwdev.c @@ -97,6 +97,18 @@ { int i; + /* + * FW_SSTBUF hands us fully user-controlled values; without + * validation the 32-bit nchunk*npacket product in + * fwdma_malloc_multiseg() can wrap to 0 while bnchunk/bnpacket + * stay huge, leaving a 0-segment buffer that fw_read/fw_write + * index out of bounds via fwdma_v_addr(). (DF-5413) + */ + if (b->nchunk == 0 || b->npacket == 0 || + b->nchunk > 4096 || b->npacket > 512 || + b->psize == 0 || b->psize > FWPMAX_S400) + return(EINVAL); + if (q->flag & (FWXFERQ_RUNNING | FWXFERQ_EXTBUF)) return(EBUSY); @@ -223,8 +235,15 @@ /* remove binding */ for (fwb = STAILQ_FIRST(&ir->binds); fwb != NULL; fwb = STAILQ_FIRST(&ir->binds)) { + crit_enter(); STAILQ_REMOVE(&fc->binds, fwb, fw_bind, fclist); STAILQ_REMOVE_HEAD(&ir->binds, chlist); + crit_exit(); + /* free the bind's private xfer (leak, DF-5417) */ + while ((xfer = STAILQ_FIRST(&fwb->xferlist)) != NULL) { + STAILQ_REMOVE_HEAD(&fwb->xferlist, link); + fw_xfer_free(xfer); + } kfree(fwb, M_FW); } ir->flag &= ~(FWXFERQ_OPEN | @@ -302,20 +321,15 @@ err = EIO; return err; } else if(xfer != NULL) { -#if 0 /* XXX broken */ /* per packet mode or FWACT_CH bind?*/ crit_enter(); ir->queued --; STAILQ_REMOVE_HEAD(&ir->q, link); crit_exit(); - fp = &xfer->recv.hdr; - if (sc->fc->irx_post != NULL) - sc->fc->irx_post(sc->fc, fp->mode.ld); - err = uiomove((void *)fp, 1 /* XXX header size */, uio); - /* XXX copy payload too */ - /* XXX we should recycle this xfer */ -#endif - fw_xfer_free( xfer); + /* Dequeue before freeing: leaving the freed xfer linked + * in ir->q makes every later read free it again. + * (DF-5414) */ + fw_xfer_free(xfer); } else if(ir->stproc != NULL) { /* iso bulkxfer */ fp = (struct fw_pkt *)fwdma_v_addr(ir->buf, @@ -592,12 +606,37 @@ case FW_CBINDADDR: fwb = fw_bindlookup(sc->fc, bindreq->start.hi, bindreq->start.lo); - if(fwb == NULL){ + if (fwb == NULL || ir == NULL){ err = EINVAL; break; } + { + struct fw_bind *tfwb; + int found = 0; + + /* fw_bindlookup() is global: only unbind binds + * that were registered through this xferq, + * otherwise STAILQ_REMOVE() below walks a list + * that never contained fwb. (DF-5416) */ + STAILQ_FOREACH(tfwb, &ir->binds, chlist) + if (tfwb == fwb) { + found = 1; + break; + } + if (!found) { + err = EINVAL; + break; + } + } + crit_enter(); STAILQ_REMOVE(&sc->fc->binds, fwb, fw_bind, fclist); STAILQ_REMOVE(&ir->binds, fwb, fw_bind, chlist); + crit_exit(); + /* the bind's private xfer must not leak (DF-5417) */ + while ((xfer = STAILQ_FIRST(&fwb->xferlist)) != NULL) { + STAILQ_REMOVE_HEAD(&fwb->xferlist, link); + fw_xfer_free(xfer); + } kfree(fwb, M_FW); break; case FW_SBINDADDR: @@ -631,6 +670,10 @@ STAILQ_INSERT_TAIL(&fwb->xferlist, xfer, link); crit_exit(); err = fw_bindadd(sc->fc, fwb); + if (err) { + fw_xfer_free(xfer); + kfree(fwb, M_FW); + } break; case FW_GDEVLST: i = len = 1; @@ -720,6 +763,13 @@ ir = ((struct fw_drv1 *)dev->si_drv1)->ir; + /* no receive stream bound yet: rkq would be dereferenced + * through a NULL ir. (DF-5415) */ + if (ir == NULL) { + ap->a_result = ENODEV; + return (0); + } + ap->a_result = 0; switch (kn->kn_filter) { |