From: Stephen Smalley <sds@tycho.nsa.gov>
To: Petr Lautrbach <plautrba@redhat.com>, selinux@vger.kernel.org
Subject: Re: [PATCH v4] libselinux: Eliminate use of security_compute_user()
Date: Mon, 10 Feb 2020 13:40:15 -0500 [thread overview]
Message-ID: <5e88f99a-555c-9467-4cb4-6949b7cfdc98@tycho.nsa.gov> (raw)
In-Reply-To: <20200210182302.352398-1-plautrba@redhat.com>
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@redhat.com>
> ---
> 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;
[...]
next prev parent reply other threads:[~2020-02-10 18:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-10 18:23 [PATCH v4] libselinux: Eliminate use of security_compute_user() Petr Lautrbach
2020-02-10 18:40 ` Stephen Smalley [this message]
2020-02-10 19:10 ` Petr Lautrbach
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5e88f99a-555c-9467-4cb4-6949b7cfdc98@tycho.nsa.gov \
--to=sds@tycho.nsa.gov \
--cc=plautrba@redhat.com \
--cc=selinux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).