On Friday, February 13, 2015, Martin Pieuchot <mpieuc...@nolizard.org>
wrote:
> 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?
> Partially but not enough. I had already figured out that I needed that
to solve the NULL pointer dereference. See my 2nd paragraph above.
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);
>
