Re: panic in upd

[Martin Pieuchot]

On 01/06/16(Wed) 16:39, Martijn van Duren wrote:
> [...] 
> upd0 detached
> uhidev0 detached
> kernel: protection fault trap, code=0
> Stopped at    upd_sensor_invalidate+0xe:      movq    0xc8(%rsi),%rbx
> ddb{0}> trace
> upd_sensor_invalidate() at upd_sensor_invalidate+0xe
> upd_update_report_cb() at upd_update_report_cb+0x5b
> uhidev_get_report_async_cb() at uhidev_get_report_async_cb+0x39
> usb_transfer_complete() at usb_transfer_complete+0x26c
> xhci_event_command() at xhci_event_command+0x1c8
> xhci_event_dequeue() at xhci_event_dequeue+0x8a
> xhci_softintr() at xhci_softintr+0x21
> softintr_dispatch() at softintr_dispatch+0x8b
> end of kernel
> end trace frame: 0x72defae4a00, count: -8

This looks like a race between the asynchronous callback and the device
being detached.

The problem is that the driver already freed its memory when the
transfer completed.  By checking if the device is dying before calling
the callback we should prevent such crash.

Could you at least confirm that the diff below does not introduce any
regression?

Index: uhidev.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.73
diff -u -p -r1.73 uhidev.c
--- uhidev.c    9 Jan 2016 04:14:42 -0000       1.73
+++ uhidev.c    7 Jun 2016 15:21:15 -0000
@@ -96,8 +96,7 @@ void uhidev_attach(struct device *, stru
 int uhidev_detach(struct device *, int);
 int uhidev_activate(struct device *, int);
 
-void uhidev_get_report_async_cb(struct usbd_xfer *xfer, void *priv,
-    usbd_status status);
+void uhidev_get_report_async_cb(struct usbd_xfer *, void *, usbd_status);
 
 struct cfdriver uhidev_cd = {
        NULL, "uhidev", DV_DULL
@@ -754,17 +753,19 @@ uhidev_get_report_async_cb(struct usbd_x
        char *buf;
        int len = -1;
 
-       if (err == USBD_NORMAL_COMPLETION || err == USBD_SHORT_XFER) {
-               len = xfer->actlen;
-               buf = KERNADDR(&xfer->dmabuf, 0);
-               if (info->id > 0) {
-                       len--;
-                       memcpy(info->data, buf + 1, len);
-               } else {
-                       memcpy(info->data, buf, len);
+       if (!usbd_is_dying(xfer->pipe->device)) {
+               if (err == USBD_NORMAL_COMPLETION || err == USBD_SHORT_XFER) {
+                       len = xfer->actlen;
+                       buf = KERNADDR(&xfer->dmabuf, 0);
+                       if (info->id > 0) {
+                               len--;
+                               memcpy(info->data, buf + 1, len);
+                       } else {
+                               memcpy(info->data, buf, len);
+                       }
                }
+               info->callback(info->priv, info->id, info->data, len);
        }
-       info->callback(info->priv, info->id, info->data, len);
        free(info, M_TEMP, sizeof(*info));
        usbd_free_xfer(xfer);
 }