Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Dave Hansen]

On 02/22/2018 01:59 PM, Andy Lutomirski wrote:
>> I think it's good practice to just expose only the *minimal* amount of
>> data necessary.  It's easier to audit and less likely to expose things
>> accidentall.
> But espfix64 is geniunely global.  I'm confused.

I'm the confused one.

In my *first* set of patches to do this, I missed making the espfix64
area global.  This new code apparently got it right, but I assumed the
"new" global area that showed up was a mistake, thus this patch.

I'll change this patch into a commenting patch that calls out the need
for keeping _PAGE_GLOBAL.

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Dave Hansen]

On 02/22/2018 01:59 PM, Andy Lutomirski wrote:
>> I think it's good practice to just expose only the *minimal* amount of
>> data necessary.  It's easier to audit and less likely to expose things
>> accidentall.
> But espfix64 is geniunely global.  I'm confused.

I'm the confused one.

In my *first* set of patches to do this, I missed making the espfix64
area global.  This new code apparently got it right, but I assumed the
"new" global area that showed up was a mistake, thus this patch.

I'll change this patch into a commenting patch that calls out the need
for keeping _PAGE_GLOBAL.

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Andy Lutomirski]

On Thu, Feb 22, 2018 at 9:30 PM, Dave Hansen
 wrote:
> On 02/22/2018 01:27 PM, Nadav Amit wrote:
>> Dave Hansen  wrote:
>>> From: Dave Hansen 
>>> In creating its page tables, the espfix code masks its PGTABLE_PROT
>>> value with the supported mask: __supported_pte_mask.  This ensures
>>> that unsupported bits are not set in the final PTE.  But, it also
>>> sets _PAGE_GLOBAL which we do not want for PTE.  Use
>>> __default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.
>>
>> Can you please explain what is your concern? Exposing more gadgets for
>> speculative ROP attacks?
>>
>> Or is it a general rule of not exposing any kernel code  more than
>> absolutely necessary?
>
> I think it's good practice to just expose only the *minimal* amount of
> data necessary.  It's easier to audit and less likely to expose things
> accidentall.

But espfix64 is geniunely global.  I'm confused.

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Andy Lutomirski]

On Thu, Feb 22, 2018 at 9:30 PM, Dave Hansen
 wrote:
> On 02/22/2018 01:27 PM, Nadav Amit wrote:
>> Dave Hansen  wrote:
>>> From: Dave Hansen 
>>> In creating its page tables, the espfix code masks its PGTABLE_PROT
>>> value with the supported mask: __supported_pte_mask.  This ensures
>>> that unsupported bits are not set in the final PTE.  But, it also
>>> sets _PAGE_GLOBAL which we do not want for PTE.  Use
>>> __default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.
>>
>> Can you please explain what is your concern? Exposing more gadgets for
>> speculative ROP attacks?
>>
>> Or is it a general rule of not exposing any kernel code  more than
>> absolutely necessary?
>
> I think it's good practice to just expose only the *minimal* amount of
> data necessary.  It's easier to audit and less likely to expose things
> accidentall.

But espfix64 is geniunely global.  I'm confused.

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Dave Hansen]

On 02/22/2018 01:27 PM, Nadav Amit wrote:
> Dave Hansen  wrote:
>> From: Dave Hansen 
>> In creating its page tables, the espfix code masks its PGTABLE_PROT
>> value with the supported mask: __supported_pte_mask.  This ensures
>> that unsupported bits are not set in the final PTE.  But, it also
>> sets _PAGE_GLOBAL which we do not want for PTE.  Use
>> __default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.
> 
> Can you please explain what is your concern? Exposing more gadgets for
> speculative ROP attacks?
> 
> Or is it a general rule of not exposing any kernel code  more than
> absolutely necessary?

I think it's good practice to just expose only the *minimal* amount of
data necessary.  It's easier to audit and less likely to expose things
accidentall.

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Dave Hansen]

On 02/22/2018 01:27 PM, Nadav Amit wrote:
> Dave Hansen  wrote:
>> From: Dave Hansen 
>> In creating its page tables, the espfix code masks its PGTABLE_PROT
>> value with the supported mask: __supported_pte_mask.  This ensures
>> that unsupported bits are not set in the final PTE.  But, it also
>> sets _PAGE_GLOBAL which we do not want for PTE.  Use
>> __default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.
> 
> Can you please explain what is your concern? Exposing more gadgets for
> speculative ROP attacks?
> 
> Or is it a general rule of not exposing any kernel code  more than
> absolutely necessary?

I think it's good practice to just expose only the *minimal* amount of
data necessary.  It's easier to audit and less likely to expose things
accidentall.

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Nadav Amit]

Dave Hansen  wrote:

> 
> From: Dave Hansen 
> 
> In creating its page tables, the espfix code masks its PGTABLE_PROT
> value with the supported mask: __supported_pte_mask.  This ensures
> that unsupported bits are not set in the final PTE.  But, it also
> sets _PAGE_GLOBAL which we do not want for PTE.  Use
> __default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.

Can you please explain what is your concern? Exposing more gadgets for
speculative ROP attacks?

Or is it a general rule of not exposing any kernel code  more than
absolutely necessary?

Re: [RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Nadav Amit]

Dave Hansen  wrote:

> 
> From: Dave Hansen 
> 
> In creating its page tables, the espfix code masks its PGTABLE_PROT
> value with the supported mask: __supported_pte_mask.  This ensures
> that unsupported bits are not set in the final PTE.  But, it also
> sets _PAGE_GLOBAL which we do not want for PTE.  Use
> __default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.

Can you please explain what is your concern? Exposing more gadgets for
speculative ROP attacks?

Or is it a general rule of not exposing any kernel code  more than
absolutely necessary?

[RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Dave Hansen]

From: Dave Hansen 

In creating its page tables, the espfix code masks its PGTABLE_PROT
value with the supported mask: __supported_pte_mask.  This ensures
that unsupported bits are not set in the final PTE.  But, it also
sets _PAGE_GLOBAL which we do not want for PTE.  Use
__default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/kernel/espfix_64.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN arch/x86/kernel/espfix_64.c~espfix-use-kern-defaults-not-supported 
arch/x86/kernel/espfix_64.c
--- a/arch/x86/kernel/espfix_64.c~espfix-use-kern-defaults-not-supported
2018-02-22 12:36:19.217036552 -0800
+++ b/arch/x86/kernel/espfix_64.c   2018-02-22 12:36:19.221036552 -0800
@@ -167,7 +167,7 @@ void init_espfix_ap(int cpu)
goto unlock_done;
 
node = cpu_to_node(cpu);
-   ptemask = __supported_pte_mask;
+   ptemask = __default_kernel_pte_mask;
 
pud_p = _pud_page[pud_index(addr)];
pud = *pud_p;
_

[RFC][PATCH 04/10] x86/espfix: use kernel-default PTE mask

[Dave Hansen]

From: Dave Hansen 

In creating its page tables, the espfix code masks its PGTABLE_PROT
value with the supported mask: __supported_pte_mask.  This ensures
that unsupported bits are not set in the final PTE.  But, it also
sets _PAGE_GLOBAL which we do not want for PTE.  Use
__default_kernel_pte_mask instead which clears _PAGE_GLOBAL for PTI.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/kernel/espfix_64.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN arch/x86/kernel/espfix_64.c~espfix-use-kern-defaults-not-supported 
arch/x86/kernel/espfix_64.c
--- a/arch/x86/kernel/espfix_64.c~espfix-use-kern-defaults-not-supported
2018-02-22 12:36:19.217036552 -0800
+++ b/arch/x86/kernel/espfix_64.c   2018-02-22 12:36:19.221036552 -0800
@@ -167,7 +167,7 @@ void init_espfix_ap(int cpu)
goto unlock_done;
 
node = cpu_to_node(cpu);
-   ptemask = __supported_pte_mask;
+   ptemask = __default_kernel_pte_mask;
 
pud_p = _pud_page[pud_index(addr)];
pud = *pud_p;
_