diff mbox

selinux: normalize input to /sys/fs/selinux/enforce

Message ID 1479479438-7123-1-git-send-email-sds@tycho.nsa.gov (mailing list archive)
State Accepted
Headers show

Commit Message

Stephen Smalley Nov. 18, 2016, 2:30 p.m. UTC 
At present, one can write any signed integer value to
/sys/fs/selinux/enforce and it will be stored,
e.g. echo -1 > /sys/fs/selinux/enforce or echo 2 >
/sys/fs/selinux/enforce. This makes no real difference
to the kernel, since it only ever cares if it is zero or non-zero,
but some userspace code compares it with 1 to decide if SELinux
is enforcing, and this could confuse it. Only a process that is
already root and is allowed the setenforce permission in SELinux
policy can write to /sys/fs/selinux/enforce, so this is not considered
to be a security issue, but it should be fixed.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 security/selinux/selinuxfs.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Paul Moore Nov. 20, 2016, 10:16 p.m. UTC | #1 
On Fri, Nov 18, 2016 at 9:30 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> At present, one can write any signed integer value to
> /sys/fs/selinux/enforce and it will be stored,
> e.g. echo -1 > /sys/fs/selinux/enforce or echo 2 >
> /sys/fs/selinux/enforce. This makes no real difference
> to the kernel, since it only ever cares if it is zero or non-zero,
> but some userspace code compares it with 1 to decide if SELinux
> is enforcing, and this could confuse it. Only a process that is
> already root and is allowed the setenforce permission in SELinux
> policy can write to /sys/fs/selinux/enforce, so this is not considered
> to be a security issue, but it should be fixed.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>

Merged, thanks.
diff mbox

Patch

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 72c145d..4c3e439 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -163,6 +163,8 @@  static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 	if (sscanf(page, "%d", &new_value) != 1)
 		goto out;
 
+	new_value = !!new_value;
+
 	if (new_value != selinux_enforcing) {
 		length = task_has_security(current, SECURITY__SETENFORCE);
 		if (length)