On Tue, 03 Oct 2017 16:21:57 +0200, Alan Stern wrote: > > On Tue, 3 Oct 2017, Takashi Iwai wrote: > > > On Mon, 25 Sep 2017 14:39:51 +0200, > > Andrey Konovalov wrote: > > > > > > Hi! > > > > > > I've got the following report while fuzzing the kernel with syzkaller. > > > > > > On commit e19b205be43d11bff638cad4487008c48d21c103 (4.14-rc2). > > > > > > It seems that there's no check of the endpoint type. > > > > > > usb 1-1: BOGUS urb xfer, pipe 1 != type 3 > > > ------------[ cut here ]------------ > > > WARNING: CPU: 0 PID: 1846 at drivers/usb/core/urb.c:449 > > > > How is this bug triggered? As it's syzkaller with QEMU, it looks > > hitting an inconsistent state the driver didn't expect (it sets the > > fixed endpoint), then USB-core detects the inconsistency and spews the > > kernel warning with stack trace. If so, it's no serious problem as it > > appears. > > > > Suppose my guess is right, I'm not sure what's the best way to fix > > this. Certainly we can add more sanity check in the caller side. > > OTOH, I find the reaction of USB core too aggressive, it's not > > necessary to be dev_WARN() but a normal dev_err(). > > Or I might be looking at a wrong place? > > It's a dev_WARN because it indicates a potentially serious error in the > driver: The driver has submitted an interrupt URB to a bulk endpoint. > That may not sound bad, but the same check gets triggered if a driver > submits a bulk URB to an isochronous endpoint, or any other invalid > combination. > > Most likely the explanation here is that the driver doesn't bother to > check the endpoint type because it expects the endpoint will always be > interrupt. But that is not a safe strategy. USB devices and their > firmware should not be trusted unnecessarily. > > The best fix is, like you said, to add a sanity check in the caller. OK, but then do we have some handy helper for the check? As other bug reports by syzkaller suggest, there are a few other drivers that do the same, submitting a urb with naive assumption of the fixed EP for specific devices. In the end we'll need to put the very same checks there in multiple places. thanks, Takashi -- 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
