On 08/18/2016 01:25 PM, Felipe Balbi wrote: > 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. > Don't we need some "set feature" IOCTL as well? -- Binyamin Sharet, Cisco, STARE-C -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
