Bug 508283 - libselinux initialization vs. /proc/mounts
Summary: libselinux initialization vs. /proc/mounts
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: libselinux
Version: rawhide
Hardware: All
OS: Linux
low
high
Target Milestone: ---
Assignee: Daniel Walsh
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-06-26 12:08 UTC by Karel Zak
Modified: 2009-07-14 15:30 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-07-14 15:30:29 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

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


Note You need to log in before you can comment on or make changes to this bug.