-----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-----