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;
[...]