Hi, Binyamin Sharet <bsharet@xxxxxxxxx> writes: >>>>>>> I get your point, what I propose is not to change the default behavior >>>>>>> of gadgetfs, >>>>>>> but allow it to enter to a special mode by the user. I am aware of the >>>>>>> issues that it >>>>>>> might raise, and understand your concerns. However, I am asking about >>>>>>> modifications in a specific, contained context. I would prefer to have it in the >>>>>>> mainline kernel, but if you don't think it fits - I will keep those >>>>>>> changes as an >>>>>>> out-of-tree module. >>>>>> you're gonna have a really hard time getting anything in mainline >>>>>> without explaining your goal. All you said is: "I wanna do fuzzying", >>>>>> but fuzzying of what exactly? Why do you need to fuzz during >>>>>> enumeration? This makes no sense. Gadgets are _not_ allowed to change >>>>>> their descriptors and kernel doesn't allow userspace to do that. If you >>>>>> wanna change your descriptor, then you _must_ disconnect from the host >>>>>> and write brand new descriptors during gadgetfs initialization. >>>>>> >>>>> Sorry if I wasn't clear enough. We maintain a tool called Umap2, its goal is >>>>> to perform security assessment of USB hosts. It does so in multiple ways, >>>>> and one of them is fuzzing the USB host by sending invalid/unexpected >>>>> packets over USB, including packets in the enumeration phase. There could >>>>> be multiple issues with USB host stack during enumeration, and the fuzzing >>>>> umap2 performs target those issues. >>>> have you found any bugs yet? >>>> >>>>> However, this kind of operation requires a very low level control over the >>>>> traffic, and until now it was done using Facedancer, which is a designated >>>>> HW. gadgetfs looked to me like the USB equivalent for "raw socket" as a >>>>> USB device, so I thought we could use it to enable umap2 on beaglebone >>>>> black and similar boards that run Linux instead of Facedancer. >>>>> >>>>> I hope this is clearer. If you think that there is some other kernel module >>>>> that is better suited to this task, I would be happy to hear about it. >>>> The kernel is not exactly meant for pentesting USB host stacks :-) >>>> >>>> Here's what we can do, for now keep an out-of-tree patch. If/when you >>>> find actual bugs, then we can reconsider adding this support if, and >>>> only if, it's really difficult to enable (for example, it should depend >>>> on EXPERT or something along those lines). We don't want this to ever >>>> leak to production systems, so we need to make it really annoying to get >>>> enabled. >>>> >>>> Anyway, first things first, let us know if/when you trigger any bugs >>>> with this. >>>> >>>> -- >>>> balbi >>> Many USB host implementations, including at least older versions of Linux, >>> have bugs in the enumeration phase. While I cannot pinpoint a ToC/ToU >>> vulnerability in the configuration descriptor at the moment, I found more than >>> a couple of issues with configuration descriptor parsing. I will post them here >>> soon, hopefully today. >>> >>> However, just over the last year multiple USB related CVEs in the Linux kernel >>> were published (not by me). >>> >>> Also, while there might not be a specific ToC/ToU bug in configuration >>> descriptor >>> parsing in Linux at the moment, there might still be in the future, or >>> in a different >>> operating system, or in a user application that queries those descriptor. >>> My goal is to test all those cases, not just the current Linux kernel. >> Fair enough, let's just wrap this "forward everything to userspace" with >> a Kconfig choice depending on EXPERT so that it doesn't leak to >> production kernel builds. >> >> Thanks >> > I think this will cause existing implementation over gadgetfs to fail > with this > special kernel (as now it will delegate everything all of the time). How > about > using a ioctl to configure it, but wrapping this ioctl with Kconfig? > This way > gadgetfs will operate as always unless a user activates "delegate all" > in runtime. > > Sounds reasonable? You mean a ep0 ioctl? Makes sense to me. Then we can add more features later. Perhaps just add a "Get supported features" IOCTL which returns a struct with 256 bits (4x uint64_t). I doubt we will ever need that many bits, but better safe than sorry, I guess. -- balbi
Attachment:
signature.asc
Description: PGP signature
