[PATCH] libselinux init functions

[PATCH] libselinux init functions

[Xin Ouyang]

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

Hi all,

I am working on the newest release of SELinux userspace packages and
sysvinit, and find there are a defect for init function of libselinux.
  * libselinux-2.1.9
  * sysvinit-2.88dsf

AFAIK, while sysvinit is configured with SELinux, /sbin/init would be
linked to libselinux. So before /sbin/init running, init_selinuxmnt()
would be called.

// libselinux-2.1.9/src/init.c

static void init_lib(void) __attribute__ ((constructor));
static void init_lib(void)
{
        selinux_page_size = sysconf(_SC_PAGE_SIZE);
        init_selinuxmnt();
}

As called before /sbin/init, init_selinuxmnt() is called before any
other userspace process.
At this time, procfs(/proc) and selinuxfs(/sys/fs/selinux or /selinux)
have not been mounted, because kernel do not mount them.

So, after init_lib() -> init_selinuxmnt(), global variable
"selinux_mnt" is still null.

/sbin/init would call is_selinux_enabled() to check whether selinux
enabled. Because "selinux_mnt" is null, is_selinux_enabled() always
return 0.

// libselinux-2.1.9/src/enabled.c

int is_selinux_enabled(void)
{
        int enabled = 0;

        if (selinux_mnt) {
                enabled = 1;
                if (getcon_raw(&con) == 0) {
                        if (!strcmp(con, "kernel"))
                                enabled = 0;
                        freecon(con);
                }
        }

        return enabled;
}

Then, selinux_init_load_policy(&enforce) is never called, so SELinux
init failed. :(

// sysvinit-2.88dsf/src/init.c

#ifdef WITH_SELINUX
	if (getenv("SELINUX_INIT") == NULL) {
		const int rc = mount("proc", "/proc", "proc", 0, 0);
		if (is_selinux_enabled() > 0) {
			putenv("SELINUX_INIT=YES");
			...
			if (selinux_init_load_policy(&enforce) == 0) {
				execv(myname, argv);
			} else {
				if (enforce > 0) {
					....
					exit(1);
				}
			}
		}
		....
	}
#endif


My patch is mounting procfs before selinuxfs_exists(), which would
check whether selinux exist in /proc/filesystem.
And mounting selinuxfs mountpoint after selinuxfs_exists().

Then, the init_lib() flow should be:
init_lib()
 `-- if (selinux_mnt) return
 `-- mount proc fs
 `-- if (!selinuxfs_exists()) goto out
 `-- mount selinuxfs (/sys/fs/selinux or /selinux)
 `-- check selinuxfs mountpoint in /proc/mounts
 `-- verify_selinuxmnt()
       `-- set selinux_mnt

Any suggestion would be welcome. Thanks. :)

-- 
- Pascal

[-- Attachment #2: 0001-libselinux-Mount-proc-and-SELINUXFS.patch --]
[-- Type: text/x-patch, Size: 2093 bytes --]

From 4a36b34ea23e56814b56c043df6088caad6022d3 Mon Sep 17 00:00:00 2001
From: Xin Ouyang <Xin.Ouyang@windriver.com>
Date: Mon, 12 Mar 2012 18:05:16 +0800
Subject: [PATCH] libselinux: Mount /proc and SELINUXFS.

Since /proc and SELINUXFS are not mounted before init process's
first running, checking for filesystem type and mount point of
selinuxfs would always fail.
So mount them before checking to fix this.
---
 src/init.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/src/init.c b/src/init.c
index 00afde7..a4ce726 100644
--- a/src/init.c
+++ b/src/init.c
@@ -11,12 +11,17 @@
 #include <sys/vfs.h>
 #include <stdint.h>
 #include <limits.h>
+#include <sys/mount.h>
 
 #include "dso.h"
 #include "policy.h"
 #include "selinux_internal.h"
 #include "setrans_internal.h"
 
+#ifndef MNT_DETACH /* Missing in glibc2.7 */
+#  define MNT_DETACH 2
+#endif
+
 char *selinux_mnt = NULL;
 int selinux_page_size = 0;
 int obj_class_compat = 1;
@@ -86,6 +91,7 @@ static void init_selinuxmnt(void)
 	FILE *fp=NULL;
 	size_t len;
 	ssize_t num;
+	int rc = -1;
 
 	if (selinux_mnt)
 		return;
@@ -94,10 +100,22 @@ static void init_selinuxmnt(void)
 
 	if (verify_selinuxmnt(OLDSELINUXMNT) == 0) return;
 
+	rc = mount("none", "/proc", "proc", 0, 0);
+
 	/* Drop back to detecting it the long way. */
 	if (!selinuxfs_exists())
 		goto out;
 
+	/* Since SELINUXFS is not mounted before init process's first running, 
+	 * there would be no SELINUXFS item in /proc/mounts.
+	 * So mount it before checking /proc/mounts to fix this. */
+        if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, 0, 0) != 0 
+				&& errno != EBUSY)
+		goto out;
+        else if (mount(SELINUXFS, OLDSELINUXMNT, SELINUXFS, 0, 0) != 0 
+				&& errno != EBUSY)
+		goto out;
+
 	/* At this point, the usual spot doesn't have an selinuxfs so
 	 * we look around for it */
 	fp = fopen("/proc/mounts", "r");
@@ -126,6 +144,10 @@ static void init_selinuxmnt(void)
 
       out:
 	free(buf);
+
+	if (rc == 0)
+		umount2("/proc", MNT_DETACH);
+
 	if (fp)
 		fclose(fp);
 	return;
-- 
1.7.5.4

Re: [PATCH] libselinux init functions

[Stephen Smalley]

On Mon, 2012-03-12 at 18:17 +0800, Xin Ouyang wrote:
> Hi all,
> 
> I am working on the newest release of SELinux userspace packages and
> sysvinit, and find there are a defect for init function of libselinux.
>   * libselinux-2.1.9
>   * sysvinit-2.88dsf
> 
> AFAIK, while sysvinit is configured with SELinux, /sbin/init would be
> linked to libselinux. So before /sbin/init running, init_selinuxmnt()
> would be called.
> 
> // libselinux-2.1.9/src/init.c
> 
> static void init_lib(void) __attribute__ ((constructor));
> static void init_lib(void)
> {
>         selinux_page_size = sysconf(_SC_PAGE_SIZE);
>         init_selinuxmnt();
> }
> 
> As called before /sbin/init, init_selinuxmnt() is called before any
> other userspace process.
> At this time, procfs(/proc) and selinuxfs(/sys/fs/selinux or /selinux)
> have not been mounted, because kernel do not mount them.
> 
> So, after init_lib() -> init_selinuxmnt(), global variable
> "selinux_mnt" is still null.
> 
> /sbin/init would call is_selinux_enabled() to check whether selinux
> enabled. Because "selinux_mnt" is null, is_selinux_enabled() always
> return 0.

/sbin/init isn't supposed to call is_selinux_enabled() before calling
selinux_init_load_policy(), as is_selinux_enabled() will always return 0
if policy has not yet been loaded.  That has always been true.  Sounds
like you have an incorrect selinux patch for your sysvinit.

selinux_init_load_policy() internally tests whether SELinux is enabled
in the kernel (by trying to mount selinuxfs) and will correctly return
-1 with *enforce set to 0 so that the caller knows to proceed despite
the failure to load.

Certainly the Debian sysvinit did it correctly in the past, as did the
Fedora sysvinit (back when Fedora used sysvinit).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libselinux init functions

[Stephen Smalley]

On Mon, 2012-03-12 at 10:11 -0400, Stephen Smalley wrote:
> On Mon, 2012-03-12 at 18:17 +0800, Xin Ouyang wrote:
> > Hi all,
> > 
> > I am working on the newest release of SELinux userspace packages and
> > sysvinit, and find there are a defect for init function of libselinux.
> >   * libselinux-2.1.9
> >   * sysvinit-2.88dsf
> > 
> > AFAIK, while sysvinit is configured with SELinux, /sbin/init would be
> > linked to libselinux. So before /sbin/init running, init_selinuxmnt()
> > would be called.
> > 
> > // libselinux-2.1.9/src/init.c
> > 
> > static void init_lib(void) __attribute__ ((constructor));
> > static void init_lib(void)
> > {
> >         selinux_page_size = sysconf(_SC_PAGE_SIZE);
> >         init_selinuxmnt();
> > }
> > 
> > As called before /sbin/init, init_selinuxmnt() is called before any
> > other userspace process.
> > At this time, procfs(/proc) and selinuxfs(/sys/fs/selinux or /selinux)
> > have not been mounted, because kernel do not mount them.
> > 
> > So, after init_lib() -> init_selinuxmnt(), global variable
> > "selinux_mnt" is still null.
> > 
> > /sbin/init would call is_selinux_enabled() to check whether selinux
> > enabled. Because "selinux_mnt" is null, is_selinux_enabled() always
> > return 0.
> 
> /sbin/init isn't supposed to call is_selinux_enabled() before calling
> selinux_init_load_policy(), as is_selinux_enabled() will always return 0
> if policy has not yet been loaded.  That has always been true.  Sounds
> like you have an incorrect selinux patch for your sysvinit.
> 
> selinux_init_load_policy() internally tests whether SELinux is enabled
> in the kernel (by trying to mount selinuxfs) and will correctly return
> -1 with *enforce set to 0 so that the caller knows to proceed despite
> the failure to load.
> 
> Certainly the Debian sysvinit did it correctly in the past, as did the
> Fedora sysvinit (back when Fedora used sysvinit).

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=580272

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libselinux init functions

[Stephen Smalley]

On Mon, 2012-03-12 at 10:24 -0400, Stephen Smalley wrote:
> On Mon, 2012-03-12 at 10:11 -0400, Stephen Smalley wrote:
> > On Mon, 2012-03-12 at 18:17 +0800, Xin Ouyang wrote:
> > > Hi all,
> > > 
> > > I am working on the newest release of SELinux userspace packages and
> > > sysvinit, and find there are a defect for init function of libselinux.
> > >   * libselinux-2.1.9
> > >   * sysvinit-2.88dsf
> > > 
> > > AFAIK, while sysvinit is configured with SELinux, /sbin/init would be
> > > linked to libselinux. So before /sbin/init running, init_selinuxmnt()
> > > would be called.
> > > 
> > > // libselinux-2.1.9/src/init.c
> > > 
> > > static void init_lib(void) __attribute__ ((constructor));
> > > static void init_lib(void)
> > > {
> > >         selinux_page_size = sysconf(_SC_PAGE_SIZE);
> > >         init_selinuxmnt();
> > > }
> > > 
> > > As called before /sbin/init, init_selinuxmnt() is called before any
> > > other userspace process.
> > > At this time, procfs(/proc) and selinuxfs(/sys/fs/selinux or /selinux)
> > > have not been mounted, because kernel do not mount them.
> > > 
> > > So, after init_lib() -> init_selinuxmnt(), global variable
> > > "selinux_mnt" is still null.
> > > 
> > > /sbin/init would call is_selinux_enabled() to check whether selinux
> > > enabled. Because "selinux_mnt" is null, is_selinux_enabled() always
> > > return 0.
> > 
> > /sbin/init isn't supposed to call is_selinux_enabled() before calling
> > selinux_init_load_policy(), as is_selinux_enabled() will always return 0
> > if policy has not yet been loaded.  That has always been true.  Sounds
> > like you have an incorrect selinux patch for your sysvinit.
> > 
> > selinux_init_load_policy() internally tests whether SELinux is enabled
> > in the kernel (by trying to mount selinuxfs) and will correctly return
> > -1 with *enforce set to 0 so that the caller knows to proceed despite
> > the failure to load.
> > 
> > Certainly the Debian sysvinit did it correctly in the past, as did the
> > Fedora sysvinit (back when Fedora used sysvinit).
> 
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=580272

This seems to be fixed already in the upstream sysvinit,
http://svn.savannah.nongnu.org/viewvc/sysvinit/trunk/src/init.c?root=sysvinit&r1=72&r2=90

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libselinux init functions

[Xin Ouyang]

2012/3/12 Stephen Smalley <sds@tycho.nsa.gov>:
> On Mon, 2012-03-12 at 18:17 +0800, Xin Ouyang wrote:
>> Hi all,
>>
>> I am working on the newest release of SELinux userspace packages and
>> sysvinit, and find there are a defect for init function of libselinux.
>>   * libselinux-2.1.9
>>   * sysvinit-2.88dsf
>>
>> AFAIK, while sysvinit is configured with SELinux, /sbin/init would be
>> linked to libselinux. So before /sbin/init running, init_selinuxmnt()
>> would be called.
>>
>> // libselinux-2.1.9/src/init.c
>>
>> static void init_lib(void) __attribute__ ((constructor));
>> static void init_lib(void)
>> {
>>         selinux_page_size = sysconf(_SC_PAGE_SIZE);
>>         init_selinuxmnt();
>> }
>>
>> As called before /sbin/init, init_selinuxmnt() is called before any
>> other userspace process.
>> At this time, procfs(/proc) and selinuxfs(/sys/fs/selinux or /selinux)
>> have not been mounted, because kernel do not mount them.
>>
>> So, after init_lib() -> init_selinuxmnt(), global variable
>> "selinux_mnt" is still null.
>>
>> /sbin/init would call is_selinux_enabled() to check whether selinux
>> enabled. Because "selinux_mnt" is null, is_selinux_enabled() always
>> return 0.
>
> /sbin/init isn't supposed to call is_selinux_enabled() before calling
> selinux_init_load_policy(), as is_selinux_enabled() will always return 0
> if policy has not yet been loaded.  That has always been true.  Sounds
> like you have an incorrect selinux patch for your sysvinit.

So, you mean is_selinux_enabled() is not equal to the cmdline
"selinux=1" passed to kernel, it just equal to
"is_selinux_policy_loaded".

Sorry I got it wrong.

Therefore, selinux_init_load_policy() would be always be called at
/sbin/init's first running, to check if "selinux=1" passed to kernel
and load policy.

Thank you, Stephen. :)

- Pascal

>
> selinux_init_load_policy() internally tests whether SELinux is enabled
> in the kernel (by trying to mount selinuxfs)  and will correctly return
> -1 with *enforce set to 0 so that the caller knows to proceed despite
> the failure to load.
>
> Certainly the Debian sysvinit did it correctly in the past, as did the
> Fedora sysvinit (back when Fedora used sysvinit).
>
> --
> Stephen Smalley
> National Security Agency
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.