Josh Poimboeuf wrote:
On Tue, Nov 07, 2017 at 12:31:05PM +0100, Torsten Duwe wrote:
On Tue, Nov 07, 2017 at 07:34:29PM +1100, Michael Ellerman wrote:
> > So, just brainstorming a bit, here are the possible solutions I can
> > think of:
> >
> > a) Create a special klp stub for such calls (as in Kamalesh's patch)
> >
> > b) Have kpatch-build rewrite the function to insert nops after calls to
> > previously-local functions. This would also involve adjusting the
> > offsets of intra-function branches and relocations which come
> > afterwards in the same section. And also patching up the DWARF
> > debuginfo, if we care about that (I think we do). And also patching
> > up the jump tables which GCC sometimes creates for switch statements.
> > Yuck. I'm pretty sure this is a horrible idea.
>
> It's fairly horrible. It might be *less* horrible if you generated an
> assembly listing using the compiler, modified that, and then fed that
> through the assembler and linker.
Ah, that would work a lot better.
> > c) Create a new GCC flag which treats all calls as global, which can be
> > used by kpatch-build to generate the right code (assuming this flag
> > doesn't already exist). This would be a good option, I think.
>
> That's not impossible, but I doubt it will be all that popular with the
> toolchain folks who'd have to implement it :) It will also take a long
> time to percolate out to users.
BTDT ;-)
Yeah, maybe that's not so realistic.
> > d) Have kpatch-build do some other kind of transformation? For example,
> > maybe it could generate klp stubs which the callee calls into. Each
> > klp stub could then do a proper global call to the SHN_LIVEPATCH
> > symbol.
>
> That could work.
Indeed. There is no reason to load that off onto the kernel module loader.
I agree that doing the same work in tooling would be better than adding
complexity to the kernel.
[In case we're considering this as an option...]
While I agree that it's better to do this in the tool in general, for
this particular scenario, I think it is better to do this in the kernel.
From what I understand, the challenge here is the need to save/restore
toc, *without* creating a local stack frame for the stub to use. So, we
will have to use the livepatch stack in one form or the other. I'm not
sure if we can do anything else in a klp stub.
And, if we have to use the livepatch stack, I think it is better to let
the kernel control that so that kpatch doesn't need to change for any
changes in livepatch_handler() for instance. The current patch reuses
livepatch_handler() and makes its usage for kpatch quite explicit.
- Naveen
--
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
