On 13/02/15(Fri) 00:28, David Higgs wrote:
> I guess nobody else has tried calling uhidev_get_report_async() yet. :)
>
> First I was getting a NULL pointer deref in the uhidev async callback.
> Then I realized that due to USBD_NO_COPY, xfer->buffer was always
> NULL. Next, I tried to use the DMA buffer, but I ended up in DDB in a
> very cryptic way. I believe this is because the DMA buffer isn't
> available when the callback is invoked.
>
> For the async callback to get a valid dmabuf, it needs to be invoked
> prior to usb_freemem() in usbd_transfer_complete(). The xfer->status
> determination would need to move up too. I'd do this myself but I
> don't understand the logic and ordering of pipe->repeat stuff, and am
> concerned about unintentionally breaking other devices.
>
> This is partially my fault, because I "tested" the original diff that
> added the USBD_NO_COPY semantics to verify that it didn't break my
> synchronous code paths, but hadn't yet written anything for upd(4) to
> check the async ones.
Does the diff below help?
Index: uhidev.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.69
diff -u -p -r1.69 uhidev.c
--- uhidev.c 22 Jan 2015 10:27:47 -0000 1.69
+++ uhidev.c 13 Feb 2015 05:58:06 -0000
@@ -55,6 +55,7 @@
#include <dev/usb/usbdivar.h>
#include <dev/usb/hid.h>
#include <dev/usb/usb_quirks.h>
+#include <dev/usb/usb_mem.h>
#include <dev/usb/uhidev.h>
@@ -747,15 +748,17 @@ void
uhidev_get_report_async_cb(struct usbd_xfer *xfer, void *priv, usbd_status err)
{
struct uhidev_async_info *info = priv;
+ char *buffer;
int len = -1;
if (err == USBD_NORMAL_COMPLETION || err == USBD_SHORT_XFER) {
+ buffer = KERNADDR(&xfer->dmabuf, 0);
len = xfer->actlen;
if (info->id > 0) {
len--;
- memcpy(info->data, xfer->buffer + 1, len);
+ memcpy(info->data, buffer + 1, len);
} else {
- memcpy(info->data, xfer->buffer, len);
+ memcpy(info->data, buffer, len);
}
}
info->callback(info->priv, info->id, info->data, len);
