Re: [PATCH 0/4] x86/module: Out-of-tree module decode and sanitize

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/04/2020 20:41, Peter Zijlstra wrote:
> On Tue, Apr 07, 2020 at 06:23:27PM +0100, Andrew Cooper wrote:
>> On 07/04/2020 12:02, Peter Zijlstra wrote:
>>> Hi all,
>>>
>>> Driven by the SLD vs VMX interaction, here are some patches that provide means
>>> to analyze the text of out-of-tree modules.
>>>
>>> The first user of that is refusing to load modules on VMX-SLD conflicts, but it
>>> also has a second patch that refulses to load any module that tries to modify
>>> CRn/DRn.
>>>
>>> I'm thinking people will quickly come up with more and more elaborate tests to
>>> which to subject out-of-tree modules.
>> Anything playing with LGDT & friends?  Shouldn't be substantially more
>> elaborate than CR/DR to check for.
> More friends? (I wasn't sure on the Sxxx instructions, they appear
> harmless, but what do I know..)
>
> I was also eyeing LSL LTR LSS, none of which I figured a module has any
> business of using. Are there more?

Sorry - should have been more clear.  By friends, I meant LGDT, LIDT,
LLDT and LTR which are the 4 system table loading instructions.  LLDT
and LTR depend on being able to write into the GDT, but still have no
business being used.

Also, LMSW if you care about it, but its utility is somewhere close to 0
these days, so probably not worth the cycles searching for.

The Sxxx instructions have no business being used, but are also harmless
and similarly, probably not worth spending cycles searching for.

L{D,E,F,S}S are functional equivalents to "MOV val1, %sreg; mov val2,
%reg"  so harmless (also mode specific as to whether they are useable).


Other things to consider, while we're on a roll:

WRMSR and RDMSR:  There is a lot of damage which can be done with these,
and at least forcing people to use the regular hooks will get proper
paravirt support and/or exception support.  That said, this might cause
large carnage to out-of-tree modules which are a device driver for
random platform things.

POPF: Don't really want someone being able to set IOPL3.  However, this
might quite easily show up as a false positive depending on how the
irqsafe infrastructure gets inlined.

SYSRET/SYSEXIT/IRET: Don't want a module returning to userspace behind
the kernels back.  IRET may be a false positive for serialising
purposes, as may be a write to CR2 for that matter.

Looking over the list of other privileged instructions, CLTS,
{,WB,WBNO}INVD, INVLPG and HLT might be candidates for "clearly doing
something which shouldn't be done".  Not on the list is INVPCID which
falls into the same category.

Come to think about it, it might be easier to gauge on CPL0 instructions
and whitelist the ok ones, such as VMX and SVM for out-of-tree hypervisors.

~Andrew