Messages in this thread | | | Date | Tue, 7 Apr 2020 16:15:12 -0700 | From | Kees Cook <> | Subject | Re: [PATCH 3/4] x86,module: Detect VMX vs SLD conflicts |
| |
On Tue, Apr 07, 2020 at 09:25:56PM +0000, David Laight wrote: > From: Peter Zijlstra > > Sent: 07 April 2020 12:03 > > > > It turns out that with Split-Lock-Detect enabled (default) any VMX > > hypervisor needs at least a little modification in order to not blindly > > inject the #AC into the guest without the guest being ready for it. > > > > Since there is no telling which module implements a hypervisor, scan > > all out-of-tree modules' text and look for VMX instructions and refuse > > to load it when SLD is enabled (default) and the module isn't marked > > 'sld_safe'. > ... > > + while (text < text_end) { > > + kernel_insn_init(&insn, text, text_end - text); > > + insn_get_length(&insn); > > + > > + if (WARN_ON_ONCE(!insn_complete(&insn))) { > > + pr_err("Module text malformed: %s\n", mod->name); > > + return -ENOEXEC; > > + } > > + > > + if (!allow_vmx && insn_is_vmx(&insn)) { > > + pr_err("Module has VMX instructions and is not marked 'sld_safe', boot with: > > 'split_lock_detect=off': %s\n", mod->name); > > + return -ENOEXEC; > > + } > > + > > + text += insn.length; > > + } > > There is a slight flaw in the above. > A malicious module can hide the required instruction by jumping into the > middle of a long instruction. > > Even checking branch targets hit instruction barriers isn't enough, > an indirect jump could be used.
If I understand the goals here, it's to provide feedback for good actors doing things that they don't realize aren't safe. Trying to stop a malicious module from doing malicious things is basically impossible: it can just load a data blob and self-modify, etc. :)
Though, Peter, this does get me thinking: if this is meant to be helpful for module authors tripping over things they shouldn't be touching, perhaps every test needs to include explicit recommendations? It's _kind_ of doing this already. Maybe the above should be:
pr_err("%s: contains VMX instructions but is not marked 'sld_safe'. Please see <url> or boot with: 'split_lock_detect=off' to ignore.\n", mod->name);
?
-- Kees Cook
| |