tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Bug in uhidev, and possible solution



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



Home | Main Index | Thread Index | Old Index