Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Alan Stern]

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.

Alan Stern