[2/2] selinux: fix a race condition in sel_open_policy()

Commit Message

Ondrej Mosnacek Aug. 24, 2020, 11:30 a.m. UTC

The code to update the policy inode size is racy and inefficient. Move
it below the security_read_policy() call where we already know the
length of the policy we are returning.

Since after this, security_policydb_len() is only called from
security_load_policy(), remove it and just open-code it there.

Fixes: cee74f47a6ba ("SELinux: allow userspace to read policy back out of the kernel")
Signed-off-by: Ondrej Mosnacek <[e-mail]>
---
 security/selinux/include/security.h |  1 -
 security/selinux/selinuxfs.c        | 12 ++++++------
 security/selinux/ss/services.c      | 18 +++---------------
 3 files changed, 9 insertions(+), 22 deletions(-)

Patch

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index c68ed2beadff4..2c14d4165d688 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -219,7 +219,6 @@  void selinux_policy_cancel(struct selinux_state *state,
 			struct selinux_policy *policy);
 int security_read_policy(struct selinux_state *state,
 			 void **data, size_t *len);
-size_t security_policydb_len(struct selinux_state *state);
 
 int security_policycap_supported(struct selinux_state *state,
 				 unsigned int req_cap);
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 131816878e503..098d012cf40d8 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -403,16 +403,16 @@  static int sel_open_policy(struct inode *inode, struct file *filp)
 	if (!plm)
 		goto err;
 
-	if (i_size_read(inode) != security_policydb_len(state)) {
-		inode_lock(inode);
-		i_size_write(inode, security_policydb_len(state));
-		inode_unlock(inode);
-	}
-
 	rc = security_read_policy(state, &plm->data, &plm->len);
 	if (rc)
 		goto err;
 
+	if ((size_t)i_size_read(inode) != plm->len) {
+		inode_lock(inode);
+		i_size_write(inode, plm->len);
+		inode_unlock(inode);
+	}
+
 	fsi->policy_opened = 1;
 
 	filp->private_data = plm;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2c9072f095985..0745d4f3a5765 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2289,20 +2289,6 @@  err:
 	return rc;
 }
 
-size_t security_policydb_len(struct selinux_state *state)
-{
-	size_t len;
-
-	if (!selinux_initialized(state))
-		return 0;
-
-	read_lock(&state->ss->policy_rwlock);
-	len = state->ss->policy->policydb.len;
-	read_unlock(&state->ss->policy_rwlock);
-
-	return len;
-}
-
 /**
  * security_port_sid - Obtain the SID for a port.
  * @protocol: protocol number
@@ -3847,7 +3833,9 @@  int security_read_policy(struct selinux_state *state,
 	if (!selinux_initialized(state))
 		return -EINVAL;
 
-	*len = security_policydb_len(state);
+	read_lock(&state->ss->policy_rwlock);
+	*len = state->ss->policy->policydb.len;
+	read_unlock(&state->ss->policy_rwlock);
 
 again:
 	*data = vmalloc_user(*len);