> -----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? -------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. -- 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
