On 10/23/2015 05:23 PM, Konrad Rzeszutek Wilk wrote:
On Fri, Oct 23, 2015 at 04:28:25PM +0100, Ross Lagerwall wrote:
Limitations
===========
The above is enough to fully implement an update system where multiple
source patches are combined (using combinediff) and built into a single
binary which then atomically replaces any existing loaded patches (this is
why I added a REPLACE operation). This is the approach used by kPatch and
kGraft. Multiple completely independent patches can also be loaded but
unexpected interactions may occur.
As it stands, the patches are statically linked which means that independent
patches cannot be linked against one another (e.g. if one introduces a new
symbol). Using the combinediff approach above fixes this.
Backtraces containing functions from a patch module do not show the symbol
name.
There is no checking that a patch which is loaded is built for the correct
hypervisor.
Hehe.. bugs? What bugs!?
Of course there are no bugs :-)
Binary patching works at the function level.
Design thoughts
===============
Combining patches at the source level is relatively easy. Multiple binary
patches applied at runtime is tricky. I'm not convinced that it is
necessarily a good idea. Based on the discussion so far, the sanest way of
doing this that I can think of is:
* Each hypervisor has an embedded build id.
* Each binary patch has an embedded build id.
* The hypervisor should expose its build id and the build id of every loaded
binary patch.
* Each binary patch specifies one or more build ids on which it depends.
These build ids can be either a hypervisor build id or another patch build
id. The dependencies could either identified automatically or by a
developer.
* The userspace tool enforces dependency management (user can optionally
force patch apply). I don't see any reason to involve the
hypervisor for dependency management.
Implementing this scheme will require dynamically linking the binary
patches.
The CHECK phase seems unnecessary to me. I would think that any safety
checking that needs to be done would be done atomically at the point of
patch apply (or revert). Given the implemented system of applying patches,
I'm not sure if any safety checking need be done at all.
It was added as a way to do signature checking and any other type
of checking that needed to be done. And which may take quite a while
to get done - hence doing it asynchronously.
OK. There are many things that need to be done to load an xSplice
module, almost all of which are dependent on the size of the module and
may also fail (e.g. resolving symbols, performing relocations, copying
allocated sections, etc). I think signature checking should be as part
of the load procedure, and if that needs to be done asynchronously, then
so be it. The nice thing about doing signature checking at load time is
that (if it's implemented as per Linux's signature checking) once the
load phase is complete, the original uploaded payload can be freed from
memory. It might be handy to think of the load procedure as equivalent
to a basic version of the Linux kernel module loader (which is pretty
much what I did when implementing it).
And while I remember, I think the REVERTED state is unnecessary. It
seems exactly equivalent to the LOADED state, which is just confusing.
We have a meeting this Monday (Oct 26th) @ 10AM on the #xsplice channel.
And in which we can disucss the 'combining patches' part, the build-id, the
logging mechanism, and other things you had encountered and have thoughts on.
Sure, we can discuss these items.
--
Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
