On Thu, 19 Oct 2017, Josh Poimboeuf wrote: > On Thu, Oct 19, 2017 at 04:27:31PM +0200, Miroslav Benes wrote: > > On Thu, 19 Oct 2017, Josh Poimboeuf wrote: > > > > I think that klp-convert can work with both. Even with non-source-based > > > > solution you need something to generate those relocation records. I > > > > consider klp-convert as a part of the building pipeline. > > > > > > Hm. If I understand correctly, the binary diff tool (or some tool in > > > the pipeline) would create the .klp.module_relocs.* section, and then > > > klp-convert would convert that to the .klp.sym.* and .klp.rela.* > > > sections which livepatch needs. > > > > > > But if the original tool is creating a relocation section, can't it > > > instead just create the livepatch .klp.* sections directly? What's the > > > benefit of the extra conversion step? > > > > I haven't seen this patch set for a while (which is embarassing), but > > klp-convert tries to generate needed sections automatically without > > .klp.module_relocs.* section. Only when there is an ambiguity which cannot > > be solved automatically, manual annotation (KLP_MODULE_RELOC) is needed. > > In that case klp-convert provides hints what needs to be done. > > Ah right, I forgot about that improvement to the patches. But wouldn't > the binary diff tool still have to do the manual KLP_MODULE_RELOC > annotation when it's needed? If so, then I still don't see the benefit > of the extra conversion step. In a way, yes. It depends on the tool. I always pictured it as a pipeline (similar to toolchain) and klp-convert as one of its blocks. > > > > We also considered complete source-based solution. Nicolai Stange works on > > > > that (or at least on something which would make it possible). > > > > > > What is a complete source-based solution? Is it just "klp-convert + > > > some GCC optimization strategy" or is it something more? > > > > There's more. You'd give the tool a fix (patch, diff) and kernel sources, > > and it would automatically generate a source code of its livepatch. If > > possible (and there are some obstacles), there would be an advantage > > compared to kpatch-build or different asm/obj-based solution. > > Sounds nice, though I wonder what the obstacles are? Those GCC optimizations you mentioned below and which I didn't connect to klp-convert itself. More on that later. Nothing serious aside from that, I hope. Nicolai is currently implementing C parser for kernel sources. > > You could verify the result and its correctness. > > Does that mean it's easier to do code review? Or something else? Yes, the code review. > > It could also be beneficial if we'd like to pursue automatic > > verification in the future. > > What do you mean by automatic verification? Formal verification. Theoretically we could have a formal specification of our consistency model and we could prove/disprove whether a livepatch and its implementation are correct with respect to it. It is a vague idea though and I personally haven't got sufficient knowledge to do anything about it. > > > > > IMO, klp-convert will only be useful if we have a realistic strategy for > > > > > dealing with GCC optimizations. So I'd say we should follow through on > > > > > that with the compiler folks before spending too much more time on it. > > > > > > > > Yes, I'm all for a solution on GCC side, but that may take a while and > > > > even then it is still a huge step to get it into a distribution (we have > > > > GCC 4.8.5 in SLE12 :)). > > > > > > > > However, there is an easy temporary solution. You can add all > > > > referenced optimized functions to a livepatch and let klp-convert process > > > > the rest. > > > > > > How do you find all referenced optimized functions? > > > > I guess that since there is no connection between a symbol and its > > optimized counterpart, klp-convert warns about this. > > > > Joao, is this correct? > > I'm very confused by this. You're the expert, so please set me straight :-) Oh, am I now? :) > I think you're talking about functions whose symbols have been renamed > by GCC and have been given an optimized suffix, like ".isra" or > ".constprop", right? Wasn't one of your main points at Plumbers last > year that not all such function-ABI-breaking optimizations result in a > rename of the symbol? I misunderstood your question then. Yes, I was talking about changed symbol names, while you asked about something I had in a different block of "pipeline". So yes, it is still a problem, which can be easily solved by asm/obj-based approach and not so easily by source-based approach. > > I understand your position and I agree that klp-convert may become > > superfluous in the future. Maybe not. And maybe the future is far away. > > Anyway, it looks useful in its current form and it would help tremendously > > at least here at SUSE, which is the reason Joao worked on it and send it > > upstream. > > My main objection to merging klp-convert in its current state is that > it's not useful by itself. In fact, it's actively dangerous if people > assume that because it's in-tree, it's the definitive way to safely > create patches. > > I have a similar worry about the livepatch-sample module. It's also > actively dangerous. Its only decent justification for being in-tree, > IMO, is that we at least need some type of in-tree user of the klp > interfaces. Well, you could use this reasoning even for kernel livepatching codebase itself. It is hard to use it right, but it is there and thus dangerous. > (But maybe we could solve that by converting livepatch-sample to > selftests. That would also have another benefit of giving us some > automated test coverage.) Yes, that would be great. > klp-convert is a vast improvement to the livepatch-sample module, but I > view that as a bad thing because it makes it a lot easier to do > something stupid ;-) > > If it were part of a complete solution, with some supporting tooling > and/or documentation which prevent the user from making dumb mistakes, > then I think it would make sense to merge it. Right, so this is where our views differ a bit. I'd like to get to the finish line (whatever that means) slowly but steady and not to wait for the ultimate solution if it can be implemented step by step. I think it is time for others to express their opinions. We should talk about it next week at OSS. > Anyway I appreciate all the research efforts you guys are doing. All > the different options sound promising. Yeah, I think it is important to try as much as possible. Thanks, Miroslav -- 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
