2009-06-02 01:43:25

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH 2/5] TOMOYO: Clarify lock protected section.

Enclose reader section in
/***** READER SECTION START *****/
and
/***** READER SECTION END *****/
and writer section in
/***** WRITER SECTION START *****/
and
/***** WRITER SECTION END *****/
in order to avoid oversighting lock protected section.

Signed-off-by: Kentaro Takeda <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
Signed-off-by: Toshiharu Harada <[email protected]>
---
security/tomoyo/common.c | 30 ++++++++++++++++++++++++++++--
security/tomoyo/domain.c | 34 ++++++++++++++++++++++++----------
security/tomoyo/file.c | 36 ++++++++++++++++++++++++++----------
security/tomoyo/realpath.c | 2 ++
4 files changed, 80 insertions(+), 22 deletions(-)

--- security-testing-2.6.git.orig/security/tomoyo/common.c
+++ security-testing-2.6.git/security/tomoyo/common.c
@@ -706,6 +706,7 @@ static const char *tomoyo_get_exe(void)

if (!mm)
return NULL;
+ /***** READER SECTION START *****/
down_read(&mm->mmap_sem);
for (vma = mm->mmap; vma; vma = vma->vm_next) {
if ((vma->vm_flags & VM_EXECUTABLE) && vma->vm_file) {
@@ -714,6 +715,7 @@ static const char *tomoyo_get_exe(void)
}
}
up_read(&mm->mmap_sem);
+ /***** READER SECTION END *****/
return cp;
}

@@ -784,6 +786,7 @@ bool tomoyo_domain_quota_is_ok(struct to

if (!domain)
return true;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_acl_info_list_lock);
list_for_each_entry(ptr, &domain->acl_info_list, list) {
if (ptr->type & TOMOYO_ACL_DELETED)
@@ -839,6 +842,7 @@ bool tomoyo_domain_quota_is_ok(struct to
}
}
up_read(&tomoyo_domain_acl_info_list_lock);
+ /***** READER SECTION END *****/
if (count < tomoyo_check_flags(domain, TOMOYO_MAX_ACCEPT_ENTRY))
return true;
if (!domain->quota_warned) {
@@ -1053,7 +1057,7 @@ static int tomoyo_update_manager_entry(c
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_policy_manager_list_lock);
list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
if (ptr->manager != saved_manager)
@@ -1070,7 +1074,7 @@ static int tomoyo_update_manager_entry(c
error = 0;
}
up_write(&tomoyo_policy_manager_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -1108,6 +1112,7 @@ static int tomoyo_read_manager_policy(st

if (head->read_eof)
return 0;
+ /***** READER SECTION START *****/
down_read(&tomoyo_policy_manager_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_policy_manager_list) {
@@ -1122,6 +1127,7 @@ static int tomoyo_read_manager_policy(st
}
}
up_read(&tomoyo_policy_manager_list_lock);
+ /***** READER SECTION END *****/
head->read_eof = done;
return 0;
}
@@ -1144,6 +1150,7 @@ static bool tomoyo_is_policy_manager(voi
return true;
if (!tomoyo_manage_by_non_root && (task->cred->uid || task->cred->euid))
return false;
+ /***** READER SECTION START *****/
down_read(&tomoyo_policy_manager_list_lock);
list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
if (!ptr->is_deleted && ptr->is_domain
@@ -1153,11 +1160,13 @@ static bool tomoyo_is_policy_manager(voi
}
}
up_read(&tomoyo_policy_manager_list_lock);
+ /***** READER SECTION END *****/
if (found)
return true;
exe = tomoyo_get_exe();
if (!exe)
return false;
+ /***** READER SECTION START *****/
down_read(&tomoyo_policy_manager_list_lock);
list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
if (!ptr->is_deleted && !ptr->is_domain
@@ -1167,6 +1176,7 @@ static bool tomoyo_is_policy_manager(voi
}
}
up_read(&tomoyo_policy_manager_list_lock);
+ /***** READER SECTION END *****/
if (!found) { /* Reduce error messages. */
static pid_t last_pid;
const pid_t pid = current->pid;
@@ -1205,9 +1215,11 @@ static bool tomoyo_is_select_one(struct
/***** CRITICAL SECTION END *****/
} else if (!strncmp(data, "domain=", 7)) {
if (tomoyo_is_domain_def(data + 7)) {
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(data + 7);
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
}
} else
return false;
@@ -1222,6 +1234,7 @@ static bool tomoyo_is_select_one(struct
if (domain) {
struct tomoyo_domain_info *d;
head->read_var1 = NULL;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
list_for_each_entry(d, &tomoyo_domain_list, list) {
if (d == domain)
@@ -1229,6 +1242,7 @@ static bool tomoyo_is_select_one(struct
head->read_var1 = &d->list;
}
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
head->read_var2 = NULL;
head->read_bit = 0;
head->read_step = 0;
@@ -1267,9 +1281,11 @@ static int tomoyo_write_domain_policy(st
if (is_delete)
tomoyo_delete_domain(data);
else if (is_select) {
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(data);
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
} else
domain = tomoyo_find_or_assign_new_domain(data, 0);
head->write_var1 = domain;
@@ -1426,6 +1442,7 @@ static int tomoyo_read_domain_policy(str
return 0;
if (head->read_step == 0)
head->read_step = 1;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
list_for_each_cookie(dpos, head->read_var1, &tomoyo_domain_list) {
struct tomoyo_domain_info *domain;
@@ -1460,6 +1477,7 @@ acl_loop:
if (head->read_step == 3)
goto tail_mark;
/* Print ACL entries in the domain. */
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_acl_info_list_lock);
list_for_each_cookie(apos, head->read_var2,
&domain->acl_info_list) {
@@ -1472,6 +1490,7 @@ acl_loop:
}
}
up_read(&tomoyo_domain_acl_info_list_lock);
+ /***** READER SECTION END *****/
if (!done)
break;
head->read_step = 3;
@@ -1485,6 +1504,7 @@ tail_mark:
break;
}
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
head->read_eof = done;
return 0;
}
@@ -1511,9 +1531,11 @@ static int tomoyo_write_domain_profile(s
if (!cp)
return -EINVAL;
*cp = '\0';
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(cp + 1);
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
if (strict_strtoul(data, 10, &profile))
return -EINVAL;
if (domain && profile < TOMOYO_MAX_PROFILES
@@ -1543,6 +1565,7 @@ static int tomoyo_read_domain_profile(st

if (head->read_eof)
return 0;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
list_for_each_cookie(pos, head->read_var1, &tomoyo_domain_list) {
struct tomoyo_domain_info *domain;
@@ -1556,6 +1579,7 @@ static int tomoyo_read_domain_profile(st
}
}
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
head->read_eof = done;
return 0;
}
@@ -1777,6 +1801,7 @@ void tomoyo_load_policy(const char *file
tomoyo_policy_loaded = true;
{ /* Check all profiles currently assigned to domains are defined. */
struct tomoyo_domain_info *domain;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
list_for_each_entry(domain, &tomoyo_domain_list, list) {
const u8 profile = domain->profile;
@@ -1786,6 +1811,7 @@ void tomoyo_load_policy(const char *file
profile, domain->domainname->name);
}
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
}
}

--- security-testing-2.6.git.orig/security/tomoyo/domain.c
+++ security-testing-2.6.git/security/tomoyo/domain.c
@@ -137,7 +137,7 @@ static int tomoyo_update_domain_initiali
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_domain_initializer_list_lock);
list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_not != is_not ||
@@ -159,7 +159,7 @@ static int tomoyo_update_domain_initiali
error = 0;
}
up_write(&tomoyo_domain_initializer_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -176,6 +176,7 @@ bool tomoyo_read_domain_initializer_poli
struct list_head *pos;
bool done = true;

+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_initializer_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_domain_initializer_list) {
@@ -201,6 +202,7 @@ bool tomoyo_read_domain_initializer_poli
}
}
up_read(&tomoyo_domain_initializer_list_lock);
+ /***** READER SECTION END *****/
return done;
}

@@ -247,6 +249,7 @@ static bool tomoyo_is_domain_initializer
struct tomoyo_domain_initializer_entry *ptr;
bool flag = false;

+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_initializer_list_lock);
list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_deleted)
@@ -269,6 +272,7 @@ static bool tomoyo_is_domain_initializer
flag = true;
}
up_read(&tomoyo_domain_initializer_list_lock);
+ /***** READER SECTION END *****/
return flag;
}

@@ -316,7 +320,7 @@ static int tomoyo_update_domain_keeper_e
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_domain_keeper_list_lock);
list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_not != is_not ||
@@ -337,7 +341,7 @@ static int tomoyo_update_domain_keeper_e
error = 0;
}
up_write(&tomoyo_domain_keeper_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -375,6 +379,7 @@ bool tomoyo_read_domain_keeper_policy(st
struct list_head *pos;
bool done = true;

+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_keeper_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_domain_keeper_list) {
@@ -400,6 +405,7 @@ bool tomoyo_read_domain_keeper_policy(st
}
}
up_read(&tomoyo_domain_keeper_list_lock);
+ /***** READER SECTION END *****/
return done;
}

@@ -420,6 +426,7 @@ static bool tomoyo_is_domain_keeper(cons
struct tomoyo_domain_keeper_entry *ptr;
bool flag = false;

+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_keeper_list_lock);
list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_deleted)
@@ -440,6 +447,7 @@ static bool tomoyo_is_domain_keeper(cons
flag = true;
}
up_read(&tomoyo_domain_keeper_list_lock);
+ /***** READER SECTION END *****/
return flag;
}

@@ -475,7 +483,7 @@ static int tomoyo_update_alias_entry(con
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_alias_list_lock);
list_for_each_entry(ptr, &tomoyo_alias_list, list) {
if (ptr->original_name != saved_original_name ||
@@ -493,7 +501,7 @@ static int tomoyo_update_alias_entry(con
error = 0;
}
up_write(&tomoyo_alias_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -510,6 +518,7 @@ bool tomoyo_read_alias_policy(struct tom
struct list_head *pos;
bool done = true;

+ /***** READER SECTION START *****/
down_read(&tomoyo_alias_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_alias_list) {
struct tomoyo_alias_entry *ptr;
@@ -525,6 +534,7 @@ bool tomoyo_read_alias_policy(struct tom
}
}
up_read(&tomoyo_alias_list_lock);
+ /***** READER SECTION END *****/
return done;
}

@@ -562,7 +572,7 @@ int tomoyo_delete_domain(char *domainnam

name.name = domainname;
tomoyo_fill_path_info(&name);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_domain_list_lock);
/* Is there an active domain? */
list_for_each_entry(domain, &tomoyo_domain_list, list) {
@@ -576,7 +586,7 @@ int tomoyo_delete_domain(char *domainnam
break;
}
up_write(&tomoyo_domain_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
return 0;
}

@@ -602,7 +612,7 @@ struct tomoyo_domain_info *tomoyo_find_o
if (!saved_domainname)
return NULL;
new_domain = kmalloc(sizeof(*new_domain), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(domainname);
if (domain)
@@ -649,7 +659,7 @@ struct tomoyo_domain_info *tomoyo_find_o
}
out:
up_write(&tomoyo_domain_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_domain);
return domain;
}
@@ -722,6 +732,7 @@ int tomoyo_find_next_domain(struct linux
if (tomoyo_pathcmp(&r, &s)) {
struct tomoyo_alias_entry *ptr;
/* Is this program allowed to be called via symbolic links? */
+ /***** READER SECTION START *****/
down_read(&tomoyo_alias_list_lock);
list_for_each_entry(ptr, &tomoyo_alias_list, list) {
if (ptr->is_deleted ||
@@ -735,6 +746,7 @@ int tomoyo_find_next_domain(struct linux
break;
}
up_read(&tomoyo_alias_list_lock);
+ /***** READER SECTION END *****/
}

/* Check execute permission. */
@@ -765,9 +777,11 @@ int tomoyo_find_next_domain(struct linux
}
if (domain || strlen(new_domain_name) >= TOMOYO_MAX_PATHNAME_LEN)
goto done;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(new_domain_name);
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
if (domain)
goto done;
if (is_enforce)
--- security-testing-2.6.git.orig/security/tomoyo/file.c
+++ security-testing-2.6.git/security/tomoyo/file.c
@@ -168,7 +168,7 @@ static int tomoyo_update_globally_readab
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_globally_readable_list_lock);
list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
if (ptr->filename != saved_filename)
@@ -184,7 +184,7 @@ static int tomoyo_update_globally_readab
error = 0;
}
up_write(&tomoyo_globally_readable_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -201,6 +201,7 @@ static bool tomoyo_is_globally_readable_
{
struct tomoyo_globally_readable_file_entry *ptr;
bool found = false;
+ /***** READER SECTION START *****/
down_read(&tomoyo_globally_readable_list_lock);
list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
if (!ptr->is_deleted &&
@@ -210,6 +211,7 @@ static bool tomoyo_is_globally_readable_
}
}
up_read(&tomoyo_globally_readable_list_lock);
+ /***** READER SECTION END *****/
return found;
}

@@ -238,6 +240,7 @@ bool tomoyo_read_globally_readable_polic
struct list_head *pos;
bool done = true;

+ /***** READER SECTION START *****/
down_read(&tomoyo_globally_readable_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_globally_readable_list) {
@@ -254,6 +257,7 @@ bool tomoyo_read_globally_readable_polic
}
}
up_read(&tomoyo_globally_readable_list_lock);
+ /***** READER SECTION END *****/
return done;
}

@@ -284,7 +288,7 @@ static int tomoyo_update_file_pattern_en
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_pattern_list_lock);
list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
if (saved_pattern != ptr->pattern)
@@ -300,7 +304,7 @@ static int tomoyo_update_file_pattern_en
error = 0;
}
up_write(&tomoyo_pattern_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -318,6 +322,7 @@ tomoyo_get_file_pattern(const struct tom
struct tomoyo_pattern_entry *ptr;
const struct tomoyo_path_info *pattern = NULL;

+ /***** READER SECTION START *****/
down_read(&tomoyo_pattern_list_lock);
list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
if (ptr->is_deleted)
@@ -333,6 +338,7 @@ tomoyo_get_file_pattern(const struct tom
}
}
up_read(&tomoyo_pattern_list_lock);
+ /***** READER SECTION END *****/
if (pattern)
filename = pattern;
return filename;
@@ -363,6 +369,7 @@ bool tomoyo_read_file_pattern(struct tom
struct list_head *pos;
bool done = true;

+ /***** READER SECTION START *****/
down_read(&tomoyo_pattern_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_pattern_list) {
struct tomoyo_pattern_entry *ptr;
@@ -376,6 +383,7 @@ bool tomoyo_read_file_pattern(struct tom
}
}
up_read(&tomoyo_pattern_list_lock);
+ /***** READER SECTION END *****/
return done;
}

@@ -406,7 +414,7 @@ static int tomoyo_update_no_rewrite_entr
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_no_rewrite_list_lock);
list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->pattern != saved_pattern)
@@ -422,7 +430,7 @@ static int tomoyo_update_no_rewrite_entr
error = 0;
}
up_write(&tomoyo_no_rewrite_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -440,6 +448,7 @@ static bool tomoyo_is_no_rewrite_file(co
struct tomoyo_no_rewrite_entry *ptr;
bool found = false;

+ /***** READER SECTION START *****/
down_read(&tomoyo_no_rewrite_list_lock);
list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->is_deleted)
@@ -450,6 +459,7 @@ static bool tomoyo_is_no_rewrite_file(co
break;
}
up_read(&tomoyo_no_rewrite_list_lock);
+ /***** READER SECTION END *****/
return found;
}

@@ -478,6 +488,7 @@ bool tomoyo_read_no_rewrite_policy(struc
struct list_head *pos;
bool done = true;

+ /***** READER SECTION START *****/
down_read(&tomoyo_no_rewrite_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_no_rewrite_list) {
struct tomoyo_no_rewrite_entry *ptr;
@@ -491,6 +502,7 @@ bool tomoyo_read_no_rewrite_policy(struc
}
}
up_read(&tomoyo_no_rewrite_list_lock);
+ /***** READER SECTION END *****/
return done;
}

@@ -556,6 +568,7 @@ static int tomoyo_check_single_path_acl2
struct tomoyo_acl_info *ptr;
int error = -EPERM;

+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_acl_info_list_lock);
list_for_each_entry(ptr, &domain->acl_info_list, list) {
struct tomoyo_single_path_acl_record *acl;
@@ -576,6 +589,7 @@ static int tomoyo_check_single_path_acl2
break;
}
up_read(&tomoyo_domain_acl_info_list_lock);
+ /***** READER SECTION END *****/
return error;
}

@@ -742,7 +756,7 @@ static int tomoyo_update_single_path_acl
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
@@ -799,7 +813,7 @@ static int tomoyo_update_single_path_acl
}
out:
up_write(&tomoyo_domain_acl_info_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -838,7 +852,7 @@ static int tomoyo_update_double_path_acl
return -ENOMEM;
if (!is_delete)
new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
- /***** EXCLUSIVE SECTION START *****/
+ /***** WRITER SECTION START *****/
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
@@ -888,7 +902,7 @@ static int tomoyo_update_double_path_acl
}
out:
up_write(&tomoyo_domain_acl_info_list_lock);
- /***** EXCLUSIVE SECTION END *****/
+ /***** WRITER SECTION END *****/
kfree(new_entry);
return error;
}
@@ -934,6 +948,7 @@ static int tomoyo_check_double_path_acl(

if (!tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE))
return 0;
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_acl_info_list_lock);
list_for_each_entry(ptr, &domain->acl_info_list, list) {
struct tomoyo_double_path_acl_record *acl;
@@ -951,6 +966,7 @@ static int tomoyo_check_double_path_acl(
break;
}
up_read(&tomoyo_domain_acl_info_list_lock);
+ /***** READER SECTION END *****/
return error;
}

--- security-testing-2.6.git.orig/security/tomoyo/realpath.c
+++ security-testing-2.6.git/security/tomoyo/realpath.c
@@ -326,10 +326,12 @@ void __init tomoyo_realpath_init(void)
INIT_LIST_HEAD(&tomoyo_kernel_domain.acl_info_list);
tomoyo_kernel_domain.domainname = tomoyo_save_name(TOMOYO_ROOT_NAME);
list_add_tail(&tomoyo_kernel_domain.list, &tomoyo_domain_list);
+ /***** READER SECTION START *****/
down_read(&tomoyo_domain_list_lock);
if (tomoyo_find_domain(TOMOYO_ROOT_NAME) != &tomoyo_kernel_domain)
panic("Can't register tomoyo_kernel_domain");
up_read(&tomoyo_domain_list_lock);
+ /***** READER SECTION END *****/
}

/* Memory allocated for temporary purpose. */


2009-06-02 02:19:53

by Li Zefan

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

Tetsuo Handa wrote:
> Enclose reader section in
> /***** READER SECTION START *****/
> and
> /***** READER SECTION END *****/
> and writer section in
> /***** WRITER SECTION START *****/
> and
> /***** WRITER SECTION END *****/
> in order to avoid oversighting lock protected section.
>

This makes me a bit uncomfortable..

IMHO this seems ugly, useless, and even harmful. If it's helpful,
we'd be doing this for the whole kernel tree, which is crazy..

Or does tomoyo do this for it's special reason?

> Signed-off-by: Kentaro Takeda <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> Signed-off-by: Toshiharu Harada <[email protected]>
> ---
> security/tomoyo/common.c | 30 ++++++++++++++++++++++++++++--
> security/tomoyo/domain.c | 34 ++++++++++++++++++++++++----------
> security/tomoyo/file.c | 36 ++++++++++++++++++++++++++----------
> security/tomoyo/realpath.c | 2 ++
> 4 files changed, 80 insertions(+), 22 deletions(-)
>
> --- security-testing-2.6.git.orig/security/tomoyo/common.c
> +++ security-testing-2.6.git/security/tomoyo/common.c
> @@ -706,6 +706,7 @@ static const char *tomoyo_get_exe(void)
>
> if (!mm)
> return NULL;
> + /***** READER SECTION START *****/
> down_read(&mm->mmap_sem);
> for (vma = mm->mmap; vma; vma = vma->vm_next) {
> if ((vma->vm_flags & VM_EXECUTABLE) && vma->vm_file) {
> @@ -714,6 +715,7 @@ static const char *tomoyo_get_exe(void)
> }
> }
> up_read(&mm->mmap_sem);
> + /***** READER SECTION END *****/
> return cp;
> }
>
> @@ -784,6 +786,7 @@ bool tomoyo_domain_quota_is_ok(struct to
>
> if (!domain)
> return true;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_acl_info_list_lock);
> list_for_each_entry(ptr, &domain->acl_info_list, list) {
> if (ptr->type & TOMOYO_ACL_DELETED)
> @@ -839,6 +842,7 @@ bool tomoyo_domain_quota_is_ok(struct to
> }
> }
> up_read(&tomoyo_domain_acl_info_list_lock);
> + /***** READER SECTION END *****/
> if (count < tomoyo_check_flags(domain, TOMOYO_MAX_ACCEPT_ENTRY))
> return true;
> if (!domain->quota_warned) {
> @@ -1053,7 +1057,7 @@ static int tomoyo_update_manager_entry(c
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_policy_manager_list_lock);
> list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
> if (ptr->manager != saved_manager)
> @@ -1070,7 +1074,7 @@ static int tomoyo_update_manager_entry(c
> error = 0;
> }
> up_write(&tomoyo_policy_manager_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -1108,6 +1112,7 @@ static int tomoyo_read_manager_policy(st
>
> if (head->read_eof)
> return 0;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_policy_manager_list_lock);
> list_for_each_cookie(pos, head->read_var2,
> &tomoyo_policy_manager_list) {
> @@ -1122,6 +1127,7 @@ static int tomoyo_read_manager_policy(st
> }
> }
> up_read(&tomoyo_policy_manager_list_lock);
> + /***** READER SECTION END *****/
> head->read_eof = done;
> return 0;
> }
> @@ -1144,6 +1150,7 @@ static bool tomoyo_is_policy_manager(voi
> return true;
> if (!tomoyo_manage_by_non_root && (task->cred->uid || task->cred->euid))
> return false;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_policy_manager_list_lock);
> list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
> if (!ptr->is_deleted && ptr->is_domain
> @@ -1153,11 +1160,13 @@ static bool tomoyo_is_policy_manager(voi
> }
> }
> up_read(&tomoyo_policy_manager_list_lock);
> + /***** READER SECTION END *****/
> if (found)
> return true;
> exe = tomoyo_get_exe();
> if (!exe)
> return false;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_policy_manager_list_lock);
> list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
> if (!ptr->is_deleted && !ptr->is_domain
> @@ -1167,6 +1176,7 @@ static bool tomoyo_is_policy_manager(voi
> }
> }
> up_read(&tomoyo_policy_manager_list_lock);
> + /***** READER SECTION END *****/
> if (!found) { /* Reduce error messages. */
> static pid_t last_pid;
> const pid_t pid = current->pid;
> @@ -1205,9 +1215,11 @@ static bool tomoyo_is_select_one(struct
> /***** CRITICAL SECTION END *****/
> } else if (!strncmp(data, "domain=", 7)) {
> if (tomoyo_is_domain_def(data + 7)) {
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> domain = tomoyo_find_domain(data + 7);
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> }
> } else
> return false;
> @@ -1222,6 +1234,7 @@ static bool tomoyo_is_select_one(struct
> if (domain) {
> struct tomoyo_domain_info *d;
> head->read_var1 = NULL;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> list_for_each_entry(d, &tomoyo_domain_list, list) {
> if (d == domain)
> @@ -1229,6 +1242,7 @@ static bool tomoyo_is_select_one(struct
> head->read_var1 = &d->list;
> }
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> head->read_var2 = NULL;
> head->read_bit = 0;
> head->read_step = 0;
> @@ -1267,9 +1281,11 @@ static int tomoyo_write_domain_policy(st
> if (is_delete)
> tomoyo_delete_domain(data);
> else if (is_select) {
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> domain = tomoyo_find_domain(data);
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> } else
> domain = tomoyo_find_or_assign_new_domain(data, 0);
> head->write_var1 = domain;
> @@ -1426,6 +1442,7 @@ static int tomoyo_read_domain_policy(str
> return 0;
> if (head->read_step == 0)
> head->read_step = 1;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> list_for_each_cookie(dpos, head->read_var1, &tomoyo_domain_list) {
> struct tomoyo_domain_info *domain;
> @@ -1460,6 +1477,7 @@ acl_loop:
> if (head->read_step == 3)
> goto tail_mark;
> /* Print ACL entries in the domain. */
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_acl_info_list_lock);
> list_for_each_cookie(apos, head->read_var2,
> &domain->acl_info_list) {
> @@ -1472,6 +1490,7 @@ acl_loop:
> }
> }
> up_read(&tomoyo_domain_acl_info_list_lock);
> + /***** READER SECTION END *****/
> if (!done)
> break;
> head->read_step = 3;
> @@ -1485,6 +1504,7 @@ tail_mark:
> break;
> }
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> head->read_eof = done;
> return 0;
> }
> @@ -1511,9 +1531,11 @@ static int tomoyo_write_domain_profile(s
> if (!cp)
> return -EINVAL;
> *cp = '\0';
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> domain = tomoyo_find_domain(cp + 1);
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> if (strict_strtoul(data, 10, &profile))
> return -EINVAL;
> if (domain && profile < TOMOYO_MAX_PROFILES
> @@ -1543,6 +1565,7 @@ static int tomoyo_read_domain_profile(st
>
> if (head->read_eof)
> return 0;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> list_for_each_cookie(pos, head->read_var1, &tomoyo_domain_list) {
> struct tomoyo_domain_info *domain;
> @@ -1556,6 +1579,7 @@ static int tomoyo_read_domain_profile(st
> }
> }
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> head->read_eof = done;
> return 0;
> }
> @@ -1777,6 +1801,7 @@ void tomoyo_load_policy(const char *file
> tomoyo_policy_loaded = true;
> { /* Check all profiles currently assigned to domains are defined. */
> struct tomoyo_domain_info *domain;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> list_for_each_entry(domain, &tomoyo_domain_list, list) {
> const u8 profile = domain->profile;
> @@ -1786,6 +1811,7 @@ void tomoyo_load_policy(const char *file
> profile, domain->domainname->name);
> }
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> }
> }
>
> --- security-testing-2.6.git.orig/security/tomoyo/domain.c
> +++ security-testing-2.6.git/security/tomoyo/domain.c
> @@ -137,7 +137,7 @@ static int tomoyo_update_domain_initiali
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_domain_initializer_list_lock);
> list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
> if (ptr->is_not != is_not ||
> @@ -159,7 +159,7 @@ static int tomoyo_update_domain_initiali
> error = 0;
> }
> up_write(&tomoyo_domain_initializer_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -176,6 +176,7 @@ bool tomoyo_read_domain_initializer_poli
> struct list_head *pos;
> bool done = true;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_initializer_list_lock);
> list_for_each_cookie(pos, head->read_var2,
> &tomoyo_domain_initializer_list) {
> @@ -201,6 +202,7 @@ bool tomoyo_read_domain_initializer_poli
> }
> }
> up_read(&tomoyo_domain_initializer_list_lock);
> + /***** READER SECTION END *****/
> return done;
> }
>
> @@ -247,6 +249,7 @@ static bool tomoyo_is_domain_initializer
> struct tomoyo_domain_initializer_entry *ptr;
> bool flag = false;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_initializer_list_lock);
> list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
> if (ptr->is_deleted)
> @@ -269,6 +272,7 @@ static bool tomoyo_is_domain_initializer
> flag = true;
> }
> up_read(&tomoyo_domain_initializer_list_lock);
> + /***** READER SECTION END *****/
> return flag;
> }
>
> @@ -316,7 +320,7 @@ static int tomoyo_update_domain_keeper_e
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_domain_keeper_list_lock);
> list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
> if (ptr->is_not != is_not ||
> @@ -337,7 +341,7 @@ static int tomoyo_update_domain_keeper_e
> error = 0;
> }
> up_write(&tomoyo_domain_keeper_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -375,6 +379,7 @@ bool tomoyo_read_domain_keeper_policy(st
> struct list_head *pos;
> bool done = true;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_keeper_list_lock);
> list_for_each_cookie(pos, head->read_var2,
> &tomoyo_domain_keeper_list) {
> @@ -400,6 +405,7 @@ bool tomoyo_read_domain_keeper_policy(st
> }
> }
> up_read(&tomoyo_domain_keeper_list_lock);
> + /***** READER SECTION END *****/
> return done;
> }
>
> @@ -420,6 +426,7 @@ static bool tomoyo_is_domain_keeper(cons
> struct tomoyo_domain_keeper_entry *ptr;
> bool flag = false;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_keeper_list_lock);
> list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
> if (ptr->is_deleted)
> @@ -440,6 +447,7 @@ static bool tomoyo_is_domain_keeper(cons
> flag = true;
> }
> up_read(&tomoyo_domain_keeper_list_lock);
> + /***** READER SECTION END *****/
> return flag;
> }
>
> @@ -475,7 +483,7 @@ static int tomoyo_update_alias_entry(con
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_alias_list_lock);
> list_for_each_entry(ptr, &tomoyo_alias_list, list) {
> if (ptr->original_name != saved_original_name ||
> @@ -493,7 +501,7 @@ static int tomoyo_update_alias_entry(con
> error = 0;
> }
> up_write(&tomoyo_alias_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -510,6 +518,7 @@ bool tomoyo_read_alias_policy(struct tom
> struct list_head *pos;
> bool done = true;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_alias_list_lock);
> list_for_each_cookie(pos, head->read_var2, &tomoyo_alias_list) {
> struct tomoyo_alias_entry *ptr;
> @@ -525,6 +534,7 @@ bool tomoyo_read_alias_policy(struct tom
> }
> }
> up_read(&tomoyo_alias_list_lock);
> + /***** READER SECTION END *****/
> return done;
> }
>
> @@ -562,7 +572,7 @@ int tomoyo_delete_domain(char *domainnam
>
> name.name = domainname;
> tomoyo_fill_path_info(&name);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_domain_list_lock);
> /* Is there an active domain? */
> list_for_each_entry(domain, &tomoyo_domain_list, list) {
> @@ -576,7 +586,7 @@ int tomoyo_delete_domain(char *domainnam
> break;
> }
> up_write(&tomoyo_domain_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> return 0;
> }
>
> @@ -602,7 +612,7 @@ struct tomoyo_domain_info *tomoyo_find_o
> if (!saved_domainname)
> return NULL;
> new_domain = kmalloc(sizeof(*new_domain), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_domain_list_lock);
> domain = tomoyo_find_domain(domainname);
> if (domain)
> @@ -649,7 +659,7 @@ struct tomoyo_domain_info *tomoyo_find_o
> }
> out:
> up_write(&tomoyo_domain_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_domain);
> return domain;
> }
> @@ -722,6 +732,7 @@ int tomoyo_find_next_domain(struct linux
> if (tomoyo_pathcmp(&r, &s)) {
> struct tomoyo_alias_entry *ptr;
> /* Is this program allowed to be called via symbolic links? */
> + /***** READER SECTION START *****/
> down_read(&tomoyo_alias_list_lock);
> list_for_each_entry(ptr, &tomoyo_alias_list, list) {
> if (ptr->is_deleted ||
> @@ -735,6 +746,7 @@ int tomoyo_find_next_domain(struct linux
> break;
> }
> up_read(&tomoyo_alias_list_lock);
> + /***** READER SECTION END *****/
> }
>
> /* Check execute permission. */
> @@ -765,9 +777,11 @@ int tomoyo_find_next_domain(struct linux
> }
> if (domain || strlen(new_domain_name) >= TOMOYO_MAX_PATHNAME_LEN)
> goto done;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> domain = tomoyo_find_domain(new_domain_name);
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> if (domain)
> goto done;
> if (is_enforce)
> --- security-testing-2.6.git.orig/security/tomoyo/file.c
> +++ security-testing-2.6.git/security/tomoyo/file.c
> @@ -168,7 +168,7 @@ static int tomoyo_update_globally_readab
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_globally_readable_list_lock);
> list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
> if (ptr->filename != saved_filename)
> @@ -184,7 +184,7 @@ static int tomoyo_update_globally_readab
> error = 0;
> }
> up_write(&tomoyo_globally_readable_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -201,6 +201,7 @@ static bool tomoyo_is_globally_readable_
> {
> struct tomoyo_globally_readable_file_entry *ptr;
> bool found = false;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_globally_readable_list_lock);
> list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
> if (!ptr->is_deleted &&
> @@ -210,6 +211,7 @@ static bool tomoyo_is_globally_readable_
> }
> }
> up_read(&tomoyo_globally_readable_list_lock);
> + /***** READER SECTION END *****/
> return found;
> }
>
> @@ -238,6 +240,7 @@ bool tomoyo_read_globally_readable_polic
> struct list_head *pos;
> bool done = true;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_globally_readable_list_lock);
> list_for_each_cookie(pos, head->read_var2,
> &tomoyo_globally_readable_list) {
> @@ -254,6 +257,7 @@ bool tomoyo_read_globally_readable_polic
> }
> }
> up_read(&tomoyo_globally_readable_list_lock);
> + /***** READER SECTION END *****/
> return done;
> }
>
> @@ -284,7 +288,7 @@ static int tomoyo_update_file_pattern_en
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_pattern_list_lock);
> list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
> if (saved_pattern != ptr->pattern)
> @@ -300,7 +304,7 @@ static int tomoyo_update_file_pattern_en
> error = 0;
> }
> up_write(&tomoyo_pattern_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -318,6 +322,7 @@ tomoyo_get_file_pattern(const struct tom
> struct tomoyo_pattern_entry *ptr;
> const struct tomoyo_path_info *pattern = NULL;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_pattern_list_lock);
> list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
> if (ptr->is_deleted)
> @@ -333,6 +338,7 @@ tomoyo_get_file_pattern(const struct tom
> }
> }
> up_read(&tomoyo_pattern_list_lock);
> + /***** READER SECTION END *****/
> if (pattern)
> filename = pattern;
> return filename;
> @@ -363,6 +369,7 @@ bool tomoyo_read_file_pattern(struct tom
> struct list_head *pos;
> bool done = true;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_pattern_list_lock);
> list_for_each_cookie(pos, head->read_var2, &tomoyo_pattern_list) {
> struct tomoyo_pattern_entry *ptr;
> @@ -376,6 +383,7 @@ bool tomoyo_read_file_pattern(struct tom
> }
> }
> up_read(&tomoyo_pattern_list_lock);
> + /***** READER SECTION END *****/
> return done;
> }
>
> @@ -406,7 +414,7 @@ static int tomoyo_update_no_rewrite_entr
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_no_rewrite_list_lock);
> list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
> if (ptr->pattern != saved_pattern)
> @@ -422,7 +430,7 @@ static int tomoyo_update_no_rewrite_entr
> error = 0;
> }
> up_write(&tomoyo_no_rewrite_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -440,6 +448,7 @@ static bool tomoyo_is_no_rewrite_file(co
> struct tomoyo_no_rewrite_entry *ptr;
> bool found = false;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_no_rewrite_list_lock);
> list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
> if (ptr->is_deleted)
> @@ -450,6 +459,7 @@ static bool tomoyo_is_no_rewrite_file(co
> break;
> }
> up_read(&tomoyo_no_rewrite_list_lock);
> + /***** READER SECTION END *****/
> return found;
> }
>
> @@ -478,6 +488,7 @@ bool tomoyo_read_no_rewrite_policy(struc
> struct list_head *pos;
> bool done = true;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_no_rewrite_list_lock);
> list_for_each_cookie(pos, head->read_var2, &tomoyo_no_rewrite_list) {
> struct tomoyo_no_rewrite_entry *ptr;
> @@ -491,6 +502,7 @@ bool tomoyo_read_no_rewrite_policy(struc
> }
> }
> up_read(&tomoyo_no_rewrite_list_lock);
> + /***** READER SECTION END *****/
> return done;
> }
>
> @@ -556,6 +568,7 @@ static int tomoyo_check_single_path_acl2
> struct tomoyo_acl_info *ptr;
> int error = -EPERM;
>
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_acl_info_list_lock);
> list_for_each_entry(ptr, &domain->acl_info_list, list) {
> struct tomoyo_single_path_acl_record *acl;
> @@ -576,6 +589,7 @@ static int tomoyo_check_single_path_acl2
> break;
> }
> up_read(&tomoyo_domain_acl_info_list_lock);
> + /***** READER SECTION END *****/
> return error;
> }
>
> @@ -742,7 +756,7 @@ static int tomoyo_update_single_path_acl
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_domain_acl_info_list_lock);
> if (is_delete)
> goto delete;
> @@ -799,7 +813,7 @@ static int tomoyo_update_single_path_acl
> }
> out:
> up_write(&tomoyo_domain_acl_info_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -838,7 +852,7 @@ static int tomoyo_update_double_path_acl
> return -ENOMEM;
> if (!is_delete)
> new_entry = kmalloc(sizeof(*new_entry), GFP_KERNEL);
> - /***** EXCLUSIVE SECTION START *****/
> + /***** WRITER SECTION START *****/
> down_write(&tomoyo_domain_acl_info_list_lock);
> if (is_delete)
> goto delete;
> @@ -888,7 +902,7 @@ static int tomoyo_update_double_path_acl
> }
> out:
> up_write(&tomoyo_domain_acl_info_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> + /***** WRITER SECTION END *****/
> kfree(new_entry);
> return error;
> }
> @@ -934,6 +948,7 @@ static int tomoyo_check_double_path_acl(
>
> if (!tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE))
> return 0;
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_acl_info_list_lock);
> list_for_each_entry(ptr, &domain->acl_info_list, list) {
> struct tomoyo_double_path_acl_record *acl;
> @@ -951,6 +966,7 @@ static int tomoyo_check_double_path_acl(
> break;
> }
> up_read(&tomoyo_domain_acl_info_list_lock);
> + /***** READER SECTION END *****/
> return error;
> }
>
> --- security-testing-2.6.git.orig/security/tomoyo/realpath.c
> +++ security-testing-2.6.git/security/tomoyo/realpath.c
> @@ -326,10 +326,12 @@ void __init tomoyo_realpath_init(void)
> INIT_LIST_HEAD(&tomoyo_kernel_domain.acl_info_list);
> tomoyo_kernel_domain.domainname = tomoyo_save_name(TOMOYO_ROOT_NAME);
> list_add_tail(&tomoyo_kernel_domain.list, &tomoyo_domain_list);
> + /***** READER SECTION START *****/
> down_read(&tomoyo_domain_list_lock);
> if (tomoyo_find_domain(TOMOYO_ROOT_NAME) != &tomoyo_kernel_domain)
> panic("Can't register tomoyo_kernel_domain");
> up_read(&tomoyo_domain_list_lock);
> + /***** READER SECTION END *****/
> }
>
> /* Memory allocated for temporary purpose. */

2009-06-02 02:30:56

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

Li Zefan wrote:
> Tetsuo Handa wrote:
> > Enclose reader section in
> > /***** READER SECTION START *****/
> > and
> > /***** READER SECTION END *****/
> > and writer section in
> > /***** WRITER SECTION START *****/
> > and
> > /***** WRITER SECTION END *****/
> > in order to avoid oversighting lock protected section.
> >
>
> This makes me a bit uncomfortable..
>
> IMHO this seems ugly, useless, and even harmful. If it's helpful,
> we'd be doing this for the whole kernel tree, which is crazy..
>
> Or does tomoyo do this for it's special reason?

I intended to help reviewers to visualize the range of protected section
at a glance. But if reviewers feel noisy, I can remove these markers.

Thanks.

2009-06-02 02:35:59

by Li Zefan

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

Tetsuo Handa wrote:
> Li Zefan wrote:
>> Tetsuo Handa wrote:
>>> Enclose reader section in
>>> /***** READER SECTION START *****/
>>> and
>>> /***** READER SECTION END *****/
>>> and writer section in
>>> /***** WRITER SECTION START *****/
>>> and
>>> /***** WRITER SECTION END *****/
>>> in order to avoid oversighting lock protected section.
>>>
>> This makes me a bit uncomfortable..
>>
>> IMHO this seems ugly, useless, and even harmful. If it's helpful,
>> we'd be doing this for the whole kernel tree, which is crazy..
>>
>> Or does tomoyo do this for it's special reason?
>
> I intended to help reviewers to visualize the range of protected section
> at a glance. But if reviewers feel noisy, I can remove these markers.
>

I don't think it help review, but instead those '*'+UPPERCASE comments
are disturbing.


2009-06-02 02:37:24

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

Quoting Tetsuo Handa ([email protected]):
> Li Zefan wrote:
> > Tetsuo Handa wrote:
> > > Enclose reader section in
> > > /***** READER SECTION START *****/
> > > and
> > > /***** READER SECTION END *****/
> > > and writer section in
> > > /***** WRITER SECTION START *****/
> > > and
> > > /***** WRITER SECTION END *****/
> > > in order to avoid oversighting lock protected section.
> > >
> >
> > This makes me a bit uncomfortable..
> >
> > IMHO this seems ugly, useless, and even harmful. If it's helpful,
> > we'd be doing this for the whole kernel tree, which is crazy..
> >
> > Or does tomoyo do this for it's special reason?
>
> I intended to help reviewers to visualize the range of protected section
> at a glance. But if reviewers feel noisy, I can remove these markers.

No, the real problem is that you have "protected sections" at all.

You should be locking data, not code. (Apparently a quote to be attributed
to Alan Cox - huh)

See the bottom of:
http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/x376.html

-serge

2009-06-02 03:42:59

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

Serge E. Hallyn wrote:
> Again I feel (no offense) like I'm reading Ada code here...
I don't know much about the Linux kernel's way of coding.
I'm making a lot of out-of-conventional coding.
Please mention without hesitating.

> 1. the mutex lock belonging to this function really is just protecting
> writes to elements of tomoyo_profile_ptr. It should be defined,
> with a descriptive name and comment, next to tomoyo_profile_ptr
> at common.c:46.

(1) Declare a variable and a lock for that variable together.

I've heard (1) in the past.

On the other hand, I think there is another rule

(2) Declare variables inside a function if they are used only within
that function.

If I bring the declaration of the lock to outside the function, it widens
the scope of the lock.

But Linux kernel's way is to follow (1), isn't it?

> 2. I see no reason for this not to be a fast spinlock at this
> point.

I'm thinking that
(1) If I use mutex and rw_semaphore, the CPU which is waiting for the lock
can spend it's power for doing other process's jobs.
(2) If I use spinlock, the CPU's power is merely wasted, even though
the CPU can spend it's power for doing other process's jobs.
and therefore I'm using mutex and rw_semaphore if sleeping is permitted.

Should I use spinlock rather than mutex and rw_semaphore whenever possible?

> 3. Once it's a fast checkpoint, you can change the flow a bit
> (unless there is good reason not to) to do:
Indeed.

Thanks.

2009-06-02 05:09:06

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

On Tue, 2 Jun 2009, Tetsuo Handa wrote:

> I intended to help reviewers to visualize the range of protected section
> at a glance. But if reviewers feel noisy, I can remove these markers.

The lock functions act as annotations, you don't need to add any more.

Only add comments if the code is not obvious, but also first ask whether
this might mean that there's something wrong.

--
James Morris
<[email protected]>

2009-06-02 05:24:24

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

James Morris wrote:
> The lock functions act as annotations, you don't need to add any more.

I see. I'll remake patchset.

Thanks.
--------------------
Subject: TOMOYO: Remove redundant markers.

Remove '/***** START/STOP *****/' markers.

Signed-off-by: Tetsuo Handa <[email protected]>
---
security/tomoyo/common.c | 8 --------
security/tomoyo/domain.c | 14 --------------
security/tomoyo/file.c | 10 ----------
security/tomoyo/realpath.c | 4 ----
4 files changed, 36 deletions(-)

--- security-testing-2.6.git.orig/security/tomoyo/common.c
+++ security-testing-2.6.git/security/tomoyo/common.c
@@ -866,7 +866,6 @@ static struct tomoyo_profile *tomoyo_fin

if (profile >= TOMOYO_MAX_PROFILES)
return NULL;
- /***** EXCLUSIVE SECTION START *****/
mutex_lock(&lock);
ptr = tomoyo_profile_ptr[profile];
if (ptr)
@@ -880,7 +879,6 @@ static struct tomoyo_profile *tomoyo_fin
tomoyo_profile_ptr[profile] = ptr;
ok:
mutex_unlock(&lock);
- /***** EXCLUSIVE SECTION END *****/
return ptr;
}

@@ -1050,7 +1048,6 @@ static int tomoyo_update_manager_entry(c
saved_manager = tomoyo_save_name(manager);
if (!saved_manager)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_policy_manager_list_lock);
list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
if (ptr->manager != saved_manager)
@@ -1072,7 +1069,6 @@ static int tomoyo_update_manager_entry(c
error = 0;
out:
up_write(&tomoyo_policy_manager_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -1197,13 +1193,11 @@ static bool tomoyo_is_select_one(struct

if (sscanf(data, "pid=%u", &pid) == 1) {
struct task_struct *p;
- /***** CRITICAL SECTION START *****/
read_lock(&tasklist_lock);
p = find_task_by_vpid(pid);
if (p)
domain = tomoyo_real_domain(p);
read_unlock(&tasklist_lock);
- /***** CRITICAL SECTION END *****/
} else if (!strncmp(data, "domain=", 7)) {
if (tomoyo_is_domain_def(data + 7)) {
down_read(&tomoyo_domain_list_lock);
@@ -1594,13 +1588,11 @@ static int tomoyo_read_pid(struct tomoyo
const int pid = head->read_step;
struct task_struct *p;
struct tomoyo_domain_info *domain = NULL;
- /***** CRITICAL SECTION START *****/
read_lock(&tasklist_lock);
p = find_task_by_vpid(pid);
if (p)
domain = tomoyo_real_domain(p);
read_unlock(&tasklist_lock);
- /***** CRITICAL SECTION END *****/
if (domain)
tomoyo_io_printf(head, "%d %u %s", pid, domain->profile,
domain->domainname->name);
--- security-testing-2.6.git.orig/security/tomoyo/domain.c
+++ security-testing-2.6.git/security/tomoyo/domain.c
@@ -67,14 +67,12 @@ void tomoyo_set_domain_flag(struct tomoy
{
/* We need to serialize because this is bitfield operation. */
static DEFINE_SPINLOCK(lock);
- /***** CRITICAL SECTION START *****/
spin_lock(&lock);
if (!is_delete)
domain->flags |= flags;
else
domain->flags &= ~flags;
spin_unlock(&lock);
- /***** CRITICAL SECTION END *****/
}

/**
@@ -135,7 +133,6 @@ static int tomoyo_update_domain_initiali
saved_program = tomoyo_save_name(program);
if (!saved_program)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_initializer_list_lock);
list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_not != is_not ||
@@ -161,7 +158,6 @@ static int tomoyo_update_domain_initiali
error = 0;
out:
up_write(&tomoyo_domain_initializer_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -315,7 +311,6 @@ static int tomoyo_update_domain_keeper_e
saved_domainname = tomoyo_save_name(domainname);
if (!saved_domainname)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_keeper_list_lock);
list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_not != is_not ||
@@ -341,7 +336,6 @@ static int tomoyo_update_domain_keeper_e
error = 0;
out:
up_write(&tomoyo_domain_keeper_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -476,7 +470,6 @@ static int tomoyo_update_alias_entry(con
saved_aliased_name = tomoyo_save_name(aliased_name);
if (!saved_original_name || !saved_aliased_name)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_alias_list_lock);
list_for_each_entry(ptr, &tomoyo_alias_list, list) {
if (ptr->original_name != saved_original_name ||
@@ -499,7 +492,6 @@ static int tomoyo_update_alias_entry(con
error = 0;
out:
up_write(&tomoyo_alias_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -567,7 +559,6 @@ int tomoyo_delete_domain(char *domainnam

name.name = domainname;
tomoyo_fill_path_info(&name);
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_list_lock);
/* Is there an active domain? */
list_for_each_entry(domain, &tomoyo_domain_list, list) {
@@ -581,7 +572,6 @@ int tomoyo_delete_domain(char *domainnam
break;
}
up_write(&tomoyo_domain_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return 0;
}

@@ -600,7 +590,6 @@ struct tomoyo_domain_info *tomoyo_find_o
struct tomoyo_domain_info *domain = NULL;
const struct tomoyo_path_info *saved_domainname;

- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(domainname);
if (domain)
@@ -619,7 +608,6 @@ struct tomoyo_domain_info *tomoyo_find_o
domain->domainname != saved_domainname)
continue;
flag = false;
- /***** CRITICAL SECTION START *****/
read_lock(&tasklist_lock);
for_each_process(p) {
if (tomoyo_real_domain(p) != domain)
@@ -628,7 +616,6 @@ struct tomoyo_domain_info *tomoyo_find_o
break;
}
read_unlock(&tasklist_lock);
- /***** CRITICAL SECTION END *****/
if (flag)
continue;
list_for_each_entry(ptr, &domain->acl_info_list, list) {
@@ -651,7 +638,6 @@ struct tomoyo_domain_info *tomoyo_find_o
}
out:
up_write(&tomoyo_domain_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return domain;
}

--- security-testing-2.6.git.orig/security/tomoyo/file.c
+++ security-testing-2.6.git/security/tomoyo/file.c
@@ -166,7 +166,6 @@ static int tomoyo_update_globally_readab
saved_filename = tomoyo_save_name(filename);
if (!saved_filename)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_globally_readable_list_lock);
list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
if (ptr->filename != saved_filename)
@@ -187,7 +186,6 @@ static int tomoyo_update_globally_readab
error = 0;
out:
up_write(&tomoyo_globally_readable_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -284,7 +282,6 @@ static int tomoyo_update_file_pattern_en
saved_pattern = tomoyo_save_name(pattern);
if (!saved_pattern)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_pattern_list_lock);
list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
if (saved_pattern != ptr->pattern)
@@ -305,7 +302,6 @@ static int tomoyo_update_file_pattern_en
error = 0;
out:
up_write(&tomoyo_pattern_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -407,7 +403,6 @@ static int tomoyo_update_no_rewrite_entr
saved_pattern = tomoyo_save_name(pattern);
if (!saved_pattern)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_no_rewrite_list_lock);
list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->pattern != saved_pattern)
@@ -428,7 +423,6 @@ static int tomoyo_update_no_rewrite_entr
error = 0;
out:
up_write(&tomoyo_no_rewrite_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -745,7 +739,6 @@ static int tomoyo_update_single_path_acl
saved_filename = tomoyo_save_name(filename);
if (!saved_filename)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
@@ -800,7 +793,6 @@ static int tomoyo_update_single_path_acl
}
out:
up_write(&tomoyo_domain_acl_info_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

@@ -836,7 +828,6 @@ static int tomoyo_update_double_path_acl
saved_filename2 = tomoyo_save_name(filename2);
if (!saved_filename1 || !saved_filename2)
return -ENOMEM;
- /***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
@@ -884,7 +875,6 @@ static int tomoyo_update_double_path_acl
}
out:
up_write(&tomoyo_domain_acl_info_list_lock);
- /***** EXCLUSIVE SECTION END *****/
return error;
}

--- security-testing-2.6.git.orig/security/tomoyo/realpath.c
+++ security-testing-2.6.git/security/tomoyo/realpath.c
@@ -220,7 +220,6 @@ void *tomoyo_alloc_element(const unsigne
= roundup(size, max(sizeof(void *), sizeof(long)));
if (word_aligned_size > PATH_MAX)
return NULL;
- /***** EXCLUSIVE SECTION START *****/
mutex_lock(&lock);
if (buf_used_len + word_aligned_size > PATH_MAX) {
if (!tomoyo_quota_for_elements ||
@@ -251,7 +250,6 @@ void *tomoyo_alloc_element(const unsigne
}
}
mutex_unlock(&lock);
- /***** EXCLUSIVE SECTION END *****/
return ptr;
}

@@ -318,7 +316,6 @@ const struct tomoyo_path_info *tomoyo_sa
return NULL;
}
hash = full_name_hash((const unsigned char *) name, len - 1);
- /***** EXCLUSIVE SECTION START *****/
mutex_lock(&lock);
list_for_each_entry(ptr, &tomoyo_name_list[hash % TOMOYO_MAX_HASH],
list) {
@@ -366,7 +363,6 @@ const struct tomoyo_path_info *tomoyo_sa
}
out:
mutex_unlock(&lock);
- /***** EXCLUSIVE SECTION END *****/
return ptr ? &ptr->entry : NULL;
}

2009-06-02 21:51:27

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section.

On Tue, 2 Jun 2009, Tetsuo Handa wrote:

> James Morris wrote:
> > The lock functions act as annotations, you don't need to add any more.
>
> I see. I'll remake patchset.
>
> Thanks.

Please send new or updates patches as new self-contained emails.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next



> --------------------
> Subject: TOMOYO: Remove redundant markers.
>
> Remove '/***** START/STOP *****/' markers.
>
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> security/tomoyo/common.c | 8 --------
> security/tomoyo/domain.c | 14 --------------
> security/tomoyo/file.c | 10 ----------
> security/tomoyo/realpath.c | 4 ----
> 4 files changed, 36 deletions(-)
>
> --- security-testing-2.6.git.orig/security/tomoyo/common.c
> +++ security-testing-2.6.git/security/tomoyo/common.c
> @@ -866,7 +866,6 @@ static struct tomoyo_profile *tomoyo_fin
>
> if (profile >= TOMOYO_MAX_PROFILES)
> return NULL;
> - /***** EXCLUSIVE SECTION START *****/
> mutex_lock(&lock);
> ptr = tomoyo_profile_ptr[profile];
> if (ptr)
> @@ -880,7 +879,6 @@ static struct tomoyo_profile *tomoyo_fin
> tomoyo_profile_ptr[profile] = ptr;
> ok:
> mutex_unlock(&lock);
> - /***** EXCLUSIVE SECTION END *****/
> return ptr;
> }
>
> @@ -1050,7 +1048,6 @@ static int tomoyo_update_manager_entry(c
> saved_manager = tomoyo_save_name(manager);
> if (!saved_manager)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_policy_manager_list_lock);
> list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
> if (ptr->manager != saved_manager)
> @@ -1072,7 +1069,6 @@ static int tomoyo_update_manager_entry(c
> error = 0;
> out:
> up_write(&tomoyo_policy_manager_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -1197,13 +1193,11 @@ static bool tomoyo_is_select_one(struct
>
> if (sscanf(data, "pid=%u", &pid) == 1) {
> struct task_struct *p;
> - /***** CRITICAL SECTION START *****/
> read_lock(&tasklist_lock);
> p = find_task_by_vpid(pid);
> if (p)
> domain = tomoyo_real_domain(p);
> read_unlock(&tasklist_lock);
> - /***** CRITICAL SECTION END *****/
> } else if (!strncmp(data, "domain=", 7)) {
> if (tomoyo_is_domain_def(data + 7)) {
> down_read(&tomoyo_domain_list_lock);
> @@ -1594,13 +1588,11 @@ static int tomoyo_read_pid(struct tomoyo
> const int pid = head->read_step;
> struct task_struct *p;
> struct tomoyo_domain_info *domain = NULL;
> - /***** CRITICAL SECTION START *****/
> read_lock(&tasklist_lock);
> p = find_task_by_vpid(pid);
> if (p)
> domain = tomoyo_real_domain(p);
> read_unlock(&tasklist_lock);
> - /***** CRITICAL SECTION END *****/
> if (domain)
> tomoyo_io_printf(head, "%d %u %s", pid, domain->profile,
> domain->domainname->name);
> --- security-testing-2.6.git.orig/security/tomoyo/domain.c
> +++ security-testing-2.6.git/security/tomoyo/domain.c
> @@ -67,14 +67,12 @@ void tomoyo_set_domain_flag(struct tomoy
> {
> /* We need to serialize because this is bitfield operation. */
> static DEFINE_SPINLOCK(lock);
> - /***** CRITICAL SECTION START *****/
> spin_lock(&lock);
> if (!is_delete)
> domain->flags |= flags;
> else
> domain->flags &= ~flags;
> spin_unlock(&lock);
> - /***** CRITICAL SECTION END *****/
> }
>
> /**
> @@ -135,7 +133,6 @@ static int tomoyo_update_domain_initiali
> saved_program = tomoyo_save_name(program);
> if (!saved_program)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_domain_initializer_list_lock);
> list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
> if (ptr->is_not != is_not ||
> @@ -161,7 +158,6 @@ static int tomoyo_update_domain_initiali
> error = 0;
> out:
> up_write(&tomoyo_domain_initializer_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -315,7 +311,6 @@ static int tomoyo_update_domain_keeper_e
> saved_domainname = tomoyo_save_name(domainname);
> if (!saved_domainname)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_domain_keeper_list_lock);
> list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
> if (ptr->is_not != is_not ||
> @@ -341,7 +336,6 @@ static int tomoyo_update_domain_keeper_e
> error = 0;
> out:
> up_write(&tomoyo_domain_keeper_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -476,7 +470,6 @@ static int tomoyo_update_alias_entry(con
> saved_aliased_name = tomoyo_save_name(aliased_name);
> if (!saved_original_name || !saved_aliased_name)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_alias_list_lock);
> list_for_each_entry(ptr, &tomoyo_alias_list, list) {
> if (ptr->original_name != saved_original_name ||
> @@ -499,7 +492,6 @@ static int tomoyo_update_alias_entry(con
> error = 0;
> out:
> up_write(&tomoyo_alias_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -567,7 +559,6 @@ int tomoyo_delete_domain(char *domainnam
>
> name.name = domainname;
> tomoyo_fill_path_info(&name);
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_domain_list_lock);
> /* Is there an active domain? */
> list_for_each_entry(domain, &tomoyo_domain_list, list) {
> @@ -581,7 +572,6 @@ int tomoyo_delete_domain(char *domainnam
> break;
> }
> up_write(&tomoyo_domain_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return 0;
> }
>
> @@ -600,7 +590,6 @@ struct tomoyo_domain_info *tomoyo_find_o
> struct tomoyo_domain_info *domain = NULL;
> const struct tomoyo_path_info *saved_domainname;
>
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_domain_list_lock);
> domain = tomoyo_find_domain(domainname);
> if (domain)
> @@ -619,7 +608,6 @@ struct tomoyo_domain_info *tomoyo_find_o
> domain->domainname != saved_domainname)
> continue;
> flag = false;
> - /***** CRITICAL SECTION START *****/
> read_lock(&tasklist_lock);
> for_each_process(p) {
> if (tomoyo_real_domain(p) != domain)
> @@ -628,7 +616,6 @@ struct tomoyo_domain_info *tomoyo_find_o
> break;
> }
> read_unlock(&tasklist_lock);
> - /***** CRITICAL SECTION END *****/
> if (flag)
> continue;
> list_for_each_entry(ptr, &domain->acl_info_list, list) {
> @@ -651,7 +638,6 @@ struct tomoyo_domain_info *tomoyo_find_o
> }
> out:
> up_write(&tomoyo_domain_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return domain;
> }
>
> --- security-testing-2.6.git.orig/security/tomoyo/file.c
> +++ security-testing-2.6.git/security/tomoyo/file.c
> @@ -166,7 +166,6 @@ static int tomoyo_update_globally_readab
> saved_filename = tomoyo_save_name(filename);
> if (!saved_filename)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_globally_readable_list_lock);
> list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
> if (ptr->filename != saved_filename)
> @@ -187,7 +186,6 @@ static int tomoyo_update_globally_readab
> error = 0;
> out:
> up_write(&tomoyo_globally_readable_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -284,7 +282,6 @@ static int tomoyo_update_file_pattern_en
> saved_pattern = tomoyo_save_name(pattern);
> if (!saved_pattern)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_pattern_list_lock);
> list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
> if (saved_pattern != ptr->pattern)
> @@ -305,7 +302,6 @@ static int tomoyo_update_file_pattern_en
> error = 0;
> out:
> up_write(&tomoyo_pattern_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -407,7 +403,6 @@ static int tomoyo_update_no_rewrite_entr
> saved_pattern = tomoyo_save_name(pattern);
> if (!saved_pattern)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_no_rewrite_list_lock);
> list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
> if (ptr->pattern != saved_pattern)
> @@ -428,7 +423,6 @@ static int tomoyo_update_no_rewrite_entr
> error = 0;
> out:
> up_write(&tomoyo_no_rewrite_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -745,7 +739,6 @@ static int tomoyo_update_single_path_acl
> saved_filename = tomoyo_save_name(filename);
> if (!saved_filename)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_domain_acl_info_list_lock);
> if (is_delete)
> goto delete;
> @@ -800,7 +793,6 @@ static int tomoyo_update_single_path_acl
> }
> out:
> up_write(&tomoyo_domain_acl_info_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> @@ -836,7 +828,6 @@ static int tomoyo_update_double_path_acl
> saved_filename2 = tomoyo_save_name(filename2);
> if (!saved_filename1 || !saved_filename2)
> return -ENOMEM;
> - /***** EXCLUSIVE SECTION START *****/
> down_write(&tomoyo_domain_acl_info_list_lock);
> if (is_delete)
> goto delete;
> @@ -884,7 +875,6 @@ static int tomoyo_update_double_path_acl
> }
> out:
> up_write(&tomoyo_domain_acl_info_list_lock);
> - /***** EXCLUSIVE SECTION END *****/
> return error;
> }
>
> --- security-testing-2.6.git.orig/security/tomoyo/realpath.c
> +++ security-testing-2.6.git/security/tomoyo/realpath.c
> @@ -220,7 +220,6 @@ void *tomoyo_alloc_element(const unsigne
> = roundup(size, max(sizeof(void *), sizeof(long)));
> if (word_aligned_size > PATH_MAX)
> return NULL;
> - /***** EXCLUSIVE SECTION START *****/
> mutex_lock(&lock);
> if (buf_used_len + word_aligned_size > PATH_MAX) {
> if (!tomoyo_quota_for_elements ||
> @@ -251,7 +250,6 @@ void *tomoyo_alloc_element(const unsigne
> }
> }
> mutex_unlock(&lock);
> - /***** EXCLUSIVE SECTION END *****/
> return ptr;
> }
>
> @@ -318,7 +316,6 @@ const struct tomoyo_path_info *tomoyo_sa
> return NULL;
> }
> hash = full_name_hash((const unsigned char *) name, len - 1);
> - /***** EXCLUSIVE SECTION START *****/
> mutex_lock(&lock);
> list_for_each_entry(ptr, &tomoyo_name_list[hash % TOMOYO_MAX_HASH],
> list) {
> @@ -366,7 +363,6 @@ const struct tomoyo_path_info *tomoyo_sa
> }
> out:
> mutex_unlock(&lock);
> - /***** EXCLUSIVE SECTION END *****/
> return ptr ? &ptr->entry : NULL;
> }
>
>

--
James Morris
<[email protected]>