Created attachment 206706 [details]
kernel panic log
Kernel Panic is observed for NULL pointer dereference in FreeBSD
kernel driver code due to which kernel gets in panic then it has to reboot.
Actually, this vulnerability resides in lots of kernel drivers like
"uhub0", "ubt0", "umass0", "run0", "uhid0" etc., mostly usb devices.
Tested and observed the panic for following kernel drivers:
- umass (storage),
Devices which are using the structure
"usb_attach_arg" with device_get_ivars(9) as mentioned
"struct usb_attach_arg *uaa = device_get_ivars(dev)"
are prone to NULL pointer dereference bug as there is no check
for the same and the api device_get_ivars(9) is returning NULL.
device_get_ivars(9) from the file
"/usr/src/sys/kern/subr_bus.c" returns a NULL pointer, which get assigned
to *uaa structure object (function "uhub_probe" from file
then, after that there is a if-else condition which is checking the
usb_mode from that structure and there panic occurs due to dereferencing
the NULL pointer
Same valid for other kernel drivers.
There are still lots of drivers which are lacking this NULL pointer
dereference check, apart from what mentioned here.
[steps to reproduce]
* devctl disable uhub0
* devctl enable uhub0 <= BOOM - panic appears
Panic occurs here, after enabling the already disabled device (but only
with usb related drivers)
Root privilege is required.
Reproducibility is 100%
Attached file "panic.log"
Please find the attached patch file "patch.diff" for the file "usb_hub.c", "ng_ubt.c",
"if_run.c", "umass.c" and "uhid.c"
After applying the patch, it first returns the "ENXIO" as mentioned in the
patch code then later invocation returns "EBUSY" as device is enabled,
which can be verified by disabling it again.
Created attachment 206707 [details]
proposed patch diff for 5 drivers mentioned in the bug
This patch is not correct for USB. Please blacklist devctl from touching USB device nodes or anything that parents to a usbusX.
(In reply to Hans Petter Selasky from comment #2)
yeah you right. Actually, as per the discussion going in the mailing list, other developers said that the patch is correct *if and only if want to preserve the ivars information* and preserving the info would be a long term solution.
But, as you said that there might be a race, that devctl is
attaching/detaching drivers while USB is doing the same, it might go into the panic due to that.
So, you are correct on the point that USB drivers are not supposed to be managed outside the USB enumeration thread. Using devctl on USB driver is not supported. Only usbconfig is allowed to attach/detach USB devices.
And, I also think that it is a better solution to blacklist devctl from touching USB device nodes or anything that parents to a usbusX, instead of patching all the devices. As, we already have usbconfig to do the same.