On Fri, Apr 19, 2019 at 10:35 AM Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, Apr 17, 2019 at 06:33:39PM +0200, Andrey Konovalov wrote: > > Hi, > > > > As you might have noticed, syzbot has started reporting bugs in the > > USB subsystem that can be triggered externally by a malicious USB > > device. Right now the fuzzing is done via a GadgetFS-like interface to > > emulate USB devices through the USB Gadget subsystem and the Dummy > > HCD/UDC module to "loopback" connect those devices to the running > > kernel. There are more details in my OffensiveCon talk [1], [2]. > > > > A few questions/comments: > > > > 1. Which tree should we use for testing? > > > > Right now we're targeting the upstream tree, but we can switch to some > > USB development tree, where the fixes are likely to end up earlier. > > USB fixes land in the usb.git tree on git.kernel.org in the usb-testing > branch (for the next release), and in the usb-linus branch (for this > release). OK, I've switched to the usb-linus branch of the usb.git tree, since I see that all Alan's fixes went in that one. > > So you can just merge those two branches together if you like to be sure > you have all of the latest fixes. > > > 2. Is there an easy way to figure out which config options enable > > drivers reachable over USB? > > Looking for all options that depend on USB is a good start. > > > Right now our kernel config is based on one of the Debian kernel > > configs, that supposedly enables enough relevant USB drivers. At the > > same time it enables a lot of other unnecessary stuff, which makes the > > kernel big and long to compile. Ideally, we would to have a way to > > auto-generate a kernel config that enables all the relevant (enabled > > by at least one of the distros) USB drivers. I've looked at whether > > it's possible to figure out which particular options in some kernel > > config are related to USB, but it seems that neither the option names, > > nor the way they are grouped in the config file, are representative > > enough. > > Yeah, it's hard to just carve out this type of configuration, but here's > what I have done in the past to try to be sure I enabled all USB drivers > in my kernel configuration. > > First, start with a "minimally working configuration" by running: > make localmodconfig > on a working system, with the needed modules for booting and operating > properly already loaded. > > That gives you a .config file that should take only minutes to build, > compared to much longer for the normal distro configuration (also be > sure to disable some debugging options so you don't spend extra time > building and stripping symbols). > > Boot and make sure that configuration works. > > Then, take that .config and do: > - disable USB from the configuration by deleting the: > CONFIG_USB_SUPPORT=y > option from your .config > - run 'make oldconfig' to disable all USB drivers > - turn USB back on by setting CONFIG_USB_SUPPORT=y back on in > your .config > - run 'make oldconfig' and answer 'y' or 'm' to all of the > driver options you are presented with. > > That usually catches almost all of them. Sometimes you need to make > sure you have some other subsystem enabled (like SCSI), but odds are, if > you start with a "normally stripped down" configuration that works, you > should be fine. I suspect that make localmodconfig (+ switching CONFIG_USB_SUPPORT off and on) would likely include a lot of stuff that we don't need (there are many options that are =y, but not related to USB at all), but it definitely sounds better than what I have right now (converting almost all =m into =y). I'll give it a shot, thanks! > > > 3. Regarding that GadgetFS-like interface. > > > > Initially I was using GadgetFS (together with the Dummy HCD/UDC > > module) to perform emulation of USB devices for fuzzing, but later > > switched to a custom written interface. This interface is essentially > > implemented in the following patch [3]. An example that emulates a USB > > keyboard through this interface can be found here [4]. And the > > syzkaller parts responsible for USB fuzzing are here [5], [6]. The > > incentive to implement a different interface was to provide a somewhat > > raw and direct access to the USB Gadget layer for the userspace, where > > every USB request is passed to the userspace to get a response. > > > > The main differences between this interface (referred to as usb-fuzzer > > for now) and GadgetFS are: > > > > 1) GadgetFS does some sanity checks on the provided USB descriptors, > > which is something we don't want for fuzzing. We want the descriptors > > to be as corrupted as they can. > > > > 2) GadgetFS handles some of the USB requests internally based on the > > provided device descriptor, which is also something we don't want. For > > example we may want to be able to provide differently corrupted > > responses to the same request. > > > > 3) usb-fuzzer has ioctl-based interface instead of a filesystem-based > > one. I wouldn't say it's that big of a deal, but it makes it somewhat > > easier to incorporate into a fuzzer. > > > > 4) Somewhat related to the previous point: usb-fuzzer uses predictable > > endpoint names across different UDCs. > > > > Right now each UDC driver defines endpoint names via EP_INFO() as it > > pleases. And GadgetFS uses those names to create file entries for each > > of the endpoints. As a result, endpoint names for different UDCs will > > be different and it requires more work to write a portable userspace > > gadget. The usb-fuzzer interface auto selects and assigns an endpoint > > based on the required features like the transfer type. > > > > 5) GadgetFS binds to the first available UDC, usb-fuzzer provides a > > way to select a UDC to bind to. > > > > Since the fuzzing happens in multiple processes each of which has its > > own Dummy UDC assigned, we want to have control over which UDC we bind > > to. This part is a bit confusing, but what I found out is that a UDC > > is selected based on two different identifying names. I call the first > > one "udc_device_name" and the second one "udc_driver_name". > > "udc_device_name" has to be assigned to usb_gadget_driver->udc_name > > when usb_gadget_probe_driver() is called, and "udc_driver_name" is > > what we have to compare usb_gadget->name with inside of the > > usb_gadget_driver->bind() callback. For example, Dummy UDC has > > "dummy_udc" as its "udc_driver_name" and "dummy_udc.N" as its > > "udc_device_name". At the same time the dwc2 driver that is used on > > Raspberry Pi Zero, has "20980000.usb" as both "udc_driver_name" and > > "udc_device_name". > > > > Overall, the usb-fuzzer interface implementation has a similar > > structure to that of GadgetFS, but looks way simpler (although that > > might be because I've missed to implement some functionality :). > > > > We'd like to get this upstreamed, but I'm not sure whether this should > > be a separate interface (which we can rebrand as a raw usb gadget > > interface or something like that) or we should try to make it a > > special mode of GadgetFS. I like the former approach more, as GadgetFS > > looks quite complicated from my point of view and making fundamental > > changes to it doesn't seem like an easy task. This is where we'd like > > to get your input. > > I'll leave the gadgetfs questions to others that know that api better > than I do :) > > thanks, > > greg k-h
