linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
@ 2019-11-30  6:49 Matt Parnell
  2019-11-30 18:36 ` Kees Cook
  2019-12-02 19:43 ` Matthew Garrett
  0 siblings, 2 replies; 16+ messages in thread
From: Matt Parnell @ 2019-11-30  6:49 UTC (permalink / raw)
  To: linux-security-module; +Cc: dhowells, matthew.garrett, keescook


[-- Attachment #1.1: Type: text/plain, Size: 2587 bytes --]

From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
From: Matt Parnell <mparnell@gmail.com>
Date: Sat, 30 Nov 2019 00:44:09 -0600
Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
 in confidentiality mode.

For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
while this isn't something normally used in userspace, it does cause false
positives for the "Forshadow" vulnerability.

Additionally, Intel CPUs use MSRs for voltage and frequency controls,
which in
many cases is useful for undervolting to avoid excess heat.

Signed-off-by: Matt Parnell <mparnell@gmail.com>
---
 arch/x86/kernel/msr.c     |  5 ++++-
 security/lockdown/Kconfig | 12 ++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 1547be359d7f..4adce59455c3 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -80,10 +80,11 @@ static ssize_t msr_write(struct file *file, const
char __user *buf,
     int err = 0;
     ssize_t bytes = 0;
 
+#if defined(LOCK_DOWN_DENY_RAW_MSR)
     err = security_locked_down(LOCKDOWN_MSR);
     if (err)
         return err;
-
+#endif
     if (count % 8)
         return -EINVAL;    /* Invalid chunk size */
 
@@ -135,9 +136,11 @@ static long msr_ioctl(struct file *file, unsigned
int ioc, unsigned long arg)
             err = -EFAULT;
             break;
         }
+#if defined(LOCK_DOWN_DENY_RAW_MSR)
         err = security_locked_down(LOCKDOWN_MSR);
         if (err)
             break;
+#endif
         err = wrmsr_safe_regs_on_cpu(cpu, regs);
         if (err)
             break;
diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index e84ddf484010..f4fe72c4bf8f 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -44,4 +44,16 @@ config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
      code to read confidential material held inside the kernel are
      disabled.
 
+config LOCK_DOWN_DENY_RAW_MSR
+    bool "Lock down and deny raw MSR access"
+    depends on LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
+    default y
+    help
+      Some Intel based systems require raw MSR access to use the flush
+      MSR for MDS mitigation confirmation. Raw access can also be used
+      to undervolt many Intel CPUs.
+
+      Say Y to prevent access or N to allow raw MSR access for such
+      cases.
+
 endchoice
-- 
2.24.0



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-11-30  6:49 [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode Matt Parnell
@ 2019-11-30 18:36 ` Kees Cook
  2019-11-30 19:09   ` Matt Parnell
  2019-12-03  2:13   ` Matt Parnell
  2019-12-02 19:43 ` Matthew Garrett
  1 sibling, 2 replies; 16+ messages in thread
From: Kees Cook @ 2019-11-30 18:36 UTC (permalink / raw)
  To: Matt Parnell; +Cc: linux-security-module, dhowells, matthew.garrett

On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
> From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
> From: Matt Parnell <mparnell@gmail.com>
> Date: Sat, 30 Nov 2019 00:44:09 -0600
> Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
>  in confidentiality mode.
> 
> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
> while this isn't something normally used in userspace, it does cause false
> positives for the "Forshadow" vulnerability.
> 
> Additionally, Intel CPUs use MSRs for voltage and frequency controls,
> which in
> many cases is useful for undervolting to avoid excess heat.
> 
> Signed-off-by: Matt Parnell <mparnell@gmail.com>

I would expect this to just be implemented via LSM policy, not ifdefs
and Kconfig?

-Kees

> ---
>  arch/x86/kernel/msr.c     |  5 ++++-
>  security/lockdown/Kconfig | 12 ++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
> index 1547be359d7f..4adce59455c3 100644
> --- a/arch/x86/kernel/msr.c
> +++ b/arch/x86/kernel/msr.c
> @@ -80,10 +80,11 @@ static ssize_t msr_write(struct file *file, const
> char __user *buf,
>      int err = 0;
>      ssize_t bytes = 0;
>  
> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>      err = security_locked_down(LOCKDOWN_MSR);
>      if (err)
>          return err;
> -
> +#endif
>      if (count % 8)
>          return -EINVAL;    /* Invalid chunk size */
>  
> @@ -135,9 +136,11 @@ static long msr_ioctl(struct file *file, unsigned
> int ioc, unsigned long arg)
>              err = -EFAULT;
>              break;
>          }
> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>          err = security_locked_down(LOCKDOWN_MSR);
>          if (err)
>              break;
> +#endif
>          err = wrmsr_safe_regs_on_cpu(cpu, regs);
>          if (err)
>              break;
> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
> index e84ddf484010..f4fe72c4bf8f 100644
> --- a/security/lockdown/Kconfig
> +++ b/security/lockdown/Kconfig
> @@ -44,4 +44,16 @@ config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>       code to read confidential material held inside the kernel are
>       disabled.
>  
> +config LOCK_DOWN_DENY_RAW_MSR
> +    bool "Lock down and deny raw MSR access"
> +    depends on LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
> +    default y
> +    help
> +      Some Intel based systems require raw MSR access to use the flush
> +      MSR for MDS mitigation confirmation. Raw access can also be used
> +      to undervolt many Intel CPUs.
> +
> +      Say Y to prevent access or N to allow raw MSR access for such
> +      cases.
> +
>  endchoice
> -- 
> 2.24.0
> 
> 




-- 
Kees Cook

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-11-30 18:36 ` Kees Cook
@ 2019-11-30 19:09   ` Matt Parnell
  2019-12-01 20:53     ` Matt Parnell
  2019-12-03  2:13   ` Matt Parnell
  1 sibling, 1 reply; 16+ messages in thread
From: Matt Parnell @ 2019-11-30 19:09 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-security-module, dhowells, matthew.garrett

I can see how using a policy would be beneficial; I only did this
because as I understood it, policy wouldn't be able to change these
particular settings since anything attempting to do so would be from
userspace.

On 11/30/19 12:36 PM, Kees Cook wrote:
> On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
>> From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
>> From: Matt Parnell <mparnell@gmail.com>
>> Date: Sat, 30 Nov 2019 00:44:09 -0600
>> Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
>>  in confidentiality mode.
>>
>> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
>> while this isn't something normally used in userspace, it does cause false
>> positives for the "Forshadow" vulnerability.
>>
>> Additionally, Intel CPUs use MSRs for voltage and frequency controls,
>> which in
>> many cases is useful for undervolting to avoid excess heat.
>>
>> Signed-off-by: Matt Parnell <mparnell@gmail.com>
> I would expect this to just be implemented via LSM policy, not ifdefs
> and Kconfig?
>
> -Kees
>
>> ---
>>  arch/x86/kernel/msr.c     |  5 ++++-
>>  security/lockdown/Kconfig | 12 ++++++++++++
>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
>> index 1547be359d7f..4adce59455c3 100644
>> --- a/arch/x86/kernel/msr.c
>> +++ b/arch/x86/kernel/msr.c
>> @@ -80,10 +80,11 @@ static ssize_t msr_write(struct file *file, const
>> char __user *buf,
>>      int err = 0;
>>      ssize_t bytes = 0;
>>  
>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>      err = security_locked_down(LOCKDOWN_MSR);
>>      if (err)
>>          return err;
>> -
>> +#endif
>>      if (count % 8)
>>          return -EINVAL;    /* Invalid chunk size */
>>  
>> @@ -135,9 +136,11 @@ static long msr_ioctl(struct file *file, unsigned
>> int ioc, unsigned long arg)
>>              err = -EFAULT;
>>              break;
>>          }
>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>          err = security_locked_down(LOCKDOWN_MSR);
>>          if (err)
>>              break;
>> +#endif
>>          err = wrmsr_safe_regs_on_cpu(cpu, regs);
>>          if (err)
>>              break;
>> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
>> index e84ddf484010..f4fe72c4bf8f 100644
>> --- a/security/lockdown/Kconfig
>> +++ b/security/lockdown/Kconfig
>> @@ -44,4 +44,16 @@ config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>>       code to read confidential material held inside the kernel are
>>       disabled.
>>  
>> +config LOCK_DOWN_DENY_RAW_MSR
>> +    bool "Lock down and deny raw MSR access"
>> +    depends on LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>> +    default y
>> +    help
>> +      Some Intel based systems require raw MSR access to use the flush
>> +      MSR for MDS mitigation confirmation. Raw access can also be used
>> +      to undervolt many Intel CPUs.
>> +
>> +      Say Y to prevent access or N to allow raw MSR access for such
>> +      cases.
>> +
>>  endchoice
>> -- 
>> 2.24.0
>>
>>
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-11-30 19:09   ` Matt Parnell
@ 2019-12-01 20:53     ` Matt Parnell
  2019-12-02 18:29       ` Matt Parnell
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Parnell @ 2019-12-01 20:53 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-security-module, dhowells, matthew.garrett

That is, I was intending to use lockdown from boot, which isn't
changeable after the fact if I'm not mistaken. How possible is granular
control of what is and is not locked down?

On 11/30/19 1:09 PM, Matt Parnell wrote:
> I can see how using a policy would be beneficial; I only did this
> because as I understood it, policy wouldn't be able to change these
> particular settings since anything attempting to do so would be from
> userspace.
>
> On 11/30/19 12:36 PM, Kees Cook wrote:
>> On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
>>> From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
>>> From: Matt Parnell <mparnell@gmail.com>
>>> Date: Sat, 30 Nov 2019 00:44:09 -0600
>>> Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
>>>  in confidentiality mode.
>>>
>>> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
>>> while this isn't something normally used in userspace, it does cause false
>>> positives for the "Forshadow" vulnerability.
>>>
>>> Additionally, Intel CPUs use MSRs for voltage and frequency controls,
>>> which in
>>> many cases is useful for undervolting to avoid excess heat.
>>>
>>> Signed-off-by: Matt Parnell <mparnell@gmail.com>
>> I would expect this to just be implemented via LSM policy, not ifdefs
>> and Kconfig?
>>
>> -Kees
>>
>>> ---
>>>  arch/x86/kernel/msr.c     |  5 ++++-
>>>  security/lockdown/Kconfig | 12 ++++++++++++
>>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
>>> index 1547be359d7f..4adce59455c3 100644
>>> --- a/arch/x86/kernel/msr.c
>>> +++ b/arch/x86/kernel/msr.c
>>> @@ -80,10 +80,11 @@ static ssize_t msr_write(struct file *file, const
>>> char __user *buf,
>>>      int err = 0;
>>>      ssize_t bytes = 0;
>>>  
>>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>>      err = security_locked_down(LOCKDOWN_MSR);
>>>      if (err)
>>>          return err;
>>> -
>>> +#endif
>>>      if (count % 8)
>>>          return -EINVAL;    /* Invalid chunk size */
>>>  
>>> @@ -135,9 +136,11 @@ static long msr_ioctl(struct file *file, unsigned
>>> int ioc, unsigned long arg)
>>>              err = -EFAULT;
>>>              break;
>>>          }
>>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>>          err = security_locked_down(LOCKDOWN_MSR);
>>>          if (err)
>>>              break;
>>> +#endif
>>>          err = wrmsr_safe_regs_on_cpu(cpu, regs);
>>>          if (err)
>>>              break;
>>> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
>>> index e84ddf484010..f4fe72c4bf8f 100644
>>> --- a/security/lockdown/Kconfig
>>> +++ b/security/lockdown/Kconfig
>>> @@ -44,4 +44,16 @@ config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>>>       code to read confidential material held inside the kernel are
>>>       disabled.
>>>  
>>> +config LOCK_DOWN_DENY_RAW_MSR
>>> +    bool "Lock down and deny raw MSR access"
>>> +    depends on LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>>> +    default y
>>> +    help
>>> +      Some Intel based systems require raw MSR access to use the flush
>>> +      MSR for MDS mitigation confirmation. Raw access can also be used
>>> +      to undervolt many Intel CPUs.
>>> +
>>> +      Say Y to prevent access or N to allow raw MSR access for such
>>> +      cases.
>>> +
>>>  endchoice
>>> -- 
>>> 2.24.0
>>>
>>>
>>
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-01 20:53     ` Matt Parnell
@ 2019-12-02 18:29       ` Matt Parnell
  2019-12-02 22:55         ` Jordan Glover
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Parnell @ 2019-12-02 18:29 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-security-module, dhowells, matthew.garrett

After doing some research it appears that for Intel chips, only a single
register needs to be writeable. I'm not sure about AMD etc.

intel-undervolt/blob/master/config.h:

    #define MSR_ADDR_TEMPERATURE 0x1a2
    #define MSR_ADDR_UNITS 0x606
    #define MSR_ADDR_VOLTAGE 0x150

Perhaps add an MSR whitelist to allow writing, if
LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY=Y and
CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=Y?

CONFIG_SECURITY_LOCKDOWN_LSM_EARLY is likely what prevents Apparmor or
some other LSM policy manager allow this behavior...

as an option at build time would be more sensible?

On 12/1/19 2:53 PM, Matt Parnell wrote:
> That is, I was intending to use lockdown from boot, which isn't
> changeable after the fact if I'm not mistaken. How possible is granular
> control of what is and is not locked down?
>
> On 11/30/19 1:09 PM, Matt Parnell wrote:
>> I can see how using a policy would be beneficial; I only did this
>> because as I understood it, policy wouldn't be able to change these
>> particular settings since anything attempting to do so would be from
>> userspace.
>>
>> On 11/30/19 12:36 PM, Kees Cook wrote:
>>> On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
>>>> From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
>>>> From: Matt Parnell <mparnell@gmail.com>
>>>> Date: Sat, 30 Nov 2019 00:44:09 -0600
>>>> Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
>>>>  in confidentiality mode.
>>>>
>>>> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
>>>> while this isn't something normally used in userspace, it does cause false
>>>> positives for the "Forshadow" vulnerability.
>>>>
>>>> Additionally, Intel CPUs use MSRs for voltage and frequency controls,
>>>> which in
>>>> many cases is useful for undervolting to avoid excess heat.
>>>>
>>>> Signed-off-by: Matt Parnell <mparnell@gmail.com>
>>> I would expect this to just be implemented via LSM policy, not ifdefs
>>> and Kconfig?
>>>
>>> -Kees
>>>
>>>> ---
>>>>  arch/x86/kernel/msr.c     |  5 ++++-
>>>>  security/lockdown/Kconfig | 12 ++++++++++++
>>>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
>>>> index 1547be359d7f..4adce59455c3 100644
>>>> --- a/arch/x86/kernel/msr.c
>>>> +++ b/arch/x86/kernel/msr.c
>>>> @@ -80,10 +80,11 @@ static ssize_t msr_write(struct file *file, const
>>>> char __user *buf,
>>>>      int err = 0;
>>>>      ssize_t bytes = 0;
>>>>  
>>>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>>>      err = security_locked_down(LOCKDOWN_MSR);
>>>>      if (err)
>>>>          return err;
>>>> -
>>>> +#endif
>>>>      if (count % 8)
>>>>          return -EINVAL;    /* Invalid chunk size */
>>>>  
>>>> @@ -135,9 +136,11 @@ static long msr_ioctl(struct file *file, unsigned
>>>> int ioc, unsigned long arg)
>>>>              err = -EFAULT;
>>>>              break;
>>>>          }
>>>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>>>          err = security_locked_down(LOCKDOWN_MSR);
>>>>          if (err)
>>>>              break;
>>>> +#endif
>>>>          err = wrmsr_safe_regs_on_cpu(cpu, regs);
>>>>          if (err)
>>>>              break;
>>>> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
>>>> index e84ddf484010..f4fe72c4bf8f 100644
>>>> --- a/security/lockdown/Kconfig
>>>> +++ b/security/lockdown/Kconfig
>>>> @@ -44,4 +44,16 @@ config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>>>>       code to read confidential material held inside the kernel are
>>>>       disabled.
>>>>  
>>>> +config LOCK_DOWN_DENY_RAW_MSR
>>>> +    bool "Lock down and deny raw MSR access"
>>>> +    depends on LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>>>> +    default y
>>>> +    help
>>>> +      Some Intel based systems require raw MSR access to use the flush
>>>> +      MSR for MDS mitigation confirmation. Raw access can also be used
>>>> +      to undervolt many Intel CPUs.
>>>> +
>>>> +      Say Y to prevent access or N to allow raw MSR access for such
>>>> +      cases.
>>>> +
>>>>  endchoice
>>>> -- 
>>>> 2.24.0
>>>>
>>>>
>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-11-30  6:49 [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode Matt Parnell
  2019-11-30 18:36 ` Kees Cook
@ 2019-12-02 19:43 ` Matthew Garrett
  2019-12-02 20:39   ` Matt Parnell
  1 sibling, 1 reply; 16+ messages in thread
From: Matthew Garrett @ 2019-12-02 19:43 UTC (permalink / raw)
  To: Matt Parnell; +Cc: LSM List, David Howells, matthew.garrett, Kees Cook

On Fri, Nov 29, 2019 at 10:50 PM Matt Parnell <mparnell@gmail.com> wrote:
> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
> while this isn't something normally used in userspace, it does cause false
> positives for the "Forshadow" vulnerability.

The msr interface is pretty terrible - it exposes a consistent
interface over very inconsistent CPUs. Where there's CPU functionality
that's implemented via MSRs it makes sense to expose that over a
separate kernel interface.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-02 19:43 ` Matthew Garrett
@ 2019-12-02 20:39   ` Matt Parnell
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Parnell @ 2019-12-02 20:39 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: LSM List, David Howells, matthew.garrett, Kees Cook

Agreed.

That said, if we don't mind working with what already exists, this
whitelist addition (I have trouble calling it a module) exists. I wonder
if it could be reshaped into something that ties in with the lockdown
functionality?

It looks like a mixture of commits from Intel engineers and Lawrence
Livermore engineers (GPLv3) :

https://github.com/LLNL/msr-safe

On 12/2/19 1:43 PM, Matthew Garrett wrote:
> On Fri, Nov 29, 2019 at 10:50 PM Matt Parnell <mparnell@gmail.com> wrote:
>> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
>> while this isn't something normally used in userspace, it does cause false
>> positives for the "Forshadow" vulnerability.
> The msr interface is pretty terrible - it exposes a consistent
> interface over very inconsistent CPUs. Where there's CPU functionality
> that's implemented via MSRs it makes sense to expose that over a
> separate kernel interface.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-02 18:29       ` Matt Parnell
@ 2019-12-02 22:55         ` Jordan Glover
  2019-12-02 23:13           ` Matt Parnell
  2019-12-02 23:29           ` Matthew Garrett
  0 siblings, 2 replies; 16+ messages in thread
From: Jordan Glover @ 2019-12-02 22:55 UTC (permalink / raw)
  To: Matt Parnell; +Cc: Kees Cook, linux-security-module, dhowells, matthew.garrett

On Monday, December 2, 2019 6:29 PM, Matt Parnell <mparnell@gmail.com> wrote:

> After doing some research it appears that for Intel chips, only a single
> register needs to be writeable. I'm not sure about AMD etc.
>
> intel-undervolt/blob/master/config.h:
>
>     #define MSR_ADDR_TEMPERATURE 0x1a2
>     #define MSR_ADDR_UNITS 0x606
>     #define MSR_ADDR_VOLTAGE 0x150
>
> Perhaps add an MSR whitelist to allow writing, if
> LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY=Y and
> CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=Y?
>
> CONFIG_SECURITY_LOCKDOWN_LSM_EARLY is likely what prevents Apparmor or
> some other LSM policy manager allow this behavior...
>
> as an option at build time would be more sensible?
>
> On 12/1/19 2:53 PM, Matt Parnell wrote:
>
> > That is, I was intending to use lockdown from boot, which isn't
> > changeable after the fact if I'm not mistaken. How possible is granular
> > control of what is and is not locked down?
> > On 11/30/19 1:09 PM, Matt Parnell wrote:
> >
> > > I can see how using a policy would be beneficial; I only did this
> > > because as I understood it, policy wouldn't be able to change these
> > > particular settings since anything attempting to do so would be from
> > > userspace.
> > > On 11/30/19 12:36 PM, Kees Cook wrote:
> > >
> > > > On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
> > > >
> > > > > From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
> > > > > From: Matt Parnell mparnell@gmail.com
> > > > > Date: Sat, 30 Nov 2019 00:44:09 -0600
> > > > > Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
> > > > >  in confidentiality mode.
> > > > > For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
> > > > > while this isn't something normally used in userspace, it does cause false
> > > > > positives for the "Forshadow" vulnerability.
> > > > > Additionally, Intel CPUs use MSRs for voltage and frequency controls,
> > > > > which in
> > > > > many cases is useful for undervolting to avoid excess heat.

Could you clarify if blocking msr breaks internal power management of intel
cpu or it only prevents manual tinkering with it by user? If the latter then
I think it's ok to keep it as is.

Jordan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-02 22:55         ` Jordan Glover
@ 2019-12-02 23:13           ` Matt Parnell
  2019-12-02 23:29           ` Matthew Garrett
  1 sibling, 0 replies; 16+ messages in thread
From: Matt Parnell @ 2019-12-02 23:13 UTC (permalink / raw)
  To: Jordan Glover; +Cc: Kees Cook, linux-security-module, dhowells, matthew.garrett

I am not presently in a position to check this as I tend to use the
ondemand cpu scheduler, and not userspace tools to manage CPU power
consumption, and get decent battery life doing so because of the built
in settings in the bios.

I can say with certainty that the second option is true, though, however
it should be noted that it is not just for power adjustments that the
MSRs need, at least in many cases to be readable -
spectre-meltdown-checker, for example, relies on checking an MSR to
determine if the foreshadow exploit is mitigated or not.

https://github.com/speed47/spectre-meltdown-checker

On 12/2/19 4:55 PM, Jordan Glover wrote:
> On Monday, December 2, 2019 6:29 PM, Matt Parnell <mparnell@gmail.com> wrote:
>
>> After doing some research it appears that for Intel chips, only a single
>> register needs to be writeable. I'm not sure about AMD etc.
>>
>> intel-undervolt/blob/master/config.h:
>>
>>     #define MSR_ADDR_TEMPERATURE 0x1a2
>>     #define MSR_ADDR_UNITS 0x606
>>     #define MSR_ADDR_VOLTAGE 0x150
>>
>> Perhaps add an MSR whitelist to allow writing, if
>> LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY=Y and
>> CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=Y?
>>
>> CONFIG_SECURITY_LOCKDOWN_LSM_EARLY is likely what prevents Apparmor or
>> some other LSM policy manager allow this behavior...
>>
>> as an option at build time would be more sensible?
>>
>> On 12/1/19 2:53 PM, Matt Parnell wrote:
>>
>>> That is, I was intending to use lockdown from boot, which isn't
>>> changeable after the fact if I'm not mistaken. How possible is granular
>>> control of what is and is not locked down?
>>> On 11/30/19 1:09 PM, Matt Parnell wrote:
>>>
>>>> I can see how using a policy would be beneficial; I only did this
>>>> because as I understood it, policy wouldn't be able to change these
>>>> particular settings since anything attempting to do so would be from
>>>> userspace.
>>>> On 11/30/19 12:36 PM, Kees Cook wrote:
>>>>
>>>>> On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
>>>>>
>>>>>> From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
>>>>>> From: Matt Parnell mparnell@gmail.com
>>>>>> Date: Sat, 30 Nov 2019 00:44:09 -0600
>>>>>> Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
>>>>>>  in confidentiality mode.
>>>>>> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
>>>>>> while this isn't something normally used in userspace, it does cause false
>>>>>> positives for the "Forshadow" vulnerability.
>>>>>> Additionally, Intel CPUs use MSRs for voltage and frequency controls,
>>>>>> which in
>>>>>> many cases is useful for undervolting to avoid excess heat.
> Could you clarify if blocking msr breaks internal power management of intel
> cpu or it only prevents manual tinkering with it by user? If the latter then
> I think it's ok to keep it as is.
>
> Jordan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-02 22:55         ` Jordan Glover
  2019-12-02 23:13           ` Matt Parnell
@ 2019-12-02 23:29           ` Matthew Garrett
  2019-12-02 23:31             ` Matt Parnell
  1 sibling, 1 reply; 16+ messages in thread
From: Matthew Garrett @ 2019-12-02 23:29 UTC (permalink / raw)
  To: Jordan Glover
  Cc: Matt Parnell, Kees Cook, linux-security-module, dhowells,
	matthew.garrett

On Mon, Dec 2, 2019 at 2:55 PM Jordan Glover
<Golden_Miller83@protonmail.ch> wrote:

> Could you clarify if blocking msr breaks internal power management of intel
> cpu or it only prevents manual tinkering with it by user? If the latter then
> I think it's ok to keep it as is.

The latter.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-02 23:29           ` Matthew Garrett
@ 2019-12-02 23:31             ` Matt Parnell
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Parnell @ 2019-12-02 23:31 UTC (permalink / raw)
  To: Matthew Garrett, Jordan Glover
  Cc: Kees Cook, linux-security-module, dhowells, matthew.garrett

I suppose that turning off the early lockdown functionality, and then
having apparmor or selinux grant intel-undervolt permission to the MSRs
is probably another method of going about this, only slightly less "tight."

On 12/2/19 5:29 PM, Matthew Garrett wrote:
> On Mon, Dec 2, 2019 at 2:55 PM Jordan Glover
> <Golden_Miller83@protonmail.ch> wrote:
>
>> Could you clarify if blocking msr breaks internal power management of intel
>> cpu or it only prevents manual tinkering with it by user? If the latter then
>> I think it's ok to keep it as is.
> The latter.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-11-30 18:36 ` Kees Cook
  2019-11-30 19:09   ` Matt Parnell
@ 2019-12-03  2:13   ` Matt Parnell
  2019-12-03  2:16     ` Matthew Garrett
  1 sibling, 1 reply; 16+ messages in thread
From: Matt Parnell @ 2019-12-03  2:13 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-security-module, dhowells, matthew.garrett

I should also mention the kernel itself thinks it is vulnerable with the
MSRs locked down:

[    7.367922] L1TF CPU bug present and SMT on, data leak possible. See
CVE-2018-3646 and
https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for
details.

On 11/30/19 12:36 PM, Kees Cook wrote:
> On Sat, Nov 30, 2019 at 12:49:48AM -0600, Matt Parnell wrote:
>> From 452b8460e464422d268659a8abb93353a182f8c8 Mon Sep 17 00:00:00 2001
>> From: Matt Parnell <mparnell@gmail.com>
>> Date: Sat, 30 Nov 2019 00:44:09 -0600
>> Subject: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even
>>  in confidentiality mode.
>>
>> For Intel CPUs, some of the MDS mitigations utilize the new "flush" MSR, and
>> while this isn't something normally used in userspace, it does cause false
>> positives for the "Forshadow" vulnerability.
>>
>> Additionally, Intel CPUs use MSRs for voltage and frequency controls,
>> which in
>> many cases is useful for undervolting to avoid excess heat.
>>
>> Signed-off-by: Matt Parnell <mparnell@gmail.com>
> I would expect this to just be implemented via LSM policy, not ifdefs
> and Kconfig?
>
> -Kees
>
>> ---
>>  arch/x86/kernel/msr.c     |  5 ++++-
>>  security/lockdown/Kconfig | 12 ++++++++++++
>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
>> index 1547be359d7f..4adce59455c3 100644
>> --- a/arch/x86/kernel/msr.c
>> +++ b/arch/x86/kernel/msr.c
>> @@ -80,10 +80,11 @@ static ssize_t msr_write(struct file *file, const
>> char __user *buf,
>>      int err = 0;
>>      ssize_t bytes = 0;
>>  
>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>      err = security_locked_down(LOCKDOWN_MSR);
>>      if (err)
>>          return err;
>> -
>> +#endif
>>      if (count % 8)
>>          return -EINVAL;    /* Invalid chunk size */
>>  
>> @@ -135,9 +136,11 @@ static long msr_ioctl(struct file *file, unsigned
>> int ioc, unsigned long arg)
>>              err = -EFAULT;
>>              break;
>>          }
>> +#if defined(LOCK_DOWN_DENY_RAW_MSR)
>>          err = security_locked_down(LOCKDOWN_MSR);
>>          if (err)
>>              break;
>> +#endif
>>          err = wrmsr_safe_regs_on_cpu(cpu, regs);
>>          if (err)
>>              break;
>> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
>> index e84ddf484010..f4fe72c4bf8f 100644
>> --- a/security/lockdown/Kconfig
>> +++ b/security/lockdown/Kconfig
>> @@ -44,4 +44,16 @@ config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>>       code to read confidential material held inside the kernel are
>>       disabled.
>>  
>> +config LOCK_DOWN_DENY_RAW_MSR
>> +    bool "Lock down and deny raw MSR access"
>> +    depends on LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
>> +    default y
>> +    help
>> +      Some Intel based systems require raw MSR access to use the flush
>> +      MSR for MDS mitigation confirmation. Raw access can also be used
>> +      to undervolt many Intel CPUs.
>> +
>> +      Say Y to prevent access or N to allow raw MSR access for such
>> +      cases.
>> +
>>  endchoice
>> -- 
>> 2.24.0
>>
>>
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-03  2:13   ` Matt Parnell
@ 2019-12-03  2:16     ` Matthew Garrett
  2019-12-03  2:24       ` Matt Parnell
  0 siblings, 1 reply; 16+ messages in thread
From: Matthew Garrett @ 2019-12-03  2:16 UTC (permalink / raw)
  To: Matt Parnell; +Cc: Kees Cook, LSM List, David Howells, matthew.garrett

On Mon, Dec 2, 2019 at 6:01 PM Matt Parnell <mparnell@gmail.com> wrote:
>
> I should also mention the kernel itself thinks it is vulnerable with the
> MSRs locked down:
>
> [    7.367922] L1TF CPU bug present and SMT on, data leak possible. See
> CVE-2018-3646 and
> https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for
> details.

The lockdown code doesn't touch any of the codepaths the kernel uses
to access MSRs itself (a *lot* would break in that case), so if the
kernel is asserting this inappropriately then that seems like a kernel
bug.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-03  2:16     ` Matthew Garrett
@ 2019-12-03  2:24       ` Matt Parnell
  2019-12-03  2:50         ` Matt Parnell
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Parnell @ 2019-12-03  2:24 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Kees Cook, LSM List, David Howells, matthew.garrett

For what it is worth, this doesn't happen with lockdown disabled.

That message and the code that checks for mitigations is in
arch/x86/kvm/vmx/vmx.c - for some reason locking down the MSRs is even
making the kernel think that the MSR for the mitigation isn't there,
meaning that it is also likely not mitigating the bug.

On 12/2/19 8:16 PM, Matthew Garrett wrote:
> On Mon, Dec 2, 2019 at 6:01 PM Matt Parnell <mparnell@gmail.com> wrote:
>> I should also mention the kernel itself thinks it is vulnerable with the
>> MSRs locked down:
>>
>> [    7.367922] L1TF CPU bug present and SMT on, data leak possible. See
>> CVE-2018-3646 and
>> https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for
>> details.
> The lockdown code doesn't touch any of the codepaths the kernel uses
> to access MSRs itself (a *lot* would break in that case), so if the
> kernel is asserting this inappropriately then that seems like a kernel
> bug.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-03  2:24       ` Matt Parnell
@ 2019-12-03  2:50         ` Matt Parnell
  2019-12-03  3:57           ` Matt Parnell
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Parnell @ 2019-12-03  2:50 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Kees Cook, LSM List, David Howells, matthew.garrett

Correction: I'm out of caffeine, tired, and it has made me an idiot.

That message triggers regardless, it seems. I apologize.

On 12/2/19 8:24 PM, Matt Parnell wrote:
> For what it is worth, this doesn't happen with lockdown disabled.
>
> That message and the code that checks for mitigations is in
> arch/x86/kvm/vmx/vmx.c - for some reason locking down the MSRs is even
> making the kernel think that the MSR for the mitigation isn't there,
> meaning that it is also likely not mitigating the bug.
>
> On 12/2/19 8:16 PM, Matthew Garrett wrote:
>> On Mon, Dec 2, 2019 at 6:01 PM Matt Parnell <mparnell@gmail.com> wrote:
>>> I should also mention the kernel itself thinks it is vulnerable with the
>>> MSRs locked down:
>>>
>>> [    7.367922] L1TF CPU bug present and SMT on, data leak possible. See
>>> CVE-2018-3646 and
>>> https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for
>>> details.
>> The lockdown code doesn't touch any of the codepaths the kernel uses
>> to access MSRs itself (a *lot* would break in that case), so if the
>> kernel is asserting this inappropriately then that seems like a kernel
>> bug.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode.
  2019-12-03  2:50         ` Matt Parnell
@ 2019-12-03  3:57           ` Matt Parnell
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Parnell @ 2019-12-03  3:57 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Kees Cook, LSM List, David Howells, matthew.garrett

...possibly a bug on my older 4790k system, on my 8550u this doesn't
seem to happen. Strange.

Either way, I can stop bugging you if you'd like.

On 12/2/19 8:50 PM, Matt Parnell wrote:
> Correction: I'm out of caffeine, tired, and it has made me an idiot.
>
> That message triggers regardless, it seems. I apologize.
>
> On 12/2/19 8:24 PM, Matt Parnell wrote:
>> For what it is worth, this doesn't happen with lockdown disabled.
>>
>> That message and the code that checks for mitigations is in
>> arch/x86/kvm/vmx/vmx.c - for some reason locking down the MSRs is even
>> making the kernel think that the MSR for the mitigation isn't there,
>> meaning that it is also likely not mitigating the bug.
>>
>> On 12/2/19 8:16 PM, Matthew Garrett wrote:
>>> On Mon, Dec 2, 2019 at 6:01 PM Matt Parnell <mparnell@gmail.com> wrote:
>>>> I should also mention the kernel itself thinks it is vulnerable with the
>>>> MSRs locked down:
>>>>
>>>> [    7.367922] L1TF CPU bug present and SMT on, data leak possible. See
>>>> CVE-2018-3646 and
>>>> https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for
>>>> details.
>>> The lockdown code doesn't touch any of the codepaths the kernel uses
>>> to access MSRs itself (a *lot* would break in that case), so if the
>>> kernel is asserting this inappropriately then that seems like a kernel
>>> bug.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-12-03  3:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-30  6:49 [PATCH] Kernel Lockdown: Add an option to allow raw MSR access even, in confidentiality mode Matt Parnell
2019-11-30 18:36 ` Kees Cook
2019-11-30 19:09   ` Matt Parnell
2019-12-01 20:53     ` Matt Parnell
2019-12-02 18:29       ` Matt Parnell
2019-12-02 22:55         ` Jordan Glover
2019-12-02 23:13           ` Matt Parnell
2019-12-02 23:29           ` Matthew Garrett
2019-12-02 23:31             ` Matt Parnell
2019-12-03  2:13   ` Matt Parnell
2019-12-03  2:16     ` Matthew Garrett
2019-12-03  2:24       ` Matt Parnell
2019-12-03  2:50         ` Matt Parnell
2019-12-03  3:57           ` Matt Parnell
2019-12-02 19:43 ` Matthew Garrett
2019-12-02 20:39   ` Matt Parnell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).