Skip to content

Commit

Permalink
TOMOYO: Avoid race when retrying "file execute" permission check.
Browse files Browse the repository at this point in the history
There was a race window that the pathname which is subjected to "file execute"
permission check when retrying via supervisor's decision because the pathname
was recalculated upon retry. Though, there is an inevitable race window even
without supervisor, for we have to calculate the symbolic link's pathname from
"struct linux_binprm"->filename rather than from "struct linux_binprm"->file
because we cannot back calculate the symbolic link's pathname from the
dereferenced pathname.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
  • Loading branch information
Tetsuo Handa authored and James Morris committed Sep 13, 2011
1 parent 731d37a commit a8f7640
Showing 1 changed file with 22 additions and 34 deletions.
56 changes: 22 additions & 34 deletions security/tomoyo/domain.c
Original file line number Diff line number Diff line change
Expand Up @@ -664,9 +664,9 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
struct tomoyo_domain_info *domain = NULL;
const char *original_name = bprm->filename;
int retval = -ENOMEM;
bool need_kfree = false;
bool reject_on_transition_failure = false;
struct tomoyo_path_info rn = { }; /* real name */
const struct tomoyo_path_info *candidate;
struct tomoyo_path_info exename;
struct tomoyo_execve *ee = kzalloc(sizeof(*ee), GFP_NOFS);

if (!ee)
Expand All @@ -682,40 +682,33 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
ee->bprm = bprm;
ee->r.obj = &ee->obj;
ee->obj.path1 = bprm->file->f_path;
retry:
if (need_kfree) {
kfree(rn.name);
need_kfree = false;
}
/* Get symlink's pathname of program. */
retval = -ENOENT;
rn.name = tomoyo_realpath_nofollow(original_name);
if (!rn.name)
exename.name = tomoyo_realpath_nofollow(original_name);
if (!exename.name)
goto out;
tomoyo_fill_path_info(&rn);
need_kfree = true;

tomoyo_fill_path_info(&exename);
retry:
/* Check 'aggregator' directive. */
{
struct tomoyo_aggregator *ptr;
struct list_head *list =
&old_domain->ns->policy_list[TOMOYO_ID_AGGREGATOR];
/* Check 'aggregator' directive. */
candidate = &exename;
list_for_each_entry_rcu(ptr, list, head.list) {
if (ptr->head.is_deleted ||
!tomoyo_path_matches_pattern(&rn,
!tomoyo_path_matches_pattern(&exename,
ptr->original_name))
continue;
kfree(rn.name);
need_kfree = false;
/* This is OK because it is read only. */
rn = *ptr->aggregated_name;
candidate = ptr->aggregated_name;
break;
}
}

/* Check execute permission. */
retval = tomoyo_path_permission(&ee->r, TOMOYO_TYPE_EXECUTE, &rn);
retval = tomoyo_path_permission(&ee->r, TOMOYO_TYPE_EXECUTE,
candidate);
if (retval == TOMOYO_RETRY_REQUEST)
goto retry;
if (retval < 0)
Expand All @@ -726,20 +719,16 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
* wildcard) rather than the pathname passed to execve()
* (which never contains wildcard).
*/
if (ee->r.param.path.matched_path) {
if (need_kfree)
kfree(rn.name);
need_kfree = false;
/* This is OK because it is read only. */
rn = *ee->r.param.path.matched_path;
}
if (ee->r.param.path.matched_path)
candidate = ee->r.param.path.matched_path;

/* Calculate domain to transit to. */
switch (tomoyo_transition_type(old_domain->ns, old_domain->domainname,
&rn)) {
candidate)) {
case TOMOYO_TRANSITION_CONTROL_RESET:
/* Transit to the root of specified namespace. */
snprintf(ee->tmp, TOMOYO_EXEC_TMPSIZE - 1, "<%s>", rn.name);
snprintf(ee->tmp, TOMOYO_EXEC_TMPSIZE - 1, "<%s>",
candidate->name);
/*
* Make do_execve() fail if domain transition across namespaces
* has failed.
Expand All @@ -749,7 +738,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
case TOMOYO_TRANSITION_CONTROL_INITIALIZE:
/* Transit to the child of current namespace's root. */
snprintf(ee->tmp, TOMOYO_EXEC_TMPSIZE - 1, "%s %s",
old_domain->ns->name, rn.name);
old_domain->ns->name, candidate->name);
break;
case TOMOYO_TRANSITION_CONTROL_KEEP:
/* Keep current domain. */
Expand All @@ -765,11 +754,11 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
* before /sbin/init.
*/
domain = old_domain;
} else {
/* Normal domain transition. */
snprintf(ee->tmp, TOMOYO_EXEC_TMPSIZE - 1, "%s %s",
old_domain->domainname->name, rn.name);
break;
}
/* Normal domain transition. */
snprintf(ee->tmp, TOMOYO_EXEC_TMPSIZE - 1, "%s %s",
old_domain->domainname->name, candidate->name);
break;
}
if (!domain)
Expand Down Expand Up @@ -799,8 +788,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
/* Update reference count on "struct tomoyo_domain_info". */
atomic_inc(&domain->users);
bprm->cred->security = domain;
if (need_kfree)
kfree(rn.name);
kfree(exename.name);
if (!retval) {
ee->r.domain = domain;
retval = tomoyo_environ(ee);
Expand Down

0 comments on commit a8f7640

Please sign in to comment.