linux-kernel - Re: [RFC][PATCH v2 08/11] sysfs: introduce new interface sysfs_get

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: <20100519033541.GA25967@kroah.com>
Date:	Tue, 18 May 2010 20:35:41 -0700
From:	Greg KH <greg@...ah.com>
To:	Lin Ming <ming.m.lin@...el.com>
Cc:	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...e.hu>,
	Corey Ashford <cjashfor@...ux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Paul Mundt <lethal@...ux-sh.org>,
	"eranian@...il.com" <eranian@...il.com>,
	"Gary.Mohr@...l.com" <Gary.Mohr@...l.com>,
	"arjan@...ux.intel.com" <arjan@...ux.intel.com>,
	"Zhang, Yanmin" <yanmin_zhang@...ux.intel.com>,
	Paul Mackerras <paulus@...ba.org>,
	"David S. Miller" <davem@...emloft.net>,
	Russell King <rmk+kernel@....linux.org.uk>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Will Deacon <will.deacon@....com>,
	Maynard Johnson <mpjohn@...ibm.com>,
	Carl Love <carll@...ibm.com>,
	Kay Sievers <kay.sievers@...y.org>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH v2 08/11] sysfs: introduce new interface
	sysfs_get_kobject

On Wed, May 19, 2010 at 10:55:17AM +0800, Lin Ming wrote:
> On Wed, 2010-05-19 at 04:08 +0800, Greg KH wrote:
> > On Wed, May 19, 2010 at 01:48:43AM +0000, Lin Ming wrote:
> > > Need this interface in the later sysfs pmu lookup.
> > > 
> > > struct kobject *sysfs_get_kobject(struct file *file);
> > > Return the relative kobject of the sysfs file.
> > 
> > Ick, no.  Why would you ever have the file, yet not have the kobject
> > already?  Something is really wrong if this is needed.  Or strange.  Or
> > maybe both :)
> 
> Let me show you the scenario.
> 
> /sys/devices/system/cpu/event_source/
> `-- id
> 
> /sys/devices/system/cpu/events/
> |-- L1-dcache-load-misses
> |   |-- event_source -> ../../event_source
> 
> $perf top -e L1-dcache-load-misses
> 
> Lookup the pmu used to handle L1-dcache-load-misses as below,
> 
> 1. pmu_sys_fd =
> open("/sys/devices/system/cpu/events/L1-dcache-load-misses/event_source/id", ...)

You do that within the kernel?  or from userspace?

Either way, your id show callback will get called when you read or write
to that file, right?  Then you have your kobject.

> 2. pmu_sys_file = fget_light(pmu_sys_fd, ....)
> 
> 3. pmu_kobject = sysfs_get_kobject(pmu_sys_file)
> 
> 4. pmu_kobject is embedded in struct pmu, pmu = container_of(kobj,
> struct pmu, kobj);

Oh no.

You are just using the kobject sysfs tree to store your kobjects so you
can look them up again some time in the future from within the kernel?
Seriously?

What's wrong with a simple list of kobjects?  Or what the rest of the
kernel does (busses and devices and iterating over the devices for a
bus)?

Don't act like userspace here and try to use the sysfs filesystem layout
as a lookup into the kobject you are trying to find.  That's a
horrible abuse of sysfs.  One of the worse I have ever seen.  And I've
seen a lot of sysfs abuse over the years...

ick.

greg k-h
--
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/