| Message ID | 1314766715-27286-1-git-send-email-shyam_iyer@dell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Disregard this as it did not have stanislaw's email id. Please use the other one.. > -----Original Message----- > From: Shyam Iyer [mailto:shyam.iyer.t@gmail.com] > Sent: Wednesday, August 31, 2011 12:59 AM > To: linux-pci@vger.kernel.org > Cc: sgruzka@redhat.com; mason@myri.com; jbarnes@virtuousgeek.org; Iyer, > Shyam > Subject: [PATCH] [pci] Fix pointer dereference before call to > pcie_bus_configure_settings > > 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@redhat.com> > Signed-off-by: Shyam Iyer <shyam_iyer@dell.com> > --- > 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); > + } > } > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 30, 2011 at 11:58 PM, Shyam Iyer <shyam.iyer.t@gmail.com> 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@redhat.com> > Signed-off-by: Shyam Iyer <shyam_iyer@dell.com> > --- > 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. > + } > } > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- > owner@vger.kernel.org] On Behalf Of Jon Mason > Sent: Wednesday, August 31, 2011 11:07 AM > To: Shyam Iyer > Cc: linux-pci@vger.kernel.org; sgruzka@redhat.com; > jbarnes@virtuousgeek.org; 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@gmail.com> > 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@redhat.com> > > Signed-off-by: Shyam Iyer <shyam_iyer@dell.com> > > --- > > 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.. > > > + } > > } > > > > 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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 31, 2011 at 10:08 AM, <Shyam_Iyer@dell.com> wrote: > > >> -----Original Message----- >> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- >> owner@vger.kernel.org] On Behalf Of Jon Mason >> Sent: Wednesday, August 31, 2011 11:07 AM >> To: Shyam Iyer >> Cc: linux-pci@vger.kernel.org; sgruzka@redhat.com; >> jbarnes@virtuousgeek.org; 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@gmail.com> >> 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@redhat.com> >> > Signed-off-by: Shyam Iyer <shyam_iyer@dell.com> >> > --- >> > 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); } } > >> >> > + } >> > } >> > >> > 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@vger.kernel.org >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Jon Mason [mailto:mason@myri.com] > Sent: Wednesday, August 31, 2011 11:18 AM > To: Iyer, Shyam > Cc: shyam.iyer.t@gmail.com; linux-pci@vger.kernel.org; > jbarnes@virtuousgeek.org; sgruszka@redhat.com > 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@dell.com> wrote: > > > > > >> -----Original Message----- > >> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- > >> owner@vger.kernel.org] On Behalf Of Jon Mason > >> Sent: Wednesday, August 31, 2011 11:07 AM > >> To: Shyam Iyer > >> Cc: linux-pci@vger.kernel.org; sgruzka@redhat.com; > >> jbarnes@virtuousgeek.org; 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@gmail.com> > >> 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@redhat.com> > >> > Signed-off-by: Shyam Iyer <shyam_iyer@dell.com> > >> > --- > >> > 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@vger.kernel.org > >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); + } } 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;
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@redhat.com> Signed-off-by: Shyam Iyer <shyam_iyer@dell.com> --- 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(-)