Messages in this thread | | | Date | Tue, 1 Oct 2019 14:00:23 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 11/15] static_call: Add inline static call infrastructure |
| |
On Mon, Jun 10, 2019 at 12:19:29PM -0500, Josh Poimboeuf wrote: > On Fri, Jun 07, 2019 at 10:37:56AM +0200, Peter Zijlstra wrote: > > > > +} > > > > + > > > > +static int static_call_module_notify(struct notifier_block *nb, > > > > + unsigned long val, void *data) > > > > +{ > > > > + struct module *mod = data; > > > > + int ret = 0; > > > > + > > > > + cpus_read_lock(); > > > > + static_call_lock(); > > > > + > > > > + switch (val) { > > > > + case MODULE_STATE_COMING: > > > > + module_disable_ro(mod); > > > > + ret = static_call_add_module(mod); > > > > + module_enable_ro(mod, false); > > > > > > Doesn’t it cause some pages to be W+X ? > > How so?
This is after complete_formation() which does RO,X. If we then disable RO we end up with W+X pages, which is bad.
That said, alternatives, ftrace, dynamic_debug all run before complete_formation() specifically such that they can directly poke text.
Possibly we should add a notifier callback for MODULE_STATE_UNFORMED, but that is for another day.
> >> Can it be avoided? > > > > I don't know why it does this, jump_labels doesn't seem to need this, > > and I'm not seeing what static_call needs differently. > > I forgot why I did this, but it's probably for the case where there's a > static call site in module init code. It deserves a comment. > > Theoretically, jump labels need this to. > > BTW, there's a change coming that will require the text_mutex before > calling module_{disable,enable}_ro().
I can't find why it would need this (and I'm going to remove it). Specifically complete_formation() does enable_ro(.after_init=false), which leaves .ro_after_init writable so {jump_label,static_call}_sort_entries() will work.
But both jump_label and static_call then use the full text_poke(), not text_poke_early(), for modules.
| |