Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1264025

Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes

Show all headers | View raw

On Fri 2015-11-06 06:12:47, Josh Poimboeuf wrote:
> On Fri, Nov 06, 2015 at 11:40:55AM +0100, Petr Mladek wrote:
> > On Thu 2015-11-05 15:18:05, Josh Poimboeuf wrote:
> > > Calling set_memory_rw() and set_memory_ro() for every iteration of the
> > > loop in klp_write_object_relocations() is messy, inefficient, and
> > > error-prone.
> > > 
> > > Change all the read-only pages to read-write before the loop and convert
> > > them back to read-only again afterwards.
> > > 
> > > The {un}set_module_core_ro_nx() functions are used to change the
> > > page permissions.  Toggling NX isn't necessary in this case, but it's
> > > not highly performance sensitive code so it should be fine.
> > 
> > Hmm, the name (un)set_module_core_ro_nx() still sounds a bit strange,
> > especially the "ro_nx" suffix.
> 
> > Alternative solution would be to create
> > 
> >    set_module_text_rw()
> >    set_module_text_ro()
> > 
> > There already exists
> > 
> >    set_all_modules_text_rw()
> >    set_all_modules_text_ro()
> > 
> > They modify only the ro/rw flags. IMHO, the name is more descriptive
> > They are used by ftrace for very similar purpose.
> 
> That wouldn't be enough.  Relocations can occur not only in text, but
> also in data.  That includes read-only data.

I see. This just shows how this all is confusing. Or maybe I am just
dumb :-)

> The (un)set_module_core_ro_nx() naming was taken from the names of
> existing module functions (unset_module_{core,init}_ro_nx()).  They
> enable/disable the CONFIG_DEBUG_SET_MODULE_RONX feature on the core part
> of the module.  The name makes sense to me, though I'm certainly open to
> other ideas.

I think that we should not mix

   set_*_ro()
   set_*_rw()

with

   set_*_ro*()
   unset_*_ro*()

naming schemes. What about adding into the public API?

  set_module_ro()
  set_module_rw()

It should modify everything: init, core, text, and data but only
the ro/rw flags.

Sigh, we went quite far from the few lines patch :-/

> > They modify also the init section. But we might want to touch it
> > as well. klp_module_notify() is called too late now. But once we
> > have a more complex consistency model, we will need to reject
> > the module when the patching fails. We will need to call the
> > livepatch init earlier, close to ftrace_module_init(mod).
> > Then the init section might be interesting as well.
> 
> Init section functions don't have the __fentry() call, so they can't be
> patched.  If that were to change in the future, we could use the
> (un)set_module_init_ro_nx() functions, which already exist.

I see.

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/3] CONFIG_DEBUG_SET_MODULE_RONX bug fix and cleanups Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-05 22:20 +0100
  [PATCH v2 3/3] livepatch: Cleanup module page permission changes Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-05 22:20 +0100
    Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes Petr Mladek <pmladek@suse.com> - 2015-11-06 11:50 +0100
      Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-06 13:20 +0100
        Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes Petr Mladek <pmladek@suse.com> - 2015-11-06 14:50 +0100
          Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-06 18:20 +0100
            Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes Rusty Russell <rusty@rustcorp.com.au> - 2015-11-09 00:10 +0100
  [PATCH v2 1/3] livepatch: Fix crash with !CONFIG_DEBUG_SET_MODULE_RONX Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-05 22:30 +0100
    Re: [PATCH v2 1/3] livepatch: Fix crash with  !CONFIG_DEBUG_SET_MODULE_RONX Jiri Kosina <jikos@kernel.org> - 2015-11-06 11:20 +0100

csiph-web