+++ Josh Poimboeuf [15/07/15 13:52 -0500]:
On Mon, Jul 13, 2015 at 05:40:35PM -0500, Josh Poimboeuf wrote:
On Tue, Jul 14, 2015 at 12:04:56AM +0200, Jiri Kosina wrote:
> On Mon, 13 Jul 2015, Josh Poimboeuf wrote:
>
> > > As a distro vendor, you definitely want to ship this as a single
> > > livepatch, but you absolutely don't want it to cause force modprobing of
> > > every affected network driver on all systems that install that livepatch.
> >
> > That example is pretty much the worst case scenario.
>
> I've pulled it completely out of the air, but I am currently far from
> claiming it impossible. It's not just network drivers / netdev API. We're
> in similar situation is with filesystems / vfs, pretty much anything /
> {mm, sysfs}, etc.
>
> So I would really like to avoid ignoring such scenarios. Let's try to be
> as general as possible.
>
> > But how likely is it really? Have you encountered anything like that
> > with SLES?
>
> That's hard to say for me immediately; definitely not with our live
> patching offering, but that's been there only for relatively short time so
> far. I'll try to do some more sophisticated analysis of the kernel patches
> tomorrow, to see if I am able to find some instances of this pattern.
Thanks. I think some specifics would really help. I'll try to do the
same.
I looked at a bunch of CVE fixes. I didn't see anything as bad as this
worst case scenario. There were a few fixes which changed EXPORT_SYMBOL
prototypes. Probably the best way to handle those, if the prototype
changes are absolutely necessary for this fix, would be to add new
interfaces rather than changing the existing ones. Then you only have
one-way dependencies, and you don't break third party modules.
However, dependencies aside, I noticed that ~40% of the CVE fixes
involved patching a module. So patching modules is very prevalent.
Even if there are no dependencies involved, delayed module patching is a
really nice feature. If our goal is to minimize disruption, forcing the
user to load unused modules unnecessarily is probably a bad idea.
Thanks for the patch analysis! That's helpful to know. At this point
in the discussion I do think we need to keep this feature (klp_module_notify).
So maybe we need to keep thinking -- maybe there's a way to keep delayed
module patching while reusing the module loader's relocation code
somehow?
Yeah, I think we need to consider a hybrid approach. We can still
reuse the module loader code, but we will probably need to keep
klp_module_notify (and ultimately klp_write_module_reloc) to be able
patch unloaded modules. This also means that we will probably need to keep
dynrelas as well as the reloc code in core.c
But we can still leverage the module loader by having vmlinux patches use
the new proposed approach, narrowing dynrelas to only the relas pertaining to modules.
Then the dynrelas would really be only for use in klp_module_notify,
and the klp_write_module_reloc route will only be used to patch
unloaded modules. Already loaded modules would also use the new
proposed approach. We can also make klp_module_reloc reuse the
relocation code in the module loader too, avoiding code duplication (I
remember you suggested this before :-) ). See: https://lkml.org/lkml/2015/5/28/56
We can keep arch-specific code to a minimum by reducing it to a single
static_relocate call, like in the patch above.
Back to the s390, I think this hybrid approach will still alleviate the
reloc issues we've encountered. Keeping all the symbols and relas
intact (i.e. no stripping of syms in create-diff-object) means that the module loader
will automatically allocate the plt/got entries we need to perform s390 relocs.
vmlinux + loaded modules can apply s390 relocs the new way,
and unloaded modules just go through the klp_write_module_reloc indirection.
With that said, I don't know if this hybrid approach would work.
Haven't hashed out all the details yet. What do you think?
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
