On Thu, 2023-07-13 at 07:34 +0200, Greg Kroah-Hartman wrote: > I wasn't trying to be glib here, sorry if it came across that way. I'll > blame the heat... No worries. > > All we said is that your statement of "RNDIS is fundamentally unfixable" > > doesn't make a lot of sense. If this were the case, all USB drivers > > would have to "trust the other side" as well, right? > > No, well, yes. See the zillion patches we have had to apply to the > kernel over the years when someone decided that "usb devices are not to > be trusted" that syzbot has helped find :) Sure, I'm well aware of that. But that's also exactly my point - nowhere has anyone previously suggested that the protocol for any of those devices is fundamentally broken and the drivers should be removed. We've fixed those things and moved on. I can even understand the initial reaction of "oh hey this ancient thing is probably not used any more, let's just remove it", but even that's a different reasoning, along the lines of "this has bugs and nobody needs it". Though that nobody uses it has in fact been proven wrong, which is pretty much why we're have this discussion at all. > It's not a DMA issue here, it's a "the protocol allows for buffer > overflows and does not seem to be able to be verified to prevent this" > from what I remember (it's been a year since I looked at this last, > details are hazy.) If you s/be able to be verified/be verified in the code/ I entirely believe it, in fact I think it's quite likely given the age of the code and all. It's just that not being _able_ to verify it seems questionable to me (and you haven't given any reasons), given that it's USB and you always have a full buffer in hand when processing it, at a time where the device can no longer modify it (IOW no TOCTTOU issues either.) (As an aside, I've wondered about TOCTTOU with PCI, given that IOMMUs can and will do lazy unmap ... but that's a different discussion.) > At the time, I didn't see a way that it could be > fixed, hence this patch. Yeah I mean, the code isn't great, even if it's not _that_ much, but all the likely() and things in there don't make it easy to read, and the buffer size handling seems not immediately clear to me. So I probably couldn't fix it quickly either, though I haven't even seen the reports. Maciej seems to think it's fixable, at least. And yeah, we'd want to actually review/audit that, I suppose. So if you'd have said something like Let's disable the RNDIS driver(s) because there are known exploits there, nobody really knows how to fix this, and we need a short-term solution until the issues are public and somebody steps up to fix and maintain it. I'd have much less of a problem with that. That's not _great_, but at least it's honest and realistic. That could give us some time and maybe then we can get the bug reports public once it's no longer an immediate threat for all kernels, and go about fixing it with more time, maybe eventually backporting fixes and reverting the disablement etc. I guess this is why secret bug reports suck so much :-) Thanks, johannes
