diff options
| author | Martin Pieuchot <mpi@cvs.openbsd.org> | 2017-03-06 09:04:59 +0000 |
|---|---|---|
| committer | Martin Pieuchot <mpi@cvs.openbsd.org> | 2017-03-06 09:04:59 +0000 |
| commit | b17d53677f18cad70f07783f1d4ab96d4ebe5f40 (patch) | |
| tree | 9378519d864dabae96d6a1673a0f6ea9dffeb0df /sys | |
| parent | 6d81f1ac586504f01d184feaceff1ceee677f802 (diff) | |
It is unsafe to dereference ``xfer'' after calling the callback as it
might free it.
Prevent a use-after-free in various aynchronous cases. Found while
looking at another user-after-free pointed out by ehrhardt@.
Diffstat (limited to 'sys')
| -rw-r--r-- | sys/dev/usb/usbdi.c | 24 |
1 files changed, 15 insertions, 9 deletions
diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c index 2d25dd7bac3..b0d4946a573 100644 --- a/sys/dev/usb/usbdi.c +++ b/sys/dev/usb/usbdi.c @@ -1,4 +1,4 @@ -/* $OpenBSD: usbdi.c,v 1.85 2016/10/04 14:12:05 mpi Exp $ */ +/* $OpenBSD: usbdi.c,v 1.86 2017/03/06 09:04:58 mpi Exp $ */ /* $NetBSD: usbdi.c,v 1.103 2002/09/27 15:37:38 provos Exp $ */ /* $FreeBSD: src/sys/dev/usb/usbdi.c,v 1.28 1999/11/17 22:33:49 n_hibma Exp $ */ @@ -696,12 +696,13 @@ void usb_transfer_complete(struct usbd_xfer *xfer) { struct usbd_pipe *pipe = xfer->pipe; - int polling; + int polling = pipe->device->bus->use_polling; + int status, flags; SPLUSBCHECK; DPRINTFN(5, ("usb_transfer_complete: pipe=%p xfer=%p status=%d " - "actlen=%d\n", pipe, xfer, xfer->status, xfer->actlen)); + "actlen=%d\n", pipe, xfer, status, xfer->actlen)); #ifdef DIAGNOSTIC if (xfer->busy_free != XFER_ONQU) { printf("%s: xfer=%p not on queue\n", __func__, xfer); @@ -715,7 +716,6 @@ usb_transfer_complete(struct usbd_xfer *xfer) return; } #endif - polling = pipe->device->bus->use_polling; /* XXXX */ if (polling) pipe->running = 0; @@ -773,6 +773,13 @@ usb_transfer_complete(struct usbd_xfer *xfer) xfer->status = USBD_SHORT_XFER; } + /* + * We cannot dereference ``xfer'' after calling the callback as + * it might free it. + */ + status = xfer->status; + flags = xfer->flags; + if (pipe->repeat) { if (xfer->callback) xfer->callback(xfer, xfer->priv, xfer->status); @@ -789,17 +796,16 @@ usb_transfer_complete(struct usbd_xfer *xfer) * a new transfer as it will more likely results in the same * error. */ - if (xfer->status == USBD_IOERROR) + if (status == USBD_IOERROR) pipe->repeat = 0; - if ((xfer->flags & USBD_SYNCHRONOUS) && !polling) + if ((flags & USBD_SYNCHRONOUS) && !polling) wakeup(xfer); if (!pipe->repeat) { /* XXX should we stop the queue on all errors? */ - if ((xfer->status == USBD_CANCELLED || - xfer->status == USBD_IOERROR || - xfer->status == USBD_TIMEOUT) && + if ((status == USBD_CANCELLED || status == USBD_IOERROR || + status == USBD_TIMEOUT) && pipe->iface != NULL) /* not control pipe */ pipe->running = 0; else |