Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754795AbZFBVv1 (ORCPT ); Tue, 2 Jun 2009 17:51:27 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753703AbZFBVvU (ORCPT ); Tue, 2 Jun 2009 17:51:20 -0400 Received: from tundra.namei.org ([65.99.196.166]:44136 "EHLO tundra.namei.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753574AbZFBVvS (ORCPT ); Tue, 2 Jun 2009 17:51:18 -0400 Date: Wed, 3 Jun 2009 07:51:13 +1000 (EST) From: James Morris To: Tetsuo Handa cc: lizf@cn.fujitsu.com, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/5] TOMOYO: Clarify lock protected section. In-Reply-To: <200906020523.n525NdJe050896@www262.sakura.ne.jp> Message-ID: References: <200906020143.n521hGGP003698@www262.sakura.ne.jp> <4A248C90.9010904@cn.fujitsu.com> <200906020230.n522UjfJ013581@www262.sakura.ne.jp> <200906020523.n525NdJe050896@www262.sakura.ne.jp> User-Agent: Alpine 2.00 (LRH 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10818 Lines: 330 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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/