Messages in this thread |  | | Date | Fri, 18 Jan 2019 17:23:34 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH 17/17] module: Prevent module removal racing with text_poke() |
| |
On Thu, 17 Jan 2019 18:07:03 +0000 Nadav Amit <namit@vmware.com> wrote:
> > On Jan 16, 2019, at 11:54 PM, Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > On Wed, 16 Jan 2019 16:32:59 -0800 > > Rick Edgecombe <rick.p.edgecombe@intel.com> wrote: > > > >> From: Nadav Amit <namit@vmware.com> > >> > >> It seems dangerous to allow code modifications to take place > >> concurrently with module unloading. So take the text_mutex while the > >> memory of the module is freed. > > > > At that point, since the module itself is removed from module list, > > it seems no actual harm. Or would you have any concern? > > So it appears that you are right and all the users of text_poke() and > text_poke_bp() do install module notifiers, and remove the module from their > internal data structure when they are done (*). As long as they prevent > text_poke*() to be called concurrently (e.g., using jump_label_lock()), > everything is fine. > > Having said that, the question is whether you “trust” text_poke*() users to > do so. text_poke() description does not day explicitly that you need to > prevent modules from being removed. > > What do you say?
I agreed, but in that case, this is just a fool proof. I think we should prevent this kind of bug by review, and should comment it on text_poke(), instead of locking text_mutex.
What I thought was even if we take text_mutex here, such user can modify the (released) module code right after we exit this section.
Maybe we'd better make text_poke() more smart?
> (*) I am not sure about kgdb, but it probably does not matter much
I think we don't need to care about kgdb. It is a tool which should be able to shoot your feet and we can not prevent it. Only expert can avoid it. :)
Thank you,
-- Masami Hiramatsu <mhiramat@kernel.org>
|  |