-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 05/03/2011 11:33 AM, Stephen Smalley wrote: > On Tue, 2011-05-03 at 10:50 -0400, Daniel J Walsh wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> The Fedora Distribution is looking to standardize kernel subsystem file >> systems to be mounted under /sys/fs. They would like us to move /selinux >> to /sys/fs/selinux. This patch changes libselinux in the following ways: >> >> 1. load_policy will first check if /sys/fs/selinux exists and mount the >> selinuxfs at this location, if it does not exists it will fall back to >> mounting the file system at /selinux (if it exists). >> >> 2. The init functions of selinux will now check if /sys/fs/selinux is >> mounted, if it is and has an SELinuxfs mounted on it, the code will then >> check if the selinuxfs is mounted rw, if it is, libselinux will set the >> mountpoint, if it is readonly, libselinux will return no mountpoint. If >> /sys/fs/selinux does not exists, the same check will be done for >> /selinux and finally for an entry in /proc/mounts. >> >> NOTE: We added the check for RO, to allow tools like mock to be able to >> tell a chroot that SELinux is disabled while enforcing it outside the >> chroot. >> >> >> # getenforce >> Enabled >> # mount -t selinuxfs -o remount,ro selinuxfs /var/chroot/selinux > > Just to clarify, the right commands to use are: > mount --bind /selinux /var/chroot/selinux > mount -o remount,ro /var/chroot/selinux > > Do not use: > mount -t selinuxfs -o ro selinuxfs /var/chroot/selinux > as this will in fact change the flags on /selinux as well. Surprise! > Result of there only being a single instance (superblock) of selinuxfs, > although you can have multiple vfsmounts of it. > >> # chroot /var/chroot >> # getenforce >> Disabled >> >> 3. In order to make this work, I needed to stop enabled from checking if >> /proc/filesystem for entries if selinux_mnt did not exist. Now enabeled >> checks if selinux_mnt has been discovered otherwise it will report >> selinux disabled. > > Looks reasonable, minor comments below. > > Can we really not get all the necessary information from a single call > (as opposed to having to call both statfs() and statvfs())? Isn't > statvfs() implemented on Linux by calling the statfs system call? > Not that I can see. > I'd suggest adding a #define OLDSELINUXMNT "/selinux" to policy.h and > using OLDSELINUXMNT in init.c and load_policy.c rather than sprinkling > "/selinux" around multiple places. Wouldn't hurt to #define SELINUXFS > "selinuxfs" as well and replacing all occurrences in init.c and > load_policy.c. > Ok > As check_mountpoint() sets selinux_mnt, I'd pick a more descriptive > name. Actually, could you perhaps fold the logic into set_selinuxmnt()? > That would mean the validation would happen when set_selinuxmnt() gets > called by load_policy, which isn't strictly necessary but does no harm. > Done I have to change set_selinuxmnt to return an int now, though. Does this mean we would need an API version bump? Changing from void return to int? -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk3AJ28ACgkQrlYvE4MpobPMBwCghY08MsDjpufL/NPkFWfC7M6v 9kgAoI8Gi0Z0LROlxPYgtvcShmZkLEKb =4NO/ -----END PGP SIGNATURE-----
diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h
index f110dcf..644d6d2 100644
--- a/libselinux/include/selinux/selinux.h
+++ b/libselinux/include/selinux/selinux.h
@@ -513,7 +513,7 @@ extern int selinux_check_securetty_context(const security_context_t tty_context)
Normally, this is determined automatically during libselinux
initialization, but this is not always possible, e.g. for /sbin/init
which performs the initial mount of selinuxfs. */
-void set_selinuxmnt(char *mnt);
+int set_selinuxmnt(char *mnt);
/* clear selinuxmnt variable and free allocated memory */
void fini_selinuxmnt(void);
diff --git a/libselinux/src/enabled.c b/libselinux/src/enabled.c
index b3c8c47..018c787 100644
--- a/libselinux/src/enabled.c
+++ b/libselinux/src/enabled.c
@@ -11,10 +11,6 @@
int is_selinux_enabled(void)
{
- char *buf=NULL;
- FILE *fp;
- ssize_t num;
- size_t len;
int enabled = 0;
security_context_t con;
@@ -32,37 +28,8 @@ int is_selinux_enabled(void)
enabled = 0;
freecon(con);
}
- return enabled;
}
- /* Drop back to detecting it the long way. */
- fp = fopen("/proc/filesystems", "r");
- if (!fp)
- return -1;
-
- __fsetlocking(fp, FSETLOCKING_BYCALLER);
- while ((num = getline(&buf, &len, fp)) != -1) {
- if (strstr(buf, "selinuxfs")) {
- enabled = 1;
- break;
- }
- }
-
- if (num < 0)
- goto out;
-
- /* Since an selinux file system is available, we consider
- * selinux enabled. If getcon_raw fails, selinux is still
- * enabled. We only consider it disabled if no policy is loaded. */
- if (getcon_raw(&con) == 0) {
- if (!strcmp(con, "kernel"))
- enabled = 0;
- freecon(con);
- }
-
- out:
- free(buf);
- fclose(fp);
return enabled;
}
diff --git a/libselinux/src/init.c b/libselinux/src/init.c
index a948920..547f1eb 100644
--- a/libselinux/src/init.c
+++ b/libselinux/src/init.c
@@ -7,6 +7,7 @@
#include <stdio.h>
#include <stdio_ext.h>
#include <dlfcn.h>
+#include <sys/statvfs.h>
#include <sys/vfs.h>
#include <stdint.h>
#include <limits.h>
@@ -24,8 +25,6 @@ static void init_selinuxmnt(void)
{
char *buf=NULL, *p;
FILE *fp=NULL;
- struct statfs sfbuf;
- int rc;
size_t len;
ssize_t num;
int exists = 0;
@@ -33,17 +32,9 @@ static void init_selinuxmnt(void)
if (selinux_mnt)
return;
- /* We check to see if the preferred mount point for selinux file
- * system has a selinuxfs. */
- do {
- rc = statfs(SELINUXMNT, &sfbuf);
- } while (rc < 0 && errno == EINTR);
- if (rc == 0) {
- if ((uint32_t)sfbuf.f_type == (uint32_t)SELINUX_MAGIC) {
- selinux_mnt = strdup(SELINUXMNT);
- return;
- }
- }
+ if (set_selinuxmnt(SELINUXMNT) == 0) return;
+
+ if (set_selinuxmnt(OLDSELINUXMNT) == 0) return;
/* Drop back to detecting it the long way. */
fp = fopen("/proc/filesystems", "r");
@@ -52,7 +43,7 @@ static void init_selinuxmnt(void)
__fsetlocking(fp, FSETLOCKING_BYCALLER);
while ((num = getline(&buf, &len, fp)) != -1) {
- if (strstr(buf, "selinuxfs")) {
+ if (strstr(buf, SELINUXFS)) {
exists = 1;
break;
}
@@ -79,7 +70,7 @@ static void init_selinuxmnt(void)
tmp = strchr(p, ' ');
if (!tmp)
goto out;
- if (!strncmp(tmp + 1, "selinuxfs ", 10)) {
+ if (!strncmp(tmp + 1, SELINUXFS, 10)) {
*tmp = '\0';
break;
}
@@ -87,7 +78,7 @@ static void init_selinuxmnt(void)
/* If we found something, dup it */
if (num > 0)
- selinux_mnt = strdup(p);
+ set_selinuxmnt(p);
out:
free(buf);
@@ -104,9 +95,30 @@ void fini_selinuxmnt(void)
hidden_def(fini_selinuxmnt)
-void set_selinuxmnt(char *mnt)
+int set_selinuxmnt(char *mnt)
{
- selinux_mnt = strdup(mnt);
+ struct statfs sfbuf;
+ int rc;
+
+ /* We check to see if the preferred mount point for selinux file
+ * system has a selinuxfs. */
+ do {
+ rc = statfs(mnt, &sfbuf);
+ } while (rc < 0 && errno == EINTR);
+ if (rc == 0) {
+ if ((uint32_t)sfbuf.f_type == (uint32_t)SELINUX_MAGIC) {
+ struct statvfs vfsbuf;
+ rc = statvfs(mnt, &vfsbuf);
+ if (rc == 0) {
+ if (!(vfsbuf.f_flag & ST_RDONLY)) {
+ selinux_mnt = strdup(mnt);
+ return 0;
+ }
+ }
+ }
+ }
+
+ return -1;
}
hidden_def(set_selinuxmnt)
diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
index 83d2143..f6eae49 100644
--- a/libselinux/src/load_policy.c
+++ b/libselinux/src/load_policy.c
@@ -369,7 +369,17 @@ int selinux_init_load_policy(int *enforce)
* Check for the existence of SELinux via selinuxfs, and
* mount it if present for use in the calls below.
*/
- if (mount("selinuxfs", SELINUXMNT, "selinuxfs", 0, 0) < 0 && errno != EBUSY) {
+ char *mntpoint = NULL;
+ if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, 0, 0) == 0 || errno == EBUSY) {
+ mntpoint = SELINUXMNT;
+ } else {
+ /* check old mountpoint */
+ if (mount(SELINUXFS, OLDSELINUXMNT, SELINUXFS, 0, 0) == 0 || errno == EBUSY) {
+ mntpoint = OLDSELINUXMNT;
+ }
+ }
+
+ if (! mntpoint ) {
if (errno == ENODEV) {
/*
* SELinux was disabled in the kernel, either
@@ -384,8 +394,11 @@ int selinux_init_load_policy(int *enforce)
}
goto noload;
+ }
+ if (set_selinuxmnt(mntpoint) != 0) {
+ fprintf(stderr, "Mount failed for selinuxfs on %s: %s\n", mntpoint, strerror(errno));
+ goto noload;
}
- set_selinuxmnt(SELINUXMNT);
/*
* Note: The following code depends on having selinuxfs
@@ -397,7 +410,7 @@ int selinux_init_load_policy(int *enforce)
rc = security_disable();
if (rc == 0) {
/* Successfully disabled, so umount selinuxfs too. */
- umount(SELINUXMNT);
+ umount(selinux_mnt);
fini_selinuxmnt();
}
/*
diff --git a/libselinux/src/policy.h b/libselinux/src/policy.h
index 10e8712..bf270b5 100644
--- a/libselinux/src/policy.h
+++ b/libselinux/src/policy.h
@@ -9,11 +9,15 @@
/* Initial length guess for getting contexts. */
#define INITCONTEXTLEN 255
+/* selinux file system type */
+#define SELINUXFS "selinuxfs"
+
/* selinuxfs magic number */
#define SELINUX_MAGIC 0xf97cff8c
/* Preferred selinux mount location */
-#define SELINUXMNT "/selinux"
+#define SELINUXMNT "/sys/fs/selinux"
+#define OLDSELINUXMNT "/selinux"
/* selinuxfs mount point */
extern char *selinux_mnt;
Attachment:
libselinux-mountpoint.patch.sig
Description: PGP signature
