all: introduce syz_usb_connect_scripted to improve usb driver fuzzing #6372
Conversation
Add a new pseudo-syscall syz_usb_connect_scripted in order to increase coverage in usb drivers that require processing of driver-specific control requests between drivers and devices. This is another step in addressing open problems partially discussed in issue [1]. The idea is to describe variants of this call in syzlang for drivers that lack in coverage to all but ensure a successful probe. The main difference from generic syz_usb_connect is that another argument is added to the list, specifically, responses to CTRL requests often utilized by syz_usb_control_io pseudo-call in the form of vusb_responses_<driver> struct. That way syzkaller knows how to react to such calls early instead of relying on a mere lucky call to syz_usb_control_io during active fuzzing. Moreover, some requests deemed important enough (for instance, those that are necessary to be processed in order to properly finish probing) can be affixed with a number of times they should be dealt with by syzkaller before finishing device setup and configuraion. This is quite useful for dealing with drivers that send/receive multiple similar requests, or for those cases when a request comes even before SET_CONFIGURATION directive is issued. Note: as stated before, this is a transitionary change on the way to implementing a pseudo-call that takes into account all possible usb driver quirks, of which are many. [1] google#6206
Update select seeds that have broken due to changes to vusb_responses struct. This is caused by introduction of a new field 'expected' to struct vusb_response. This field holds the number of times syzkaller is required to process a specific request before finishing syz_usb_connect_scripted and, in turn, device setup and configuration. No changes are made to the way generic syz_usb_connect works.
Add descriptions for variants of generic syz_usb_connect and new syz_usb_connect_scripted pseudo-syscalls to target 3 underperforming (coverage-wise) usb drivers. Generic examples are not particularly effective here but keep them nonetheless for finding unorthodox error paths. Specifically, concentrate on these modules: - drivers/net/usb/r8152.c Interestingly, there are CTRL-requests before *and* after SET_CONFIGURATION directive. - drivers/usb/misc/legousbtower.c Basic example of a driver that needs help to pass early interface and endpoint checks, as well as a mandatory control request. - drivers/comedi/drivers/usbdux.c Of particular interest is a mandatory standard USB_REQ_SET_INTERFACE request. For now, deal with it in a new syz_usb_connect_scripted_impl helper function. Requires firmware from linux-firmware package. Also, add seeds for these descriptions. Focus on XXX_scripted calls as only they are capable of properly broadening code coverage in these cases.
Multiple usb drivers (among others) are missing firmware thus
stopping short any probe() attempts.
A couple examples are:
1) comedi usb drivers: usbdux, usbduxfast etc.
2) drivers/media/usb/{s2255,go7007}
and a few others at the very least.
Add the required module 'firmware-linux' (carries both free and
nonfree batches of built firmware) to the list of default packages.
This is not an ideal solution considering the goal of a *minimal*
distribution. Still, this will increase code coverage.
|
Thank you for working on this @fellair! Keeping However, please first see the comment I just left in #6206 about a different pseudo-syscall design idea. Sorry about not thinking of that approach before. But I like it considerably more than extending If you think that other approach makes sense and have the energy to rework this pull request — please do. Otherwise, since you already wrote the code and the syzlang description to implement |
|
Per conversation in #6206 (comment), I'll rework this change to adopt @xairy's suggestion over current version of syz_usb_connect_scripted. I think, if no one objects, I'll do the transition in this PR instead of creating a new one. |
2 participants
CC: @xairy
This change is another step closer to addressing open issues with current state of usb driver fuzzing. Some problems were voiced in issue #6206, as well as PR: #6354. There are a few new ones, I'll update the issue soon.
In short, use a modified copy of a generic
syz_usb_connectthat takes another argument - a list of responses to usb control requests in a form of already existingvusb_responsesstruct. That way syzkaller knows how to react to non-standard CTRL requests (provided there are necessary descriptions of said requests in syzlang) duringsyz_usb_connect_scripted. Also, we can specify the number of times each request should be processed to finish driverprobe()- syzkaller will keepsyz_usb_connect_scriptedrunning until all requests have been accounted for (or something breaks, timeout is hit etc.). Add a couple descriptions as an example, seeds as well.As this new syscall is just a stepping stone, I've settled on a few compromises:
syz_usb_connect_scriptedseparate: all new functionality may be merged into generic in the future, once all usb driver quirks are figured out.vusb_response.expectedholds the number of times a request needs to be processed. It is zero by default, does not affect classicsyz_usb_XXXcalls.syz_usb_connect_ath9kto_scriptedand update the docs. But I'll refer to maintainers on this.Apart from obvious additions, I've also:
syz_usb_XXX.linux-firmwarepackage intotools/create-image.shscript. This is important as multiple drivers are not fuzzed solely due to missing firmware binaries. Comedi usb drivers, such as usbdux, are part of this problem and are addressed by this series as well.