![[BACK]](/icons/back.gif){kind=icon}
Up to [cvs.NetBSD.org] / src / sys / dev / usb
Request diff between arbitrary revisions
Keyword substitution: kv
Default branch: MAIN
vhci(4): Make vhci_usb_attach/detach return void. These never fail, so no need to return zero.
vhci(4): Don't fail with ENOBUFS if no intrxfer is set up. uhub(4) will set up the intrxfer and query the current state at its leisure -- no need to treat racing with it as a failure. (If there's some reason the caller needs to know about this state, then (a) there should be a comment explaining why, and (b) the assertion in vhci_fd_close needs to change.) Should fix a host of syzbot crashes that were all tripping over the same assertion but with different gobbledegook on the console -- here's all the ones I found in a quick skim of the front page: Reported-by: syzbot+58b183ac688d656e1bfd@syzkaller.appspotmail.com Reported-by: syzbot+e7b0e904184aa2c18224@syzkaller.appspotmail.com Reported-by: syzbot+476b25a0a3655f3565d6@syzkaller.appspotmail.com Reported-by: syzbot+e5b69892daf87a7464f2@syzkaller.appspotmail.com Reported-by: syzbot+db7f0bc71c33a488d0fc@syzkaller.appspotmail.com Reported-by: syzbot+71d0e82df292c56739da@syzkaller.appspotmail.com Reported-by: syzbot+dbfaad061b2c909d6332@syzkaller.appspotmail.com Reported-by: syzbot+d8b90cead59b887fee64@syzkaller.appspotmail.com Reported-by: syzbot+ea147adc4461acb9f491@syzkaller.appspotmail.com Reported-by: syzbot+cb7239776d4f51c39ca3@syzkaller.appspotmail.com Reported-by: syzbot+ffbae2dd4d4a0196b026@syzkaller.appspotmail.com Reported-by: syzbot+95d4852ea931f775cf35@syzkaller.appspotmail.com Reported-by: syzbot+3236a5e1bc356909b322@syzkaller.appspotmail.com Reported-by: syzbot+f5ac32d58eab38bce263@syzkaller.appspotmail.com Reported-by: syzbot+beb9643da72188117748@syzkaller.appspotmail.com Reported-by: syzbot+896191203695ba350566@syzkaller.appspotmail.com Reported-by: syzbot+7c175b48b2682cc329a5@syzkaller.appspotmail.com Reported-by: syzbot+caa5bc391d36d75335ea@syzkaller.appspotmail.com Reported-by: syzbot+9fe6d4c43fa10f9e4dfa@syzkaller.appspotmail.com Reported-by: syzbot+ae9ae663386e72d171b3@syzkaller.appspotmail.com Reported-by: syzbot+a0c3a5c2f7af91e44c17@syzkaller.appspotmail.com Reported-by: syzbot+3c157b017d0cafa7aea9@syzkaller.appspotmail.com Reported-by: syzbot+1e05efbbf2d7df821bfd@syzkaller.appspotmail.com Reported-by: syzbot+999f20b408f61e22f4e0@syzkaller.appspotmail.com Reported-by: syzbot+22d227370f78b3a34442@syzkaller.appspotmail.com Reported-by: syzbot+33760fa9b95349460293@syzkaller.appspotmail.com Reported-by: syzbot+75d865aafbc9ebadb0f6@syzkaller.appspotmail.com Reported-by: syzbot+3ddff5cb80bc0c9ac635@syzkaller.appspotmail.com Reported-by: syzbot+0f942570160d533d892d@syzkaller.appspotmail.com
usb: Hold pipe lock across upm_transfer and upm_start. This simplifies the code and fixes races with abort. Access to the pipe's queue is now done exclusively while the pipe is locked.
usb: Factor usb_insert_transfer out of upm_transfer and make private. Almost every upm_transfer function starts with: mutex_enter(&sc->sc_lock); err = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (err) return err; Some of them have debug messages sprinkled in here too, or assert that err == USBD_NORMAL_COMPLETION (alternative is USBD_IN_PROGRESS, only for pipes with up_running or up_serialise, presumably not applicable for these types of pipes). Some of them also assert xfer->ux_status == USBD_NOT_STARTED, which is guaranteed on entry and preserved by usb_insert_transer. Exceptions: - arch/mips/adm5120/dev/ahci.c ahci_device_isoc_transfer just returns USBD_NORMAL_COMPLETION, but I'm pretty sure this is and always has been broken anyway, so won't make anything worse (if anything, might make it better...) - external/bsd/dwc2/dwc2.c dwc2_device_bulk_transfer and dwc2_device_isoc_transfer _also_ issue dwc2_device_start(xfer) under the lock. This is probably a better way to do it, but let's do it uniformly across all HCIs at once. - rump/dev/lib/libugenhc/ugenhc.c rumpusb_device_bulk_transfer sometimes returns USBD_IN_PROGRESS _without_ queueing the transfer, in the !rump_threads case. Not really sure how this is supposed to work... If it actually breaks anything, we can figure it out.
sys: Fix various abuse of struct device internals. Will help to make struct device opaque later.
Merge thorpej-cfargs2.
Adapt to CFARGS().
Merge thorpej-cfargs branch: Simplify and make extensible the config_search() / config_found() / config_attach() interfaces: rather than having different variants for which arguments you want pass along, just have a single call that takes a variadic list of tag-value arguments. Adjust all call sites: - Simplify wherever possible; don't pass along arguments that aren't actually needed. - Don't be explicit about what interface attribute is attaching if the device only has one. (More simplification.) - Add a config_probe() function to be used in indirect configuiration situations, making is visibly easier to see when indirect config is in play, and allowing for future change in semantics. (As of now, this is just a wrapper around config_match(), but that is an implementation detail.) Remove unnecessary or redundant interface attributes where they're not needed. There are currently 5 "cfargs" defined: - CFARG_SUBMATCH (submatch function for direct config) - CFARG_SEARCH (search function for indirect config) - CFARG_IATTR (interface attribte) - CFARG_LOCATORS (locators array) - CFARG_DEVHANDLE (devhandle_t - wraps OFW, ACPI, etc. handles) ...and a sentinel value CFARG_EOL. Add some extra sanity checking to ensure that interface attributes aren't ambiguous. Use CFARG_DEVHANDLE in MI FDT, OFW, and ACPI code, and macppc and shark ports to associate those device handles with device_t instance. This will trickle trough to more places over time (need back-end for pre-OFW Sun OBP; any others?).
Give config_found() the same variadic arguments treatment as config_search(). This commit only adds the CFARG_EOL sentinel to the existing config_found() calls. Conversion of config_found_sm_loc() and config_found_ia() call sites will be in subsequent commits.
Register eight vHCI buses, and use separate KCOV mailboxes for them.
Add comments.
Increase the number of ports to 8.
Introduce KCOV remote support. This allows to collect KCOV coverage on threads other than curlwp, which is useful when fuzzing components that defer processing, such as the network stack (partially runs in softints) and the USB stack (partially runs in uhub kthreads). A subsystem that whishes to provide coverage for its threads creates a "mailbox" via kcov_remote_register() and gives it a (subsystem, id) identifier. There is one mailbox per "target lwp". The target lwp(s) must then call kcov_remote_enter() and kcov_remote_leave() with the identifier, to respectively enable and disable coverage within the thread. On the userland side, the fuzzer has access to the mailboxes on the system with the KCOV_IOC_REMOTE_ATTACH and KCOV_IOC_REMOTE_DETACH ioctls. When attached to a mailbox with a given identifier, the KCOV_IOC_ENABLE, KCOV_IOC_DISABLE and mmap() operations will affect the mailbox. As a demonstrator, the vHCI subsystem is changed to use KCOV mailboxes. When the vHCI bus attaches it creates as many mailboxes as it has USB ports, each mailbox being associated with a distinct port. Uhub is changed to enable KCOV coverage in usbd_new_device(). With that in place, all of the USB enumeration procedure can be traced with KCOV.
Mostly merge changes from HEAD upto 20200411
file vhci.c was added on branch phil-wifi on 2020-04-13 08:04:51 +0000
Publish the request/response structures too.
Put the ioctl definitions in a header, and install it.
Allow short transfers. We introduce a third packet, in the U->H list, that contains a vhci_response_t, which indicates the size.
store the request buffer in the vxfer instead of the packet, clearer
Remove the argument from USB_{ATTACH,DETACH}, for consistency.
Use a vhci_request_t, will be required for future changes.
Add internal support for multiple endpoints.
clarify and explain
revert the 0x% -> %# change for fixed width formats pointed out by uwe.
PR/55068: sc.dying: Fix printf formats: - no %s/%p for kernel log - 0x% -> %# - always %j for kernel log
Sync with head.
Fix mistakes in previous sloppy change with root intr xfers. - Make sure ux_status is set to USBD_IN_PROGRESS when started. Otherwise, if it is still in flight when we abort the pipe, usbd_ar_pipe will skip calling upm_abort. - Initialize ux_status under the lock; in principle a completion interrupt (or a delay) could race with the initialization. - KASSERT that the xfer is in progress when we're about to complete it. Candidate fix for PR kern/54963 for other HCI drivers than uhci. ok nick ok phone (This is the change that nick evidently MEANT to ok when he ok'd the previous one!)
Fix steady state of root intr xfers.
Why?
- Avoid completing a root intr xfer multiple times in races.
- Avoid potential use-after-free in poll_hub callouts (uhci, ahci).
How?
- Use sc->sc_intr_xfer or equivalent to store only a pending xfer
that has not yet completed -- whether successfully, by timeout, or
by synchronous abort. When any of those happens, set it to null
under the lock, so the xfer is completed only once.
- For hci drivers that use a callout to poll the root hub (uhci, ahci):
. Pass the softc pointer, not the xfer, to the callout, so the
callout is not even tempted to use xfer after free -- if the
callout fires, but the xfer is synchronously aborted before the
callout can do anything, the xfer might be freed by the time the
callout starts to examine it.
. Teach the callout to do nothing if it is callout_pending after it
has fired. This way:
1. completion or synchronous abort can just callout_stop
2. start can just callout_schedule
If the callout had already fired before (1), and doesn't acquire
the bus lock until after (2), it may be tempted to abort the new
root intr xfer just after submission, which would be wrong -- so
instead we just have the callout do nothing if it notices it has
been rescheduled, since it will fire again after the appropriate
time has elapsed.
Not a bug strictly speaking, but compute the address only after the length checks, for clarity and to appease kUBSan.
Improvements: - Don't process packets if the USB device is detached. Contrary to the other HCIs, vHCI has no timeout, so we never collect the pending packets, and must drop them synchronously. - Fix refcounting bug in vhci_device_ctrl_abort. - Implement vhci_activate. - Add a few KASSERTs.
Fixes: - Insert at the tail and not the head. I just noticed that the packets were in inverted order in the fifos when attaching a virtual urtw0. - Remove VHCI_DEBUG, which I mistakenly left enabled in rev1.
Add vHCI, a driver which allows to send and receive USB packets directly from userland via /dev/vhci. Using this, it becomes possible to test and fuzz the USB stack and all the USB drivers without having the associated hardware. The vHCI device has four ports independently addressable. For each xfer on each port, we create two packets: a setup packet (which indicates mostly the type of request) and a data packet (which contains the raw data). These packets are processed by read and write operations on /dev/vhci: userland poll-reads it to fetch usb_device_request_t structures, and dispatches the requests depending on bRequest and bmRequestType. A few ioctls are available: VHCI_IOC_GET_INFO - Get the current status VHCI_IOC_SET_PORT - Choose a vHCI port VHCI_IOC_USB_ATTACH - Attach a USB device on the current port VHCI_IOC_USB_DETACH - Detach the USB device on the current port vHCI has already allowed me to automatically find several bugs in the USB stack and its drivers.