linux-kernel - Re: [PATCH] TOMOYO: Use RCU primitives for list operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091029054029.GE11558@us.ibm.com>
Date:	Thu, 29 Oct 2009 00:40:29 -0500
From:	"Serge E. Hallyn" <serue@...ibm.com>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] TOMOYO: Use RCU primitives for list operation

Quoting Tetsuo Handa (penguin-kernel@...ove.SAKURA.ne.jp):
> [PATCH] TOMOYO: Use RCU primitives for list operation
> 
> Remove down_read()/up_read() by replacing with RCU primitives.
> SRCU based garbage collector will be added in the future.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> ---
>  security/tomoyo/common.c   |   52 ++++++++++-----------------------------------
>  security/tomoyo/common.h   |   14 ++++++------
>  security/tomoyo/domain.c   |   38 ++++++++++----------------------
>  security/tomoyo/file.c     |   50 ++++++++++++++-----------------------------
>  security/tomoyo/realpath.c |    4 ---
>  5 files changed, 49 insertions(+), 109 deletions(-)
> 
> --- security-testing-2.6.orig/security/tomoyo/common.c
> +++ security-testing-2.6/security/tomoyo/common.c
> @@ -365,9 +365,6 @@ bool tomoyo_is_domain_def(const unsigned
>   *
>   * @domainname: The domainname to find.
>   *
> - * Caller must call down_read(&tomoyo_domain_list_lock); or
> - * down_write(&tomoyo_domain_list_lock); .
> - *
>   * Returns pointer to "struct tomoyo_domain_info" if found, NULL otherwise.
>   */
>  struct tomoyo_domain_info *tomoyo_find_domain(const char *domainname)
> @@ -377,7 +374,7 @@ struct tomoyo_domain_info *tomoyo_find_d
> 
>  	name.name = domainname;
>  	tomoyo_fill_path_info(&name);
> -	list_for_each_entry(domain, &tomoyo_domain_list, list) {
> +	list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
>  		if (!domain->is_deleted &&
>  		    !tomoyo_pathcmp(&name, domain->domainname))
>  			return domain;
> @@ -837,8 +834,7 @@ bool tomoyo_domain_quota_is_ok(struct to
> 
>  	if (!domain)
>  		return true;
> -	down_read(&tomoyo_domain_acl_info_list_lock);
> -	list_for_each_entry(ptr, &domain->acl_info_list, list) {
> +	list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
>  		if (ptr->type & TOMOYO_ACL_DELETED)
>  			continue;
>  		switch (tomoyo_acl_type2(ptr)) {

You are removing the down_read()s, but not replacing them with
rcu_read_lock()s.  I assume this is based on the same discussions
you had with Paul awhile ago about the safety of walking the list
bc you only append to the end (which I trust must have concluded
in your favor)?

If you'll be adding gc eventually anyway, is it really worthwhile
to 'violate the rules' now by calling list_for_each_entry_rcu()
without being inside rcu_read_lock() now?  I fear it'll only serve
to confuse readers, especially those looking for rcu users to serve
as examples.

-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/