After more testing, it seems that checking if the status is
USBD_INVAL is not really reliable - in fact there is a possibility of a crash.
Replacing the code to check if status is USBD_INVAL with this:
if(sc->sc_ipipe == NULL && sc->sc_opipe == NULL
&& sc->sc_oxfer == NULL)
return;
fixes the issue.
This code is safer (after all it checks for the values set by the
error handling code),
but there is possibly a cleaner way of doing it; unfortunately I do
not know the internals
of usbhid or how USB support is implemented in NetBSD beyond what I
needed to understand
in order to come with the solution to my issue.
In any case, I'm attaching a patch again, hopefully this is the last time :)
On 12/26/13, Just a Normal Person <tails92%gmail.com@localhost> wrote:
> My machine would crash once I exited any SDL application; I researched the
> issue
> and the uhidev code was the culprit.
>
> The crash happened after the free() call in the following part of
> uhidev_close(),
> in sys/dev/usb/uhidev.c:
>
> vvvvvvv
>
> if (sc->sc_ibuf != NULL) {
> free(sc->sc_ibuf, M_USBDEV); // <-- this is the call
> sc->sc_ibuf = NULL;
> }
>
> ^^^^^^^
>
> Now the reason for which this bug happens is because the libSDL code
> triggers
> a call to uhidev_open(), which fails.
> There would be no problem here, but in the error handling code
> after freeing the memory area pointed by sc->sc_ibuf, sc->sc_ibuf is
> not set to NULL;
> this makes free() try to free the memory pointed by sc->sc_ibuf again,
> which
> obviously leads to an error where free() gets wrong addresses for the
> start and end VM addresses,
> leading to the following assertion being fired at line 521 of
> uvm_km.c, in uvm_km_pgremove_intrsafe():
>
> KASSERT(start < end);
>
> To fix the issue, my proposed solution is:
>
> - setting sc->sc_ibuf to NULL in the error handling code in
> uhidev_open() after freeing the memory
> pointed by it
> - checking if the status is USBD_INVAL in uhidev_intr() at the very
> beginning, right after
> the variable declarations; if the status is USBD_INVAL the function
> does nothing.
> This is necessary otherwise the code in uhidev_intr() would be using
> the content of sc->sc_ibuf, now
> set to NULL after the error in uhidev_open(), causing a trap.
>
> I am running NetBSD amd64; the sources of the kernel I used is the
> December 24th snapshot of HEAD
> from nyftp.netbsd.org - should not be a problem considering the
> changes are small, and that
> sys/dev/usb/uhidev.c has not received an update since October.
> I enabled both the DIAGNOSTIC and DEBUG options in the kernel
> configuration.
>
> By the way, I am attaching a patch which can be used to patch
> sys/dev/usb/uhidev.c in the way
> I described.
>
Attachment:
uhidev.c.patch
Description: Binary data