Date: Wed, 2 Jan 2008 19:55:17 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 132327 for review Message-ID: <200801021955.m02JtHGU033165@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=132327 Change 132327 by hselasky@hselasky_laptop001 on 2008/01/02 19:55:12 Fix a race in if_zyd regarding the interrupt endpoint. We need to do all verification in the "zyd_cfg_usb_intr_read()" function else we get trouble if the data arrives too early, hence the interrupt endpoint is running in the background. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/if_zyd.c#34 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/if_zyd.c#34 (text+ko) ==== @@ -409,7 +409,6 @@ struct zyd_softc *sc = xfer->priv_sc; struct zyd_cmd *cmd = &(sc->sc_intr_ibuf); uint32_t actlen; - uint32_t x; switch (USBD_GET_STATE(xfer)) { case USBD_ST_TRANSFERRED: @@ -454,12 +453,6 @@ /* try to clear stall first */ sc->sc_flags |= ZYD_FLAG_INTR_READ_STALL; usbd_transfer_start(sc->sc_xfer[ZYD_TR_INTR_CS_RD]); - } else { - /* tearing down */ - if (sc->sc_intr_iwakeup) { - bzero(sc->sc_intr_ibuf.data, sc->sc_intr_ilen); - wakeup(&(sc->sc_intr_iwakeup)); - } } break; } @@ -506,33 +499,13 @@ goto tr_setup; /* HMAC interrupt */ } if (actlen < 4) { + DPRINTF(sc, -1, "too short, %u bytes\n", actlen); goto tr_setup; /* too short */ } actlen -= 4; - if (actlen != sc->sc_intr_ilen) { - DPRINTF(sc, -1, "unexpected length %u != %u\n", - actlen, sc->sc_intr_ilen); - goto tr_setup; /* invalid length */ - } - actlen /= 4; + sc->sc_intr_ilen = actlen; - /* verify register values */ - for (x = 0; x != actlen; x++) { - if (sc->sc_intr_obuf.data[(2 * x)] != - sc->sc_intr_ibuf.data[(4 * x)]) { - /* invalid register */ - DPRINTF(sc, 0, "Invalid register (1)!\n"); - goto tr_setup; - } - if (sc->sc_intr_obuf.data[(2 * x) + 1] != - sc->sc_intr_ibuf.data[(4 * x) + 1]) { - /* invalid register */ - DPRINTF(sc, 0, "Invalid register (2)!\n"); - goto tr_setup; - } - } - if (sc->sc_intr_iwakeup) { sc->sc_intr_iwakeup = 0; wakeup(&(sc->sc_intr_iwakeup)); @@ -540,8 +513,8 @@ sc->sc_intr_iwakeup = 1; } /* - * We pause reading data from the interrupt endpoint until the data - * has been picked up! + * We pause reading data from the interrupt endpoint until the + * data has been picked up! */ return; } @@ -552,6 +525,9 @@ static void zyd_cfg_usb_intr_read(struct zyd_softc *sc, void *data, uint32_t size) { + uint16_t actlen; + uint16_t x; + if (size > sizeof(sc->sc_intr_ibuf.data)) { DPRINTF(sc, -1, "truncating transfer size!\n"); size = sizeof(sc->sc_intr_ibuf.data); @@ -563,33 +539,56 @@ if (sc->sc_intr_iwakeup) { DPRINTF(sc, 0, "got data already!\n"); sc->sc_intr_iwakeup = 0; - goto got_data; + goto skip0; } - /* else wait for data */ +repeat: + sc->sc_intr_iwakeup = 1; + + while (sc->sc_intr_iwakeup) { - sc->sc_intr_iwakeup = 1; - sc->sc_intr_ilen = size; + /* wait for data */ - usbd_transfer_start(sc->sc_xfer[ZYD_TR_INTR_DT_RD]); + usbd_transfer_start(sc->sc_xfer[ZYD_TR_INTR_DT_RD]); - while (sc->sc_intr_iwakeup) { if (mtx_sleep(&(sc->sc_intr_iwakeup), &(sc->sc_mtx), 0, "zyd-ird", hz / 2)) { /* should not happen */ } if (usbd_config_td_is_gone(&(sc->sc_config_td))) { - sc->sc_intr_iwakeup = 0; bzero(data, size); goto done; } } +skip0: + if (size != sc->sc_intr_ilen) { + DPRINTF(sc, -1, "unexpected length %u != %u\n", + size, sc->sc_intr_ilen); + goto repeat; + } + actlen = sc->sc_intr_ilen; + actlen /= 4; + + /* verify register values */ + for (x = 0; x != actlen; x++) { + if (sc->sc_intr_obuf.data[(2 * x)] != + sc->sc_intr_ibuf.data[(4 * x)]) { + /* invalid register */ + DPRINTF(sc, -1, "Invalid register (1) at %u!\n", x); + goto repeat; + } + if (sc->sc_intr_obuf.data[(2 * x) + 1] != + sc->sc_intr_ibuf.data[(4 * x) + 1]) { + /* invalid register */ + DPRINTF(sc, -1, "Invalid register (2) at %u!\n", x); + goto repeat; + } + } -got_data: bcopy(sc->sc_intr_ibuf.data, data, size); /* - * We have fetched the data from the shared buffer and it is safe to - * restart the interrupt transfer! + * We have fetched the data from the shared buffer and it is + * safe to restart the interrupt transfer! */ usbd_transfer_start(sc->sc_xfer[ZYD_TR_INTR_DT_RD]); done:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200801021955.m02JtHGU033165>