RW GDT replaced by Read-Only GDT and a GPL Interface

RW GDT replaced by Read-Only GDT and a GPL Interface

[Gregory Panic]

Hi,

I have been working on a VMX driver for a custom hypervisor which
depends on features that KVM is not built for.  Up until recently
(4.12.x), our module has been working just fine.  When we started to
build support for Ubuntu 18.04 (4.15.x+), we ran into an issue.

Unfortunately the following commits broke functionality, causing
kernel segfaults, due to a placing the GDT in the FIXMAP area, and
setting that mapping to Read-Only.
#69218e47994da614e7af600bf06887750ab6657a
and
#45fc8757d1d2128e342b4e7ef39adedf7752faac

Up until now, the GDT has been Read-Write, which allowed for resetting
the TSS to available, and then Reloading it after a VMExit.  The KVM
and Xen work-arounds for this were implemented by creating a GPL'd
interface to remap the GDT to the original Read/Write mapping and then
back.

Up to this point we've been able to maintain independence from GPL,
but with this change the only alternative to using this interface
would be to basically subvert the intent of the security patch in my
module.  Unfortunately this leaves me with the choice of letting this
one interface force GPL upon years of work, or implementing
unsafe/unadvised code.

I was wondering if it would be possible to change the gdt_page,
load_direct_gdt, and load_fixmap_gdt functions to EXPORT_SYMBOL
instead of EXPORT_SYMBOL_GPL as listed below.  This would restore the
lost functionality which we had relied on while using the recommended
interface to the kernel for this purpose.

Thank you for your consideration

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8a5b185..82fbd67 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -152,7 +152,7 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct gdt_page,
gdt_page) = { .gdt = {
        GDT_STACK_CANARY_INIT
 #endif
 } };
-EXPORT_PER_CPU_SYMBOL_GPL(gdt_page);
+EXPORT_PER_CPU_SYMBOL(gdt_page);

 static int __init x86_mpx_setup(char *s)
 {
@@ -519,7 +519,7 @@ void load_direct_gdt(int cpu)
        gdt_descr.size = GDT_SIZE - 1;
        load_gdt(&gdt_descr);
 }
-EXPORT_SYMBOL_GPL(load_direct_gdt);
+EXPORT_SYMBOL(load_direct_gdt);

 /* Load a fixmap remapping of the per-cpu GDT */
 void load_fixmap_gdt(int cpu)
@@ -530,7 +530,7 @@ void load_fixmap_gdt(int cpu)
        gdt_descr.size = GDT_SIZE - 1;
        load_gdt(&gdt_descr);
 }
-EXPORT_SYMBOL_GPL(load_fixmap_gdt);
+EXPORT_SYMBOL(load_fixmap_gdt);

Re: RW GDT replaced by Read-Only GDT and a GPL Interface

[Greg KH]

On Tue, Apr 17, 2018 at 05:33:25PM -0400, Gregory Panic wrote:
> Hi,
> 
> I have been working on a VMX driver for a custom hypervisor which
> depends on features that KVM is not built for.  Up until recently
> (4.12.x), our module has been working just fine.  When we started to
> build support for Ubuntu 18.04 (4.15.x+), we ran into an issue.
> 
> Unfortunately the following commits broke functionality, causing
> kernel segfaults, due to a placing the GDT in the FIXMAP area, and
> setting that mapping to Read-Only.
> #69218e47994da614e7af600bf06887750ab6657a
> and
> #45fc8757d1d2128e342b4e7ef39adedf7752faac
> 
> Up until now, the GDT has been Read-Write, which allowed for resetting
> the TSS to available, and then Reloading it after a VMExit.  The KVM
> and Xen work-arounds for this were implemented by creating a GPL'd
> interface to remap the GDT to the original Read/Write mapping and then
> back.
> 
> Up to this point we've been able to maintain independence from GPL,

As has been stated numberous times, the lack of EXPORT_SYMBOL_GPL() does
NOT mean that the interface you are using is not covered under the GPL
license of the kernel.

Please consult with an IP lawyer for the details if you are interested.

Best of luck with your driver!

greg k-h

Re: RW GDT replaced by Read-Only GDT and a GPL Interface

[Gregory Panic]

On Sun, Apr 29, 2018 at 11:59 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Apr 17, 2018 at 05:33:25PM -0400, Gregory Panic wrote:
>> Hi,
>>
>> I have been working on a VMX driver for a custom hypervisor which
>> depends on features that KVM is not built for.  Up until recently
>> (4.12.x), our module has been working just fine.  When we started to
>> build support for Ubuntu 18.04 (4.15.x+), we ran into an issue.
>>
>> Unfortunately the following commits broke functionality, causing
>> kernel segfaults, due to a placing the GDT in the FIXMAP area, and
>> setting that mapping to Read-Only.
>> #69218e47994da614e7af600bf06887750ab6657a
>> and
>> #45fc8757d1d2128e342b4e7ef39adedf7752faac
>>
>> Up until now, the GDT has been Read-Write, which allowed for resetting
>> the TSS to available, and then Reloading it after a VMExit.  The KVM
>> and Xen work-arounds for this were implemented by creating a GPL'd
>> interface to remap the GDT to the original Read/Write mapping and then
>> back.
>>
>> Up to this point we've been able to maintain independence from GPL,
>
> As has been stated numberous times, the lack of EXPORT_SYMBOL_GPL() does
> NOT mean that the interface you are using is not covered under the GPL
> license of the kernel.
>
> Please consult with an IP lawyer for the details if you are interested.
>
> Best of luck with your driver!
>
> greg k-h


I concede that the lack of EXPORT_SYMBOL_GPL may not imply lack of
coverage by the GPL.

However, let me clarify my issue with these patches.

These patches explicitly REMOVE a previously working (architecture-level)
functionality and seemingly PURPOSEFULLY breaks proprietary modules.

There is no middle-ground as a result of this patch.  For VMX drivers under
Linux you must now either GPL your code or subvert the security of the kernel.
This is not a good place to be.


The operation in particular is the clearing of the TSS busy bit.  This
is a primitive
operation that is well defined in the intel developers manual, and one that must
be done upon a VMExit so that the TR can be reloaded.  This can no longer be
accomplished by proprietary drivers under Linux due to its R/O GDT - except by
using an explicitly GPL'd interface.


A colleague suggested an alternative patch would be to implement a primitive
interface to clear the TSS busy bit that does not result in derivative works.

// returns previous value, or -1 if no TSS there
int change_tss_busy(uint16_t selector, bool busy);

-- 
Gregory