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]

Date:	Wed, 24 Jun 2009 18:17:18 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
cc:	Larry Finger <Larry.Finger@...inger.net>,
	Gary Hade <garyhade@...ibm.com>,
	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, 24 Jun 2009, Jaswinder Singh Rajput wrote:
> On Wed, 2009-06-24 at 16:51 +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;
> > > +
> 
> We need to test this otherwise we will get another oops.

Right, because you would store beyond the end of the array.
 
> > >  	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;
> > 
> 
> We need to set the resource even it is transparent bridge, otherwise pci
> will fail and system will not able to boot.

The fact that the system does boot with that patch is not making the
patch more correct. It fixes the boot problem at hand, but it does not
fix the root cause of the problem.

You stick the resource into the resource array, but it does not call
insert_resource(). Also you print a warning while the system seems to
be happy to use the device.

This is just inconsistent and the patch simply papers over the root
cause of the problem.

The reservation of the last 3 resources in the transparent case is
more likely the real reason as it reduces the number of available
slots in bus->resource[].

Can you please increase PCI_BUS_NUM_RESOURCES by at least 3 and test
with the one liner patch which removes the increment applied.

Also please provide the output of lspci on 2.6.30 (which does not have
f9cde5f applied).

Thanks,

	tglx

--
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/