Re: [PATCH v4] libselinux: Eliminate use of security_compute_user()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/10/20 1:23 PM, Petr Lautrbach wrote:
get_ordered_context_list() code used to ask the kernel to compute the complete
set of reachable contexts using /sys/fs/selinux/user aka
security_compute_user(). This set can be so huge so that it doesn't fit into a
kernel page and security_compute_user() fails. Even if it doesn't fail,
get_ordered_context_list() throws away the vast majority of the returned
contexts because they don't match anything in
/etc/selinux/targeted/contexts/default_contexts or
/etc/selinux/targeted/contexts/users/

get_ordered_context_list() is rewritten to compute set of contexts based on
/etc/selinux/targeted/contexts/users/ and
/etc/selinux/targeted/contexts/default_contexts files and to return only valid
contexts, using security_check_context(), from this set.

Fixes: https://github.com/SELinuxProject/selinux/issues/28

Signed-off-by: Petr Lautrbach <plautrba@xxxxxxxxxx>
---

diff --git a/libselinux/src/get_context_list.c b/libselinux/src/get_context_list.c
index 689e46589f30..fb53fd436650 100644
--- a/libselinux/src/get_context_list.c
+++ b/libselinux/src/get_context_list.c
@@ -243,23 +222,84 @@ static int get_context_order(FILE * fp,
  		if (*end)
  			*end++ = 0;
- /* Check for a match in the reachable list. */
-		rc = find_partialcon(reachable, nreach, start);
-		if (rc < 0) {
-			/* No match, skip it. */
+		/* Check whether a new context is valid */
+		if (SIZE_MAX - user_len < strlen(start) + 2) {
+			fprintf(stderr, "%s: one of partial contexts is too big\n", __FUNCTION__);
+			errno = EINVAL;
+			rc = -1;
+			goto out;
+		}
+		usercon_len = user_len + strlen(start) + 2;
+		usercon_str = malloc(usercon_len);
+		if (!usercon_str) {
+			rc = -1;
+			goto out;
+		}
+
+		/* set range from fromcon in the new usercon */
+		snprintf(usercon_str, usercon_len, "%s:%s", user, start);
+		usercon = context_new(usercon_str);
+		if (!usercon) {
+			if (errno != EINVAL) {
+				free(usercon_str);
+				rc = -1;
+				goto out;
+			}
+			fprintf(stderr,
+				"%s: can't create a context from %s, skipping\n",
+				__FUNCTION__, usercon_str);
+			free(usercon_str);
  			start = end;
  			continue;

Feel free to make this a fatal error too; I can't see a valid scenario for it. The only cases where context_new() can fail are a syntactically invalid context or out of memory.

  		}

I think we could lift the free(usercon_str); to here or even immediately after the context_new() if we stopped including it in the error message above. Then we don't have to repeat it below multiple times.

+		if (context_range_set(usercon, fromlevel) != 0) {
+			free(usercon_str);
+			context_free(usercon);
+			rc = -1;
+			goto out;
+		}
+		free(usercon_str);
+		usercon_str = context_str(usercon);
+		if (!usercon_str) {
+			context_free(usercon);
+			rc = -1;
+			goto out;
+		}
- /* If a match is found and the entry is not already ordered
-		   (e.g. due to prior match in prior config file), then set
-		   the ordering for it. */
-		i = rc;
-		if (ordering[i] == nreach)
-			ordering[i] = (*nordered)++;
+		/* check whether usercon is already in reachable */
+		if (is_in_reachable(*reachable, usercon_str)) {
+			start = end;

Still need a context_free(usercon); here in order to avoid leaking its memory.

+			continue;

[...]



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux