RE: [PATCH] [pci] Fix pointer dereference before call to pcie_bus_configure_settings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Jon Mason [mailto:mason@xxxxxxxx]
> Sent: Wednesday, August 31, 2011 11:18 AM
> To: Iyer, Shyam
> Cc: shyam.iyer.t@xxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> jbarnes@xxxxxxxxxxxxxxxx; sgruszka@xxxxxxxxxx
> Subject: Re: [PATCH] [pci] Fix pointer dereference before call to
> pcie_bus_configure_settings
> 
> On Wed, Aug 31, 2011 at 10:08 AM,  <Shyam_Iyer@xxxxxxxx> wrote:
> >
> >
> >> -----Original Message-----
> >> From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci-
> >> owner@xxxxxxxxxxxxxxx] On Behalf Of Jon Mason
> >> Sent: Wednesday, August 31, 2011 11:07 AM
> >> To: Shyam Iyer
> >> Cc: linux-pci@xxxxxxxxxxxxxxx; sgruzka@xxxxxxxxxx;
> >> jbarnes@xxxxxxxxxxxxxxxx; Iyer, Shyam
> >> Subject: Re: [PATCH] [pci] Fix pointer dereference before call to
> >> pcie_bus_configure_settings
> >>
> >> On Tue, Aug 30, 2011 at 11:58 PM, Shyam Iyer
> <shyam.iyer.t@xxxxxxxxx>
> >> wrote:
> >> > Commit b03e7495a862b028294f59fc87286d6d78ee7fa1 introduces a few
> >> issues by dereferencing bus->self in the call to
> >> pcie_bus_configure_settings.
> >> >
> >> > This fixes it by checking existence of bus->self before
> dereferencing
> >> it.
> >> >
> >> > Reported-by: Stanislaw Gruszka<sgruszka@xxxxxxxxxx>
> >> > Signed-off-by: Shyam Iyer <shyam_iyer@xxxxxxxx>
> >> > ---
> >> >  arch/x86/pci/acpi.c              |    8 ++++++--
> >> >  drivers/pci/hotplug/pcihp_slot.c |    3 ++-
> >> >  drivers/pci/probe.c              |    3 ---
> >> >  3 files changed, 8 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> >> > index c953302..a11b673 100644
> >> > --- a/arch/x86/pci/acpi.c
> >> > +++ b/arch/x86/pci/acpi.c
> >> > @@ -365,8 +365,12 @@ struct pci_bus * __devinit
> >> pci_acpi_scan_root(struct acpi_pci_root *root)
> >> >         */
> >> >        if (bus) {
> >> >                struct pci_bus *child;
> >> > -               list_for_each_entry(child, &bus->children, node)
> >> > -                       pcie_bus_configure_settings(child, child-
> >> >self->pcie_mpss);
> >> > +               list_for_each_entry(child, &bus->children, node) {
> >> > +                       if (child->self)
> >> > +                               pcie_bus_configure_settings(child,
> >> > +                                                           child-
> >> >self->\
> >> > +
> >> pcie_mpss);
> >>
> >> Put this on one line.  The rest looks fine to me.
> >
> > Hmm.. that takes it above the 80 char limit and checkpatch.pl
> complains..
> >
> > I guess I can make it a single line unless you have other ideas to
> arrange it..
> 
> I'd do it like:
> 
>         if (bus) {
>                 struct pci_bus *child;
>                 list_for_each_entry(child, &bus->children, node) {
>                         struct pci_bus *self = child->self;
>                         if (!self)
>                                 continue;
> 
>                         pcie_bus_configure_settings(child, self-
> >pcie_mpss);
>                 }
>         }

Yep..
That should work. Will resubmit.. Thanks!

> 
> >
> >>
> >> > +               }
> >> >        }
> >> >
> >> >        if (!bus)
> >> > diff --git a/drivers/pci/hotplug/pcihp_slot.c
> >> b/drivers/pci/hotplug/pcihp_slot.c
> >> > index 753b21a..ef5596d 100644
> >> > --- a/drivers/pci/hotplug/pcihp_slot.c
> >> > +++ b/drivers/pci/hotplug/pcihp_slot.c
> >> > @@ -169,7 +169,8 @@ void pci_configure_slot(struct pci_dev *dev)
> >> >                        (dev->class >> 8) ==
> PCI_CLASS_BRIDGE_PCI)))
> >> >                return;
> >> >
> >> > -       pcie_bus_configure_settings(dev->bus, dev->bus->self-
> >> >pcie_mpss);
> >> > +       if (dev->bus && dev->bus->self)
> >> > +               pcie_bus_configure_settings(dev->bus, dev->bus-
> >self-
> >> >pcie_mpss);
> >> >
> >> >        memset(&hpp, 0, sizeof(hpp));
> >> >        ret = pci_get_hp_params(dev, &hpp);
> >> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> >> > index 8473727..0820fc1 100644
> >> > --- a/drivers/pci/probe.c
> >> > +++ b/drivers/pci/probe.c
> >> > @@ -1456,9 +1456,6 @@ void pcie_bus_configure_settings(struct
> pci_bus
> >> *bus, u8 mpss)
> >> >  {
> >> >        u8 smpss = mpss;
> >> >
> >> > -       if (!bus->self)
> >> > -               return;
> >> > -
> >> >        if (!pci_is_pcie(bus->self))
> >> >                return;
> >> >
> >> > --
> >> > 1.7.1
> >> >
> >> >
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-pci"
> in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux