linux-kernel - Re: Regression with commit f9cde5f in 2.6.30-gitX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090624163310.GD7239@us.ibm.com>
Date:	Wed, 24 Jun 2009 09:33:10 -0700
From:	Gary Hade <garyhade@...ibm.com>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
Cc:	Gary Hade <garyhade@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Larry Finger <Larry.Finger@...inger.net>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	x86 maintainers <x86@...nel.org>, Len Brown <lenb@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: Regression with commit f9cde5f in 2.6.30-gitX

On Wed, Jun 24, 2009 at 09:45:00PM +0530, Jaswinder Singh Rajput wrote:
> On Wed, 2009-06-24 at 08:56 -0700, Gary Hade wrote:
> > On Wed, Jun 24, 2009 at 04:51:48PM +0200, Thomas Gleixner wrote:
> > > On Wed, 24 Jun 2009, Jaswinder Singh Rajput wrote:
> > > > Reported-by: Larry Finger <Larry.Finger@...inger.net>
> > > > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@...il.com>
> > > > ---
> > > >  arch/x86/pci/acpi.c |   16 ++++++++++------
> > > >  1 files changed, 10 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> > > > index 16c3fda..0bc015f 100644
> > > > --- a/arch/x86/pci/acpi.c
> > > > +++ b/arch/x86/pci/acpi.c
> > > > @@ -69,6 +69,9 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
> > > >  	struct resource *root;
> > > >  	int max_root_bus_resources = PCI_BUS_NUM_RESOURCES;
> > > >  
> > > > +	if (info->res_num >= PCI_BUS_NUM_RESOURCES)
> > > > +		return AE_OK;
> > > > +
> > > >  	status = resource_to_addr(acpi_res, &addr);
> > > >  	if (!ACPI_SUCCESS(status))
> > > >  		return AE_OK;
> > > > @@ -94,17 +97,18 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
> > > >  	if (bus_has_transparent_bridge(info->bus))
> > > >  		max_root_bus_resources -= 3;
> > > >  	if (info->res_num >= max_root_bus_resources) {
> > > > -		printk(KERN_WARNING "PCI: Failed to allocate 0x%lx-0x%lx "
> > > > -			"from %s for %s due to _CRS returning more than "
> > > > -			"%d resource descriptors\n", (unsigned long) res->start,
> > > > -			(unsigned long) res->end, root->name, info->name,
> > > > -			max_root_bus_resources);
> > > > +		pr_warning("PCI: Failed to allocate 0x%lx-0x%lx "
> > > > +			   "from %s for %s due to _CRS returning more than "
> > > > +			   "%d resource descriptors\n",
> > > > +			   (unsigned long)res->start, (unsigned long)res->end,
> > > > +			   root->name, info->name, max_root_bus_resources);
> > > 
> > > Can you please avoid mixing cleanup patches with bug fixes ? I almost
> > > did not see the line below.
> > > 
> > > > +		info->bus->resource[info->res_num] = res;
> > > 
> > > Storing the resource in defeats the purpose of the original patch,
> > > which makes sure that we do _NOT_ use the last 3 slots of the resource
> > > array for a root bus with a transparent bridge.
> > > 
> > > >  		info->res_num++;
> > > 
> > > This is the real bug. info->res_num should not be incremented when we
> > > run out of resources already.
> > 
> > This probably isn't needed but I don't think it is the cause of
> > the problem.  It is just counting the number of _CRS returned
> > resources and I believe subsequent visits to setup_resource()
> > for additional returned resources will generate the same warning
> > as it also would if the instruction were removed and no longer
> > being used to keep an accurate count.
> > 
> 
> Yes, this count is needed to avoid PCI failure.

With your proposed change that allows the last three slots
of the resource array to be populated, this is true.  Without
your change I believe removal of this instriction is benign.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@...ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/