Re: RFC: removing reloc and module notify code from livepatch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+++ Miroslav Benes [16/07/15 10:27 +0200]:
[the mail is getting long but leaving it as it is as it contains useful
info]

And I made a mistake too. Of course we have access to all these
information as long as we are hooked somewhere in load_module. So...

1. We can switch from COMING notifier to hook in a load_module(). This is
certainly possible (we do it in kGraft that way). We would have all the
information need to correctly process dynrelas in
klp_write_module_reloc(), because arch specific things are freed at the
end of load_module().

It would allow us to reject loading a patch module if the patching fails
somehow.

BUT this does not work for the to-be-patched modules loaded afterwards. It
is only a variant of your approach (which offloads the work to kernel
module loader) but we still do it ourselves in klp.

EDIT: The hook in load_module() is not even necessary. COMING notifiers
are called before arch specific info is freed (if I am not wrong) so we
have all the info in a notifier even now (I did not test it though).

2. We can change the kernel module loader behaviour to not to free arch
specific info if the module is a livepatch module (as I proposed above).
This would allow us to process also the not-loaded-yet modules later in
the notifier. Is that correct? (I have a feeling I still mix several
independent situations to one, so correct me if I am wrong)

Yep, that is correct, keeping arch specific info will let us apply
s390 relocs of not-loaded-yet modules for example. We can just check
if a module is a livepatch module, and not free those structures if it
is.
I thought about this yesterday when I proposed the preservation of PLT/GOT
tables. It is certainly possible but somewhat dangerous. If we did not
resolve the symbols ourselves and they would be called somehow, we would
have a big problem. But on the other hand we do the similar thing even
now, don't we? You create dynrelas in create-diff-object for all relevant
symbols. Then only those dynrelas for already loaded objects (vmlinux and
loaded modules) are processed in the kernel. The rest is left unresolved
and eventually process when the module is loaded.

I think that's right, we are already currently leaving syms (or
whatever syms the dynrelas reference) unresolved for not-loaded-yet
modules in livepatch, and we fill in those values when the target
module is loaded. I think the proposal is essentially doing something
similar, but this time we're just resolving the symbols in the module
loader instead of doing it ourselves in livepatch, and leaving symbols
of unloaded modules unresolved temporarily until the target module is
loaded. I think we should only leave a symbol unresolved if it is part
of a module that is not loaded; that's an invariant we must enforce.
We can't just leave random symbols unresolved, we must check they are
indeed from a module that's just not loaded yet.

But instead of outright ignoring a symbol (in the case that symbol
resolution fails in the module loader), we should mark these
module/unexported symbols with SHN_LIVEPATCH or something (see Josh's
other email) so we can tell the difference between a real error
(symbol resolution really did fail and we should exit with error) and
something we know will be eventually resolved in livepatch.

> I still don't know how to solve the problem with the delayed modules (not
> loaded when the patch is applied) :/

I don't know what the best solution is either, it is a hard problem!
See my other email on this topic. I think we might have to go with
some hybrid approach that combines these two approaches (i.e. we might need to
keep the klp module notifier stuff).

Yes, that is possible, but it sounds complicated and it could introduce
some unwanted scenarios.

Josh offered a nice solution that I think would work, see other email,
let us know if you see any issues with it. :)

Thanks,
Jessica
--
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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux