On Thu, 17 Jan 2019 18:07:03 +0000 Nadav Amit <namit@xxxxxxxxxx> wrote: > > On Jan 16, 2019, at 11:54 PM, Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote: > > > > On Wed, 16 Jan 2019 16:32:59 -0800 > > Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> wrote: > > > >> From: Nadav Amit <namit@xxxxxxxxxx> > >> > >> 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@xxxxxxxxxx>
