linux-kernel - Re: [PATCH 0/7] livepatch,module: Remove .klp.arch and module_disable

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200415161706.3tw5o4se2cakxmql@treble>
Date:   Wed, 15 Apr 2020 11:17:06 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jessica Yu <jeyu@...nel.org>
Subject: Re: [PATCH 0/7] livepatch,module: Remove .klp.arch and
 module_disable_ro()

On Wed, Apr 15, 2020 at 04:24:15PM +0200, Peter Zijlstra wrote:
> > It bothers me that both the notifiers and the module init() both see the
> > same MODULE_STATE_COMING state, but only in the former case is the text
> > writable.
> > 
> > I think it's cognitively simpler if MODULE_STATE_COMING always means the
> > same thing, like the comments imply, "fully formed" and thus
> > not-writable:
> > 
> > enum module_state {
> > 	MODULE_STATE_LIVE,	/* Normal state. */
> > 	MODULE_STATE_COMING,	/* Full formed, running module_init. */
> > 	MODULE_STATE_GOING,	/* Going away. */
> > 	MODULE_STATE_UNFORMED,	/* Still setting it up. */
> > };
> > 
> > And, it keeps tighter constraints on what a notifier can do, which is a
> > good thing if we can get away with it.
> 
> Moo! -- but jump_label and static_call are on the notifier chain and I
> was hoping to make it cheaper for them. Should we perhaps weane them off the
> notifier and, like ftrace/klp put in explicit calls?
> 
> It'd make the error handling in prepare_coming_module() a bigger mess,
> but it should work.

So you're wanting to have jump labels and static_call do direct writes
instead of text pokes, right?  Makes sense.

I don't feel strongly about "don't let module notifiers modify text".

But I still not a fan of the fact that COMING has two different
"states".  For example, after your patch, when apply_relocate_add() is
called from klp_module_coming(), it can use memcpy(), but when called
from klp module init() it has to use text poke.  But both are COMING so
there's no way to look at the module state to know which can be used.

I hate to say it, but it almost feels like another module state would be
useful.

-- 
Josh