Bug 508283

Summary:	libselinux initialization vs. /proc/mounts
Product:	[Fedora] Fedora	Reporter:	Karel Zak <kzak>
Component:	libselinux	Assignee:	Daniel Walsh <dwalsh>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	high	Docs Contact:
Priority:	low
Version:	rawhide	CC:	dwalsh, mgrepl, sdsmall, sgrubb
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2009-07-14 15:30:29 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Karel Zak 2009-06-26 12:08:45 UTC

Description of problem:

http://lkml.org/lkml/2009/6/24/460

The current libselinux initialization is broken by design. It's bad idea to use __attribute__((constructor)) and parse /proc/mounts there. 

It's bad to call anything before main() -- for example in mount(8) we need selinux stuff very rarely, but the util is always affected with selinux initialization. This thing has to be controlled by API. Really.

Don't forget that /proc/mounts could be a huge file and libselinux is pretty often used library in many many system utils.

Expected results:

 1/ add selinux_init() to the API rather than call anything before main().

or

 2/ simply require /selinux mountpoint on all selinux boxes and when you needn't /proc/mounts parsing. (you still depends on /proc, so why not on /selinux).

Comment 1 Stephen Smalley 2009-06-29 12:25:48 UTC

We can't require use of a selinux_init() call, as that will break existing userspace binaries.

Already committed a patch by Eric Paris that changes init_selinuxmnt() to check /proc/filesystems for selinuxfs first, and bail if not present.  So in the !selinux_enabled case, we won't read /proc/mounts and /proc/filesystems should be much smaller.  In the selinux_enabled case, we only read /proc/mounts until we reach the selinuxfs entry (which I'd expect would come early as it is mounted before initial policy load).

We could further change it to require /selinux mountpoint, but FWIW, it was originally Red Hat that insisted that we provide the ability to move the mountpoint without recompiling libselinux.  That however was when /sbin/init performed the initial mount of selinuxfs directly; with the initial mount logic moved into libselinux, it may not matter anymore.

There was also the lazy init patch by Steve Grubb using pthread_once, but it never got merged - maybe it should be revived.

Comment 2 Stephen Smalley 2009-07-14 15:09:12 UTC

Original bug fixed in libselinux 2.0.83 - will check /proc/filesystems for selinuxfs and bail if not present, so no reading of /proc/mounts if selinux disabled.  And when selinux is enabled, it first checks /selinux to see if it is mounted there and will only fall back to reading /proc/mounts if it is not.

lazy init patch merged in libselinux 2.0.85.  However, init_selinuxmnt is still done from constructor, since the functions that use selinux_mnt are not localized and it only requires stat'ing /selinux in the common case.  Only the reading of /etc/selinux/config and of /selinux/mls were changed to use lazy init.  We could possibly revisit this later, or would entertain patches from others.

Comment 3 Daniel Walsh 2009-07-14 15:30:29 UTC

Fixed in libselinux-2.0.85-1.FC12