On Tue, Aug 27, 2019 at 11:05:51AM -0400, Joe Lawrence wrote:

quoted

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.

I don't think there can be a mismatch unless somebody is mucking with
the .ko files directly -- and anyway that would already be a problem
today, because the patch module already assumes that the runtime version
of the module matches what the patch module was built against.

<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>

I like this.

One potential tweak: the updated modules could be delivered with the
patch module, and either replaced on disk or otherwise hooked into
modprobe.

quoted

quoted

quoted

quoted

   + 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.

Maybe it could be implemented in a way that such differences are
transparent (insert lots of hand-waving).

So as far as I can tell, we currently have three feasible options:

1) drop unloaded module changes (and blacklist the old .ko and/or replace it)
2) use per-object patches (with no exported function changes)
3) half-load unloaded modules so we can patch them

I think I like #1, if we could figure out a simple way to do it.

-- 
Josh