[src/trunk]: src/sys/dev/usb Remove race introduced by using usbd_sync_transf...

[skrll <skrll%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/7f6f4b524fd6
branches:  trunk
changeset: 789657:7f6f4b524fd6
user:      skrll <skrll%NetBSD.org@localhost>
date:      Fri Aug 30 12:59:19 2013 +0000

description:
Remove race introduced by using usbd_sync_transfer_sig with a callback
that use the xfer cv (or does wake up on the xfer). The xfer has likely
been freed or re-used.

While here update utoppy(4) to use usbd_sync_transfer_sig.

Fixes PR/48151

diffstat:

 sys/dev/usb/usbdi_util.c |  43 ++++++++++---------------------------------
 sys/dev/usb/utoppy.c     |  43 +++++++------------------------------------
 2 files changed, 17 insertions(+), 69 deletions(-)

diffs (163 lines):

diff -r 3374c7077b5b -r 7f6f4b524fd6 sys/dev/usb/usbdi_util.c
--- a/sys/dev/usb/usbdi_util.c  Fri Aug 30 12:58:22 2013 +0000
+++ b/sys/dev/usb/usbdi_util.c  Fri Aug 30 12:59:19 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdi_util.c,v 1.60 2013/08/24 08:21:55 skrll Exp $    */
+/*     $NetBSD: usbdi_util.c,v 1.61 2013/08/30 12:59:19 skrll Exp $    */
 
 /*
  * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi_util.c,v 1.60 2013/08/24 08:21:55 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi_util.c,v 1.61 2013/08/30 12:59:19 skrll Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -415,19 +415,6 @@
        return (usbd_do_request(dev, &req, conf));
 }
 
-Static void usbd_bulk_transfer_cb(usbd_xfer_handle xfer,
-                                 usbd_private_handle priv, usbd_status status);
-Static void
-usbd_bulk_transfer_cb(usbd_xfer_handle xfer, usbd_private_handle priv,
-                     usbd_status status)
-{
-
-       if (xfer->pipe->device->bus->lock)
-               cv_broadcast(&xfer->cv);
-       else
-               wakeup(xfer);   /* XXXSMP ok */
-}
-
 usbd_status
 usbd_bulk_transfer(usbd_xfer_handle xfer, usbd_pipe_handle pipe,
                   u_int16_t flags, u_int32_t timeout, void *buf,
@@ -435,33 +422,21 @@
 {
        usbd_status err;
 
-       usbd_setup_xfer(xfer, pipe, 0, buf, *size,
-                       flags, timeout, usbd_bulk_transfer_cb);
+       usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout, NULL);
+
        DPRINTFN(1, ("usbd_bulk_transfer: start transfer %d bytes\n", *size));
+       err = usbd_sync_transfer_sig(xfer);
 
-       err = usbd_sync_transfer_sig(xfer);
        usbd_get_xfer_status(xfer, NULL, NULL, size, NULL);
        DPRINTFN(1,("usbd_bulk_transfer: transferred %d\n", *size));
        if (err) {
                DPRINTF(("usbd_bulk_transfer: error=%d\n", err));
                usbd_clear_endpoint_stall(pipe);
        }
+
        return (err);
 }
 
-Static void usbd_intr_transfer_cb(usbd_xfer_handle xfer,
-                                 usbd_private_handle priv, usbd_status status);
-Static void
-usbd_intr_transfer_cb(usbd_xfer_handle xfer, usbd_private_handle priv,
-                     usbd_status status)
-{
-
-       if (xfer->pipe->device->bus->lock)
-               cv_broadcast(&xfer->cv);
-       else
-               wakeup(xfer);   /* XXXSMP ok */
-}
-
 usbd_status
 usbd_intr_transfer(usbd_xfer_handle xfer, usbd_pipe_handle pipe,
                   u_int16_t flags, u_int32_t timeout, void *buf,
@@ -469,11 +444,13 @@
 {
        usbd_status err;
 
-       usbd_setup_xfer(xfer, pipe, 0, buf, *size,
-                       flags, timeout, usbd_intr_transfer_cb);
+       usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout, NULL);
+
        DPRINTFN(1, ("usbd_intr_transfer: start transfer %d bytes\n", *size));
        err = usbd_sync_transfer_sig(xfer);
+
        usbd_get_xfer_status(xfer, NULL, NULL, size, NULL);
+
        DPRINTFN(1,("usbd_intr_transfer: transferred %d\n", *size));
        if (err) {
                DPRINTF(("usbd_intr_transfer: error=%d\n", err));
diff -r 3374c7077b5b -r 7f6f4b524fd6 sys/dev/usb/utoppy.c
--- a/sys/dev/usb/utoppy.c      Fri Aug 30 12:58:22 2013 +0000
+++ b/sys/dev/usb/utoppy.c      Fri Aug 30 12:59:19 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: utoppy.c,v 1.21 2012/10/27 17:18:38 chs Exp $  */
+/*     $NetBSD: utoppy.c,v 1.22 2013/08/30 12:59:19 skrll Exp $        */
 
 /*-
  * Copyright (c) 2006 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: utoppy.c,v 1.21 2012/10/27 17:18:38 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: utoppy.c,v 1.22 2013/08/30 12:59:19 skrll Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -512,47 +512,18 @@
 }
 #endif
 
-/*
- * Very much like usbd_bulk_transfer(), except don't catch signals
- */
-static void
-utoppy_bulk_transfer_cb(usbd_xfer_handle xfer,
-    usbd_private_handle priv,
-    usbd_status status)
-{
-
-       if (xfer->pipe->device->bus->lock)
-               cv_broadcast(&xfer->cv);
-       else
-               wakeup(xfer);
-}
-
 static usbd_status
 utoppy_bulk_transfer(usbd_xfer_handle xfer, usbd_pipe_handle pipe,
     u_int16_t flags, u_int32_t timeout, void *buf, u_int32_t *size,
     const char *lbl)
 {
        usbd_status err;
-       int s, error;
+
+       usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout, NULL);
 
-       usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout,
-           utoppy_bulk_transfer_cb);
-       usbd_lock_pipe(pipe);   /* don't want callback until tsleep() */
-       err = usbd_transfer(xfer);
-       if (err != USBD_IN_PROGRESS) {
-               usbd_unlock_pipe(pipe);
-               return (err);
-       }
-       if (pipe->device->bus->lock)
-               error = cv_wait_sig(&xfer->cv, pipe->device->bus->lock);
-       else
-               error = tsleep((void *)xfer, PZERO, lbl, 0);
-       usbd_unlock_pipe(pipe);
-       if (error) {
-               usbd_abort_pipe(pipe);
-               return (USBD_INTERRUPTED);
-       }
-       usbd_get_xfer_status(xfer, NULL, NULL, size, &err);
+       err = usbd_sync_transfer_sig(xfer);
+
+       usbd_get_xfer_status(xfer, NULL, NULL, size, NULL);
        return (err);
 }