linux-kernel - Re: [PATCH 1/2] Refine mutex and rcu method in module.c, kernel<3.2.9>

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: <4F5AD7F6.2000509@gmail.com>
Date:	Sat, 10 Mar 2012 12:26:30 +0800
From:	Cong Wang <xiyou.wangcong@...il.com>
To:	"Chen, Dennis (SRDC SW)" <Dennis1.Chen@....com>
CC:	Rusty Russell <rusty@...abs.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH 1/2] Refine mutex and rcu method in module.c, kernel<3.2.9>

On 03/09/2012 12:13 AM, Chen, Dennis (SRDC SW) wrote:
> Hi Rusty,
>
> Pls notice the following change in the patch (in set_all_modules_text_ro function):
>
> /* Iterate through all modules and set each module's text as RO */
> @@ -1693,7 +1699,7 @@
>   {
>          struct module *mod;
>
> -       mutex_lock(&module_mutex);
> +       rcu_read_lock();
>          list_for_each_entry_rcu(mod,&modules, list) {
>                  if ((mod->module_core)&&  (mod->core_text_size)) {
>                          set_page_attributes(mod->module_core,
> @@ -1706,7 +1712,7 @@
>                                                  set_memory_ro);
>                  }
>          }
> -       mutex_unlock(&module_mutex);
> +       rcu_read_unlock();
>   }
>
> This function just needs to iterate the modules list, but now it holds a unnecessary lock when it does that,
> The other module can't be inserted during this operation, also can you make sure the set_page_attributes will
> run smoothly all the time, if not it's a risk action to hold a lock.
> So summary--
> I think the idea for kernel module protection is simple:
> Writers to modules, use mutex_lock
> Readers, use rcu. __ALL__ codes here should be with a unified style! This will make our kernel gracefully.
>
> PS: my comments in the patch " /* Concurrent writers for the global modules list are protected by RCU*/" is not right, RCU
> Should be mutex lock.

I think your change makes sense, I don't know why preempt_disable() was 
used, git blame told me the related two commits are 4 years-old...

cb2a5205 2008-01-14 00:55:03 -0800 3180
d72b3751 2008-08-30 10:09:00 +0200 3181

maybe at that time rcu was not what it is today... Cc'ing Paul.
--
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/