On Fri, Dec 13, 2019 at 9:44 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > On 12/13/19 9:32 AM, Stephen Smalley wrote: > > Rename selinux_enabled to selinux_enabled_boot to make it clear that > > it only reflects whether SELinux was enabled at boot. Further make it > > unconditionally read-only-after-init and stop needlessly clearing it > > in selinux_disable() since it is only used to skip other SELinux > > initialization code. > > > > Wrap the disabled field in the struct selinux_state with > > CONFIG_SECURITY_SELINUX_DISABLE since it is only used for > > runtime disable. > > > > Introduce a selinux_is_enabled() static inline that tests both > > selinux_enabled_boot and selinux_state.disabled as appropriate > > to determine whether SELinux is in an enabled state. Use this function > > in the MAC_STATUS audit log message in place of selinux_enabled(_boot). > > It is unclear why this information is included in that audit record > > since selinuxfs is never registered at all if !selinux_enabled_boot > > and is unregistered in the runtime disable case, so this code should never > > be reached if SELinux is disabled. It is also unclear why it is logged > > twice under enabled/old-enabled since setenforce does not change its > > value. Regardless, we leave it as is for compatibility. > > Just noticed that there is another AUDIT_MAC_STATUS audit log in > sel_write_disable() that uses hardcoded 0, 1 for enabled and old-enabled > values in the audit record. Don't know if it should also use > selinux_is_enabled(), or if we should likewise just hardcode the values > used in the sel_write_enforce() case (to 1, 1, since this code shouldn't > be reachable if SELinux were disabled). If the latter, we don't need > selinux_is_enabled() at all. I see no reason why we can't hardcode the enabled/old-enabled values as they are always going to be "1" here. As for why the values are being logged there at all, it has to do with a desire to have a consistent format for each record type; in sel_write_enforce() we log the enabled/old-enabled values for the MAC_STATUS record, so we need to ensure we log the values in sel_write_disable() as well. Looks fine to me otherwise. -- paul moore www.paul-moore.com
