> -----Original Message-----
> From: Alan Stern [mailto:[email protected]]
> Sent: Monday, 29 February, 2016 16:30
> To: Dobrowolski, Robert <[email protected]>
> Cc: [email protected]; Andruszak, Adam <[email protected]>;
> Nyman, Mathias <[email protected]>
> 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 <[email protected]>
> > ---
> >  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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to