linux-kernel - Re: [PATCH] mce: fix warning messages about static struct mce

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Pine.LNX.4.44L0.1201181047060.1343-100000@iolanthe.rowland.org>
Date:	Wed, 18 Jan 2012 10:51:49 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Greg KH <gregkh@...e.de>
cc:	Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	Sergei Trofimovich <slyich@...il.com>,
	<linux-kernel@...r.kernel.org>, Kay Sievers <kay.sievers@...y.org>,
	Linux PM mailing list <linux-pm@...r.kernel.org>,
	Tony Luck <tony.luck@...el.com>,
	Borislav Petkov <bp@...64.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	<prasad@...ux.vnet.ibm.com>, Ming Lei <tom.leiming@...il.com>,
	Djalal Harouni <tixxdz@...ndz.org>,
	Borislav Petkov <borislav.petkov@....com>,
	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>,
	Andi Kleen <ak@...ux.intel.com>,
	<gouders@...bocholt.fh-gelsenkirchen.de>,
	Marcos Souza <marcos.mage@...il.com>,
	<justinmattock@...il.com>, Jeff Chua <jeff.chua.linux@...il.com>
Subject: Re: [PATCH] mce: fix warning messages about static struct mce_device

On Wed, 18 Jan 2012, Greg KH wrote:

> > > > Minor nit, i don't think we have any other such [CONFIG_NR_CPUS] 
> > > > pattern in the kernel.
> > > > 
> > > > This should be something like:
> > > > 
> > > >   DECLARE_PER_CPU(struct device *, mce_device);
> > > 
> > > That is what we used to have, but with just a static struct 
> > > device. [...]

Ingo's suggestion is fine.  The difference is that instead of a static
array of struct device's, this is a static pre-cpu pointer to a dynamic 
struct device (effectively, an array of pointers rather than an array 
of structures).

> > Which was fine in itself for a per CPU data structure - wouldnt 
> > the warning be fixed by memset()-ing before registering the 
> > device or such, if device registry absolutely needs a pre-zeroed 
> > buffer?
> 
> It was already fixed that way, but the problem is that you can not have
> statically allocated 'struct device' objects in the system.  That's what
> my add-on patch fixed, also resolving the syslog messages saying there
> was no release function for the device as well.
> 
> > I still think there must be some bug/assumption lurking in the 
> > device layer - do you require all device allocations to be one 
> > via zalloc()? Seems like a weird and unrobust requirement.
> 
> Yes, that's always been the requirement.

There's an additional requirement: Device structures may not be reused.  
Not even if the caller clears all the fields to 0 in between.  That was
the real bug in the original code -- and adding a dummy release routine
wouldn't fix it.

> > Amongst other things we use PER_CPU to have an array of just 2 
> > elements on a dual core system, even if it boots a 
> > CONFIG_NR_CPUS=512 distro kernel. That saves RAM, and with 
> > higher CONFIG_NR_CPUS values it adds up quickly.
> > 
> > > > Or the pointer should be attached to the CPU info structure.
> > > 
> > > Ok, I have no objection to that, do you want me to make a 
> > > patch doing that, now that this is already in Linus's tree?
> > 
> > Would be nice if you could do that or some other equivalent 
> > solution, i'd really not like to see the [CONFIG_NR_CPUS] 
> > pattern to spread in the kernel, we spent a lot of time getting 
> > rid of such uses ;-)
> 
> Ok, I'll work on resolving this.

A static per-cpu pointer to struct device should work perfectly.

Alan Stern

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