On Tue 2018-10-23 11:39:43, Josh Poimboeuf wrote: > On Mon, Oct 22, 2018 at 03:25:10PM +0200, Petr Mladek wrote: > > On Fri 2018-10-19 09:36:04, Josh Poimboeuf wrote: > > > On Fri, Oct 19, 2018 at 02:16:19PM +0200, Miroslav Benes wrote: > > > > On Thu, 18 Oct 2018, Josh Poimboeuf wrote: > > > > > As long as we're talking about radical changes... how about we just > > > > > don't allow disabling patches at all? Instead a patch can be replaced > > > > > with a 'revert' patch, or an empty 'nop' patch. That would make our > > > > > code simpler and also ensure there's an audit trail. > > > > > > > The revert operation allows to remove a livepatch stuck in the > > transition without forcing. > > True, though I question the real world value of that. We ended in this situation few times with kGraft when a kthread was not annotated and migrated. We have not seen this with upstream livepatch code yet but we shipped first product with it only few months ago. I would say that it is nice to have but it is not must to have. > > One big problem would be how to keep the system consistent. You > > would need to solve races between loading modules and livepatches > > anyway. > > > > For example, you could not load fixed/patched modules when the system > > is not fully patched yet. You would need to load the module and > > the related livepatch at the same time and follow the consistency > > model as we do now. > > Yeah, I think that's pretty much the crazy idea Miroslav mentioned. The > patch would consist of several modules. The parent module would > register the patch and patch vmlinux. Each child module would be > associated with a to-be-patched module. The child modules could be > loaded on demand, either by special klp code or by modprobe. Yup, something like this. > As you described, there would be some races to think about. However, it > would also have some benefits. > > I *hope* it would mean we could get rid of a lot of our ugly hacks, like > > - klp symbols, klp relas The access to external static symbols would still need so klp-specific relocations. > - preserving ELF data, PLT's, other horrible arch-specific things > - klp.arch..altinstructions, klp.arch..parainstructions > - manually calling apply_relocate_add() Yup, these might be candidates to go. > However... we might still need some of those things for another reason: > to bypass exported symbol protections. It needs some more > investigation. > > Given this discussion, I'm thinking there wouldn't be much to discuss at > LPC for this topic unless we had a prototype to look at (which I won't > have time to do). So I may drop my talk in favor of giving more time > for other more tangible discussions. Sounds reasonable. At least I would not be able to say much more about it without seeing a more detailed proposal and ideally a prototype code. That said, I definitely do not want to discourage you from playing with the idea. Best Regards, Petr
