linux-kernel - Re: Since sysfs_mount is static and used only in sysfs_init function, it could be just an automatic variable.

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: <20071202225320.GA23359@suse.de>
Date:	Sun, 2 Dec 2007 14:53:20 -0800
From:	Greg KH <gregkh@...e.de>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	rae l <crquan@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: Since sysfs_mount is static and used only in sysfs_init
	function, it could be just an automatic variable.

On Sun, Dec 02, 2007 at 03:22:46PM -0700, Eric W. Biederman wrote:
> Greg KH <gregkh@...e.de> writes:
> 
> > On Sun, Dec 02, 2007 at 02:52:17PM +0800, rae l wrote:
> >> On Dec 2, 2007 12:48 PM, Greg KH <gregkh@...e.de> wrote:
> >> ...
> >> > > and where is a detailed explaination on kern_mount? could someone give
> >> > > some comments or documentation pointers on this?
> >> >
> >> > See the patches that Eric Biederman just posted to lkml for why this
> >> > structure is a static pointer this way right now, it's in preparation
> >> > for future patches.
> >> I have checked commit 7d0c7d676cc066413e1583b5af9fba8011972d41 by Eric
> >> W. Biederman,
> >>
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d0c7d676cc066413e1583b5af9fba8011972d41
> >> 
> >> which just make sysfs_mount from externally visible to static that
> >> could be only used in one c file,
> >> 
> >> but I mean that the static variable is still on kernel bss section,
> >> this consumes a pointer (4 or 8 bytes) memory,
> >> 
> >> through a grep from fs/sysfs/, it appears that the variable
> >> sysfs_mount is only used in the sysfs_init function,
> >> 
> >> $ grep -RsInw sysfs_mount fs/sysfs/
> >> fs/sysfs/mount.c:25:static struct vfsmount *sysfs_mount;
> >> fs/sysfs/mount.c:101:           sysfs_mount = kern_mount(&sysfs_fs_type);
> >> fs/sysfs/mount.c:102:           if (IS_ERR(sysfs_mount)) {
> >> fs/sysfs/mount.c:104:                   err = PTR_ERR(sysfs_mount);
> >> fs/sysfs/mount.c:105:                   sysfs_mount = NULL;
> >> 
> >> we could mark this variable an automatic one, which scope is just in
> >> this function, thus created and destroyed with the stack,
> >> this approach does not consume a pointer on kernel bss section,
> >> 
> >> Why not do this?
> >
> > Again, see the patches he _just_ posted to lkml, the specific message
> > you are looking for is:
> > 	Message-ID: <m11wa693t0.fsf@...ederm.dsl.xmission.com>
> > 	Subject: [PATCH 01/10] sysfs: Make sysfs_mount static again.
> >
> > Also see the whole long thread for more details.
> 
> As long as you aren't talking about the subthread that spun off
> about GPL exports, and just the rest of my patches and showing
> where I am going that sounds reasonable.

Yes, that subthread really had nothing to do with your patch series :)

> > If you have further questions about this, please ask Eric.
> 
> Honestly I think there is a reasonable chance we could kill
> sysfs_mount and the kern_mount entirely.  We used to need  the
> internal kernel mount because of the coupling between sysfs_dirent
> and the directory dentries but that is gone now.

Hm, this does take into account the fact that we internally mount sysfs
as part of the boot process to determine the boot disk major:minor,
right?  As long as we don't break that, I'm happy.

thanks,

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/