On Wed, 2 Mar 2016, Dobrowolski, Robert wrote: > > -----Original Message----- > > From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > > Sent: Monday, 29 February, 2016 16:30 > > To: Dobrowolski, Robert <robert.dobrowolski@xxxxxxxxx> > > Cc: linux-usb@xxxxxxxxxxxxxxx; Andruszak, Adam <adam.andruszak@xxxxxxxxx>; > > Nyman, Mathias <mathias.nyman@xxxxxxxxx> > > Subject: Re: [PATCH v1] usb: hcd: out of bounds access in for_each_companion > > > > On Mon, 29 Feb 2016, Robert Dobrowolski wrote: > > > > > On BXT platform Host Controller and Device Controller figure as same > > > PCI device but with different device function. HCD should not pass > > > data to Device Controller. Checking if companion device is Device > > > Controller and omitting it. > > > > > > Signed-off-by: Robert Dobrowolski <robert.dobrowolski@xxxxxxxxx> > > > --- > > > drivers/usb/core/hcd-pci.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c > > > index 9eb1cff..ae977d7 100644 > > > --- a/drivers/usb/core/hcd-pci.c > > > +++ b/drivers/usb/core/hcd-pci.c > > > @@ -74,6 +74,16 @@ static void for_each_companion(struct pci_dev *pdev, > > struct usb_hcd *hcd, > > > if (companion->bus != pdev->bus || > > > PCI_SLOT(companion->devfn) != slot) > > > continue; > > > + > > > + /* > > > + * On certain platforms Host Controller and Device Controller > > > + * figure as same device with different device function. HCD > > > + * should not pass data to Device Controller. Checking if > > > + * companion device is Device Controller and omitting it. > > > + */ > > > + if (companion->class == ((PCI_CLASS_SERIAL_USB << 8) | 0xfe)) > > > + continue; > > > + > > > companion_hcd = pci_get_drvdata(companion); > > > if (!companion_hcd || !companion_hcd->self.root_hub) > > > continue; > > > > This cries out for a #define. > > > > Besides, wouldn't it be safer to check that the class _is_ equal to CL_UHCI, > > CL_OHCI, or CL_EHCI instead of checking what the class _isn't_ equal to? What > > if somebody puts a non-USB function in the same slot? > > > > Alan Stern > > Hi, > PCI_CLASS_SERIAL_USB_XDCI is not yet defined in PCI IDs. I could define it in this patch > but there is already code using PCI_CLASS_SERIAL_USB << 8) | 0xfe somewhere else. > So I was thinking I could use this here, and then do a proper #define in another patch and > change all other instances. > > I think you are right with the OHCI and other. All host controllers are defined in pci_id > However, in case some new definition show up in the future, companion device could be > Checked if it is host controller (0x0c03) and then checked if its not device controller (0c03fe) > Something like: > if (((companion->class >> 8) != PCI_CLASS_SERIAL_USB) || > companion->class == ((PCI_CLASS_SERIAL_USB << 8) | 0xfe)) > continue; > > Could that do? No. No new companion controllers will be defined in the future (they have been obsolete for quite a few years). You should check for UHCI, OHCI, or EHCI. If the device isn't one of those three then we don't care whether it's a device controller or anything else. Alan Stern -- 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
