From: Joe Lawrence <joe.lawrence@redhat.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>, Petr Mladek <pmladek@suse.com>
Cc: jikos@kernel.org, Miroslav Benes <mbenes@suse.cz>,
linux-kernel@vger.kernel.org, live-patching@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] livepatch: Clear relocation targets on a module removal
Date: Tue, 27 Aug 2019 11:05:51 -0400 [thread overview]
Message-ID: <5efd9f4e-eccb-cbe0-ec2e-0e2a6a34c0ea@redhat.com> (raw)
In-Reply-To: <20190826145449.wyo7avwpqyriem46@treble>
On 8/26/19 10:54 AM, Josh Poimboeuf wrote:
> On Fri, Aug 23, 2019 at 10:13:06AM +0200, Petr Mladek wrote:
>> On Thu 2019-08-22 17:36:49, Josh Poimboeuf wrote:
>>> On Fri, Aug 16, 2019 at 11:46:08AM +0200, Petr Mladek wrote:
>>>> On Wed 2019-08-14 10:12:44, Josh Poimboeuf wrote:
>>>>> On Wed, Aug 14, 2019 at 01:06:09PM +0200, Miroslav Benes wrote:
>>>>>>> Really, we should be going in the opposite direction, by creating module
>>>>>>> dependencies, like all other kernel modules do, ensuring that a module
>>>>>>> is loaded *before* we patch it. That would also eliminate this bug.
>>>>>
>>>>> We should look at whether it makes sense to destabilize live patching
>>>>> for everybody, for a small minority of people who care about a small
>>>>> minority of edge cases.
>>>>
>>>> I do not see it that simple. Forcing livepatched modules to be
>>>> loaded would mean loading "random" new modules when updating
>>>> livepatches:
>>>
>>> I don't want to start a long debate on this, because this idea isn't
>>> even my first choice. But we shouldn't dismiss it outright.
>>
>> I am glad to hear that this is not your first choice.
>>
>>
>>>> + It means more actions and higher risk to destabilize
>>>> the system. Different modules have different quality.
>>>
>>> Maybe the distro shouldn't ship modules which would destabilize the
>>> system.
>>
>> Is this realistic? Even the best QA could not check all scenarios.
>> My point is that the more actions we do the bigger the risk is.
>
> Sure, it introduces risk. But we have to compare that risk (which only
> affects rare edge cases) with the ones introduced by the late module
> patching code. I get the feeling that "late module patching" introduces
> risk to a broader range of use cases than "occasional loading of unused
> modules".
>
> The latter risk could be minimized by introducing a disabled state for
> modules - load it in memory, but don't expose it to users until
> explicitly loaded. Just a brainstormed idea; not sure whether it would
> work in practice.
>
Interesting idea. We would need to ensure consistency between the
loaded-but-not-enabled module and the version on disk. Does module init
run when it's enabled? Etc.
<blue sky ideas>
What about folding this the other way? ie, if a livepatch targets
unloaded module foo, loaded module bar, and vmlinux ... it effectively
patches bar and vmlinux, but the foo changes are dropped.
Responsibility is placed on the admin to install an updated foo before
loading it (in which case, livepatching core will again ignore foo.)
Building on this idea, perhaps loading that livepatch would also
blacklist specific, known vulnerable (unloaded) module versions. If the
admin tries to load one, a debug msg is generated explaining why it
can't be loaded by default.
</blue sky ideas>
>>>> + It might open more security holes that are not fixed by
>>>> the livepatch.
>>>
>>> Following the same line of thinking, the livepatch infrastructure might
>>> open security holes because of the inherent complexity of late module
>>> patching.
>>
>> Could you be more specific, please?
>> Has there been any known security hole in the late module
>> livepatching code?
>
> Just off the top of my head, I can think of two recent bugs which can be
> blamed on late module patching:
>
> 1) There was a RHEL-only bug which caused arch_klp_init_object_loaded()
> to not be loaded. This resulted in a panic when certain patched code
> was executed.
>
> 2) arch_klp_init_object_loaded() currently doesn't have any jump label
> specific code. This has recently caused panics for patched code
> which relies on static keys. The workaround is to not use jump
> labels in patched code. The real fix is to add support for them in
> arch_klp_init_object_loaded().
>
> I can easily foresee more problems like those in the future. Going
> forward we have to always keep track of which special sections are
> needed for which architectures. Those special sections can change over
> time, or can simply be overlooked for a given architecture. It's
> fragile.
FWIW, the static keys case is more involved than simple deferred
relocations -- those keys are added to lists and then the static key
code futzes with them when it needs to update code sites. That means
the code managing the data structures, kernel/jump_label.c, will need to
understand livepatch's strangely loaded-but-not-initialized variants.
I don't think the other special sections will require such invasive
changes, but it's something to keep in mind with respect to late module
patching.
-- Joe
next prev parent reply other threads:[~2019-08-27 15:05 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-19 12:28 [RFC PATCH 0/2] livepatch: Clear relocation targets on a module removal Miroslav Benes
2019-07-19 12:28 ` [PATCH 1/2] livepatch: Nullify obj->mod in klp_module_coming()'s error path Miroslav Benes
2019-07-28 19:45 ` Josh Poimboeuf
2019-08-19 11:26 ` Petr Mladek
2019-07-19 12:28 ` [RFC PATCH 2/2] livepatch: Clear relocation targets on a module removal Miroslav Benes
2019-07-22 9:33 ` Petr Mladek
2019-08-14 12:33 ` Miroslav Benes
2019-07-28 20:04 ` Josh Poimboeuf
2019-08-14 11:06 ` Miroslav Benes
2019-08-14 15:12 ` Josh Poimboeuf
2019-08-16 9:46 ` Petr Mladek
2019-08-22 22:36 ` Josh Poimboeuf
2019-08-23 8:13 ` Petr Mladek
2019-08-26 14:54 ` Josh Poimboeuf
2019-08-27 15:05 ` Joe Lawrence [this message]
2019-08-27 15:37 ` Josh Poimboeuf
2019-09-02 16:13 ` Miroslav Benes
2019-09-02 17:05 ` Joe Lawrence
2019-09-03 13:02 ` Miroslav Benes
2019-09-04 8:49 ` Petr Mladek
2019-09-04 16:26 ` Joe Lawrence
2019-09-05 2:50 ` Josh Poimboeuf
2019-09-05 11:09 ` Petr Mladek
2019-09-05 11:19 ` Jiri Kosina
2019-09-05 13:23 ` Josh Poimboeuf
2019-09-05 13:31 ` Jiri Kosina
2019-09-05 13:42 ` Josh Poimboeuf
2019-09-05 11:39 ` Joe Lawrence
2019-09-05 13:08 ` Josh Poimboeuf
2019-09-05 13:15 ` Josh Poimboeuf
2019-09-05 13:52 ` Petr Mladek
2019-09-05 14:28 ` Josh Poimboeuf
2019-09-05 12:03 ` Miroslav Benes
2019-09-05 12:35 ` Josh Poimboeuf
2019-09-05 12:49 ` Miroslav Benes
2019-09-05 11:52 ` Miroslav Benes
2019-09-05 2:32 ` Josh Poimboeuf
2019-09-05 12:16 ` Miroslav Benes
2019-09-05 12:54 ` Josh Poimboeuf
2019-09-06 12:51 ` Miroslav Benes
2019-09-06 15:38 ` Joe Lawrence
2019-09-06 16:45 ` Josh Poimboeuf
2019-08-26 13:44 ` Nicolai Stange
2019-08-26 15:02 ` Josh Poimboeuf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5efd9f4e-eccb-cbe0-ec2e-0e2a6a34c0ea@redhat.com \
--to=joe.lawrence@redhat.com \
--cc=jikos@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=pmladek@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.