Bug 508283 - libselinux initialization vs. /proc/mounts
libselinux initialization vs. /proc/mounts
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: libselinux (Show other bugs)
rawhide
All Linux
low Severity high
: ---
: ---
Assigned To: Daniel Walsh
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-26 08:08 EDT by Karel Zak
Modified: 2009-07-14 11:30 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-14 11:30:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Karel Zak 2009-06-26 08:08:45 EDT
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 08:25:48 EDT
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 11:09:12 EDT
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 11:30:29 EDT
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.