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
This commit is contained in:
skrll 2013-08-30 12:59:19 +00:00
parent 1f6a95ec03
commit 36c915dd13
2 changed files with 17 additions and 69 deletions

View File

@ -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 @@ usbd_get_config(usbd_device_handle dev, u_int8_t *conf)
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_bulk_transfer(usbd_xfer_handle xfer, usbd_pipe_handle pipe,
{
usbd_status err;
usbd_setup_xfer(xfer, pipe, 0, buf, *size,
flags, timeout, usbd_bulk_transfer_cb);
DPRINTFN(1, ("usbd_bulk_transfer: start transfer %d bytes\n", *size));
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);
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_intr_transfer(usbd_xfer_handle xfer, usbd_pipe_handle pipe,
{
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));

View File

@ -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 @@ utoppy_dump_packet(const void *b, size_t len)
}
#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,
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);
usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout, NULL);
err = usbd_sync_transfer_sig(xfer);
usbd_get_xfer_status(xfer, NULL, NULL, size, NULL);
return (err);
}