On Tue, Aug 27, 2019 at 11:05:51AM -0400, Joe Lawrence wrote: > > Sure, it introduces risk. But we have to compare that risk (which only > > affects rare edge cases) with the ones introduced by the late module > > patching code. I get the feeling that "late module patching" introduces > > risk to a broader range of use cases than "occasional loading of unused > > modules". > > > > The latter risk could be minimized by introducing a disabled state for > > modules - load it in memory, but don't expose it to users until > > explicitly loaded. Just a brainstormed idea; not sure whether it would > > work in practice. > > > > Interesting idea. We would need to ensure consistency between the > loaded-but-not-enabled module and the version on disk. Does module init run > when it's enabled? Etc. I don't think there can be a mismatch unless somebody is mucking with the .ko files directly -- and anyway that would already be a problem today, because the patch module already assumes that the runtime version of the module matches what the patch module was built against. > <blue sky ideas> > > What about folding this the other way? ie, if a livepatch targets unloaded > module foo, loaded module bar, and vmlinux ... it effectively patches bar > and vmlinux, but the foo changes are dropped. Responsibility is placed on > the admin to install an updated foo before loading it (in which case, > livepatching core will again ignore foo.) > > Building on this idea, perhaps loading that livepatch would also blacklist > specific, known vulnerable (unloaded) module versions. If the admin tries > to load one, a debug msg is generated explaining why it can't be loaded by > default. > > </blue sky ideas> I like this. One potential tweak: the updated modules could be delivered with the patch module, and either replaced on disk or otherwise hooked into modprobe. > > > > > + It might open more security holes that are not fixed by > > > > > the livepatch. > > > > > > > > Following the same line of thinking, the livepatch infrastructure might > > > > open security holes because of the inherent complexity of late module > > > > patching. > > > > > > Could you be more specific, please? > > > Has there been any known security hole in the late module > > > livepatching code? > > > > Just off the top of my head, I can think of two recent bugs which can be > > blamed on late module patching: > > > > 1) There was a RHEL-only bug which caused arch_klp_init_object_loaded() > > to not be loaded. This resulted in a panic when certain patched code > > was executed. > > > > 2) arch_klp_init_object_loaded() currently doesn't have any jump label > > specific code. This has recently caused panics for patched code > > which relies on static keys. The workaround is to not use jump > > labels in patched code. The real fix is to add support for them in > > arch_klp_init_object_loaded(). > > > > I can easily foresee more problems like those in the future. Going > > forward we have to always keep track of which special sections are > > needed for which architectures. Those special sections can change over > > time, or can simply be overlooked for a given architecture. It's > > fragile. > > FWIW, the static keys case is more involved than simple deferred relocations > -- those keys are added to lists and then the static key code futzes with > them when it needs to update code sites. That means the code managing the > data structures, kernel/jump_label.c, will need to understand livepatch's > strangely loaded-but-not-initialized variants. > > I don't think the other special sections will require such invasive changes, > but it's something to keep in mind with respect to late module patching. Maybe it could be implemented in a way that such differences are transparent (insert lots of hand-waving). So as far as I can tell, we currently have three feasible options: 1) drop unloaded module changes (and blacklist the old .ko and/or replace it) 2) use per-object patches (with no exported function changes) 3) half-load unloaded modules so we can patch them I think I like #1, if we could figure out a simple way to do it. -- Josh
