diff mbox series

selinux: avoid dereferencing the policy prior to initialization

Message ID 20200819134541.41136-1-stephen.smalley.work@gmail.com (mailing list archive)
State Accepted
Headers show
Series selinux: avoid dereferencing the policy prior to initialization | expand

Commit Message

Stephen Smalley Aug. 19, 2020, 1:45 p.m. UTC 
Certain SELinux security server functions (e.g. security_port_sid,
called during bind) were not explicitly testing to see if SELinux
has been initialized (i.e. initial policy loaded) and handling
the no-policy-loaded case.  In the past this happened to work
because the policydb was statically allocated and could always
be accessed, but with the recent encapsulation of policy state
and conversion to dynamic allocation, we can no longer access
the policy state prior to initialization.  Add a test of
!selinux_initialized(state) to all of the exported functions that
were missing them and handle appropriately.

Fixes: 461698026ffa ("selinux: encapsulate policy state, refactor policy load")
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
 security/selinux/ss/services.c | 60 ++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

Comments

Stephen Smalley Aug. 19, 2020, 2:22 p.m. UTC | #1 
On 8/19/20 9:45 AM, Stephen Smalley wrote:

> Certain SELinux security server functions (e.g. security_port_sid,
> called during bind) were not explicitly testing to see if SELinux
> has been initialized (i.e. initial policy loaded) and handling
> the no-policy-loaded case.  In the past this happened to work
> because the policydb was statically allocated and could always
> be accessed, but with the recent encapsulation of policy state
> and conversion to dynamic allocation, we can no longer access
> the policy state prior to initialization.  Add a test of
> !selinux_initialized(state) to all of the exported functions that
> were missing them and handle appropriately.
>
> Fixes: 461698026ffa ("selinux: encapsulate policy state, refactor policy load")
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>

To test this with no policy loaded, you can do the following on Fedora:

dnf remove selinux-policy-targeted

rm /etc/selinux/config

reboot

It is important to do it in that order; removing selinux-policy-targeted 
creates an /etc/selinux/config with SELINUX=disabled, which you must 
remove to keep SELinux enabled but with no policy. Also removing 
/etc/selinux/config is necessary to cause libselinux to report that 
SELinux is disabled to userspace, thereby avoiding all kinds of 
userspace breakage.
Paul Moore Aug. 20, 2020, 1:19 a.m. UTC | #2 
On Wed, Aug 19, 2020 at 9:45 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> Certain SELinux security server functions (e.g. security_port_sid,
> called during bind) were not explicitly testing to see if SELinux
> has been initialized (i.e. initial policy loaded) and handling
> the no-policy-loaded case.  In the past this happened to work
> because the policydb was statically allocated and could always
> be accessed, but with the recent encapsulation of policy state
> and conversion to dynamic allocation, we can no longer access
> the policy state prior to initialization.  Add a test of
> !selinux_initialized(state) to all of the exported functions that
> were missing them and handle appropriately.
>
> Fixes: 461698026ffa ("selinux: encapsulate policy state, refactor policy load")
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
>  security/selinux/ss/services.c | 60 ++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)

Merged into selinux/next, thanks Stephen.
Naresh Kamboju Aug. 20, 2020, 4:54 a.m. UTC | #3 
On Wed, 19 Aug 2020 at 19:15, Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> Certain SELinux security server functions (e.g. security_port_sid,
> called during bind) were not explicitly testing to see if SELinux
> has been initialized (i.e. initial policy loaded) and handling
> the no-policy-loaded case.  In the past this happened to work
> because the policydb was statically allocated and could always
> be accessed, but with the recent encapsulation of policy state
> and conversion to dynamic allocation, we can no longer access
> the policy state prior to initialization.  Add a test of
> !selinux_initialized(state) to all of the exported functions that
> were missing them and handle appropriately.
>
> Fixes: 461698026ffa ("selinux: encapsulate policy state, refactor policy load")
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>

The reported problem has been solved after applying this patch on top
of Linux next 20200819 tag.

Test log,
https://lkft.validation.linaro.org/scheduler/job/1687070#L317

- Naresh
diff mbox series

Patch

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index f6f78c65f53f..b3b610a58096 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2293,6 +2293,9 @@  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);
@@ -2314,6 +2317,11 @@  int security_port_sid(struct selinux_state *state,
 	struct ocontext *c;
 	int rc = 0;
 
+	if (!selinux_initialized(state)) {
+		*out_sid = SECINITSID_PORT;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -2359,6 +2367,11 @@  int security_ib_pkey_sid(struct selinux_state *state,
 	struct ocontext *c;
 	int rc = 0;
 
+	if (!selinux_initialized(state)) {
+		*out_sid = SECINITSID_UNLABELED;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -2405,6 +2418,11 @@  int security_ib_endport_sid(struct selinux_state *state,
 	struct ocontext *c;
 	int rc = 0;
 
+	if (!selinux_initialized(state)) {
+		*out_sid = SECINITSID_UNLABELED;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -2450,6 +2468,11 @@  int security_netif_sid(struct selinux_state *state,
 	int rc = 0;
 	struct ocontext *c;
 
+	if (!selinux_initialized(state)) {
+		*if_sid = SECINITSID_NETIF;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -2513,6 +2536,11 @@  int security_node_sid(struct selinux_state *state,
 	int rc;
 	struct ocontext *c;
 
+	if (!selinux_initialized(state)) {
+		*out_sid = SECINITSID_NODE;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -2780,6 +2808,11 @@  int security_genfs_sid(struct selinux_state *state,
 {
 	int retval;
 
+	if (!selinux_initialized(state)) {
+		*sid = SECINITSID_UNLABELED;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 	retval = __security_genfs_sid(state->ss->policy,
 				fstype, path, orig_sclass, sid);
@@ -2810,6 +2843,12 @@  int security_fs_use(struct selinux_state *state, struct super_block *sb)
 	struct superblock_security_struct *sbsec = sb->s_security;
 	const char *fstype = sb->s_type->name;
 
+	if (!selinux_initialized(state)) {
+		sbsec->behavior = SECURITY_FS_USE_NONE;
+		sbsec->sid = SECINITSID_UNLABELED;
+		return 0;
+	}
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -2906,6 +2945,9 @@  int security_set_bools(struct selinux_state *state, u32 len, int *values)
 	int rc;
 	u32 i, seqno = 0;
 
+	if (!selinux_initialized(state))
+		return -EINVAL;
+
 	/*
 	 * NOTE: We do not need to take the policy read-lock
 	 * around the code below because other policy-modifying
@@ -2982,6 +3024,9 @@  int security_get_bool_value(struct selinux_state *state,
 	int rc;
 	u32 len;
 
+	if (!selinux_initialized(state))
+		return 0;
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -3161,6 +3206,9 @@  int security_net_peersid_resolve(struct selinux_state *state,
 		return 0;
 	}
 
+	if (!selinux_initialized(state))
+		return 0;
+
 	read_lock(&state->ss->policy_rwlock);
 
 	policydb = &state->ss->policy->policydb;
@@ -3307,6 +3355,9 @@  int security_get_reject_unknown(struct selinux_state *state)
 {
 	int value;
 
+	if (!selinux_initialized(state))
+		return 0;
+
 	read_lock(&state->ss->policy_rwlock);
 	value = state->ss->policy->policydb.reject_unknown;
 	read_unlock(&state->ss->policy_rwlock);
@@ -3317,6 +3368,9 @@  int security_get_allow_unknown(struct selinux_state *state)
 {
 	int value;
 
+	if (!selinux_initialized(state))
+		return 0;
+
 	read_lock(&state->ss->policy_rwlock);
 	value = state->ss->policy->policydb.allow_unknown;
 	read_unlock(&state->ss->policy_rwlock);
@@ -3338,6 +3392,9 @@  int security_policycap_supported(struct selinux_state *state,
 {
 	int rc;
 
+	if (!selinux_initialized(state))
+		return 0;
+
 	read_lock(&state->ss->policy_rwlock);
 	rc = ebitmap_get_bit(&state->ss->policy->policydb.policycaps, req_cap);
 	read_unlock(&state->ss->policy_rwlock);
@@ -3499,6 +3556,9 @@  int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
 		return -ENOENT;
 	}
 
+	if (!selinux_initialized(state))
+		return 0;
+
 	read_lock(&state->ss->policy_rwlock);
 
 	if (rule->au_seqno < state->ss->latest_granting) {