All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
@ 2023-01-08 21:26 Maciej W. Rozycki
  2023-01-09 10:40 ` Ingo Molnar
  2023-01-10 15:19 ` Jason A. Donenfeld
  0 siblings, 2 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-08 21:26 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin
  Cc: x86, linux-kernel

For x86 kernel stack offset randomization uses the RDTSC instruction, 
which causes an invalid opcode exception with hardware that does not 
implement this instruction:

process '/sbin/init' started with executable stack
invalid opcode: 0000 [#1]
CPU: 0 PID: 1 Comm: init Not tainted 6.1.0-rc4+ #1
EIP: exit_to_user_mode_prepare+0x90/0xe1
Code: 30 02 00 75 ad 0f ba e3 16 73 05 e8 a7 a5 fc ff 0f ba e3 0e 73 05 e8 3e af fc ff a1 c4 c6 51 c0 85 c0 7e 13 8b 0d ac 01 53 c0 <0f> 31 0f b6 c0 31 c1 89 0d ac 01 53 c0 83 3d 30 ed 62 c0 00 75 33
EAX: 00000001 EBX: 00004000 ECX: 00000000 EDX: 000004ff
ESI: c10253c0 EDI: 00000000 EBP: c1027f98 ESP: c1027f8c
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010002
CR0: 80050033 CR2: bfe8659b CR3: 012e0000 CR4: 00000000
Call Trace:
 ? rest_init+0x72/0x72
 syscall_exit_to_user_mode+0x15/0x27
 ret_from_fork+0x10/0x30
EIP: 0xb7f74800
Code: Unable to access opcode bytes at 0xb7f747d6.
EAX: 00000000 EBX: 00000000 ECX: 00000000 EDX: 00000000
ESI: 00000000 EDI: 00000000 EBP: 00000000 ESP: bfe864b0
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 007b EFLAGS: 00000200
---[ end trace 0000000000000000 ]---
EIP: exit_to_user_mode_prepare+0x90/0xe1
Code: 30 02 00 75 ad 0f ba e3 16 73 05 e8 a7 a5 fc ff 0f ba e3 0e 73 05 e8 3e af fc ff a1 c4 c6 51 c0 85 c0 7e 13 8b 0d ac 01 53 c0 <0f> 31 0f b6 c0 31 c1 89 0d ac 01 53 c0 83 3d 30 ed 62 c0 00 75 33
EAX: 00000001 EBX: 00004000 ECX: 00000000 EDX: 000004ff
ESI: c10253c0 EDI: 00000000 EBP: c1027f98 ESP: c1027f8c
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010002
CR0: 80050033 CR2: b7f747d6 CR3: 012e0000 CR4: 00000000
Kernel panic - not syncing: Fatal exception

Therefore do not use randomization where the CPU does not have the TSC 
feature.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
Changes from v1:

- Disable randomization at run time rather than in configuration.
---
 arch/x86/include/asm/entry-common.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

linux-x86-randomize-kstack-offset-tsc.diff
Index: linux-macro/arch/x86/include/asm/entry-common.h
===================================================================
--- linux-macro.orig/arch/x86/include/asm/entry-common.h
+++ linux-macro/arch/x86/include/asm/entry-common.h
@@ -5,6 +5,7 @@
 #include <linux/randomize_kstack.h>
 #include <linux/user-return-notifier.h>
 
+#include <asm/cpufeature.h>
 #include <asm/nospec-branch.h>
 #include <asm/io_bitmap.h>
 #include <asm/fpu/api.h>
@@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
 	 * Therefore, final stack offset entropy will be 5 (x86_64) or
 	 * 6 (ia32) bits.
 	 */
-	choose_random_kstack_offset(rdtsc() & 0xFF);
+	if (cpu_feature_enabled(X86_FEATURE_TSC))
+		choose_random_kstack_offset(rdtsc() & 0xFF);
 }
 #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
 

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-08 21:26 [PATCH v2] x86: Disable kernel stack offset randomization for !TSC Maciej W. Rozycki
@ 2023-01-09 10:40 ` Ingo Molnar
  2023-01-09 22:53   ` Maciej W. Rozycki
  2023-01-10 15:19 ` Jason A. Donenfeld
  1 sibling, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2023-01-09 10:40 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, x86, linux-kernel


* Maciej W. Rozycki <macro@orcam.me.uk> wrote:

> For x86 kernel stack offset randomization uses the RDTSC instruction, 
> which causes an invalid opcode exception with hardware that does not 
> implement this instruction:

> @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
>  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
>  	 * 6 (ia32) bits.
>  	 */
> -	choose_random_kstack_offset(rdtsc() & 0xFF);
> +	if (cpu_feature_enabled(X86_FEATURE_TSC))
> +		choose_random_kstack_offset(rdtsc() & 0xFF);
>  }

While this is an obscure corner case, falling back to 0 offset silently 
feels a bit wrong - could we at least attempt to generate some 
unpredictability in this case?

It's not genuine entropy, but we could pass in a value that varies from 
task to task and which is not an 'obviously known' constant value like the 
0 fallback?

For example the lowest 8 bits of the virtual page number of the current 
task plus the lowest 8 bits of jiffies should vary from task to task, has 
some time dependence and is cheap to compute:

	(((unsigned long)current >> 12) + jiffies) & 0xFF

This combined with the per-CPU forward storage of previous offsets:

#define choose_random_kstack_offset(rand) do {                          \
        if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, \
                                &randomize_kstack_offset)) {            \
                u32 offset = raw_cpu_read(kstack_offset);               \
                offset ^= (rand);                                       \
                raw_cpu_write(kstack_offset, offset);                   \
        }                                                               \

Should make this reasonably hard to guess for long-running tasks even if 
there's no TSC - and make it hard to guess even for tasks whose creation an 
attacker controls, unless there's an info-leak to rely on.

Thanks,

	Ingo

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-09 10:40 ` Ingo Molnar
@ 2023-01-09 22:53   ` Maciej W. Rozycki
  2023-01-10 10:47     ` Ingo Molnar
  0 siblings, 1 reply; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-09 22:53 UTC (permalink / raw)
  To: Jason A. Donenfeld, Ingo Molnar
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, x86, linux-kernel

Jason,

 Would you mind commenting on the below?

On Mon, 9 Jan 2023, Ingo Molnar wrote:

> > For x86 kernel stack offset randomization uses the RDTSC instruction, 
> > which causes an invalid opcode exception with hardware that does not 
> > implement this instruction:
> 
> > @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
> >  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
> >  	 * 6 (ia32) bits.
> >  	 */
> > -	choose_random_kstack_offset(rdtsc() & 0xFF);
> > +	if (cpu_feature_enabled(X86_FEATURE_TSC))
> > +		choose_random_kstack_offset(rdtsc() & 0xFF);
> >  }
> 
> While this is an obscure corner case, falling back to 0 offset silently 
> feels a bit wrong - could we at least attempt to generate some 
> unpredictability in this case?
> 
> It's not genuine entropy, but we could pass in a value that varies from 
> task to task and which is not an 'obviously known' constant value like the 
> 0 fallback?
> 
> For example the lowest 8 bits of the virtual page number of the current 
> task plus the lowest 8 bits of jiffies should vary from task to task, has 
> some time dependence and is cheap to compute:
> 
> 	(((unsigned long)current >> 12) + jiffies) & 0xFF
> 
> This combined with the per-CPU forward storage of previous offsets:
> 
> #define choose_random_kstack_offset(rand) do {                          \
>         if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, \
>                                 &randomize_kstack_offset)) {            \
>                 u32 offset = raw_cpu_read(kstack_offset);               \
>                 offset ^= (rand);                                       \
>                 raw_cpu_write(kstack_offset, offset);                   \
>         }                                                               \
> 
> Should make this reasonably hard to guess for long-running tasks even if 
> there's no TSC - and make it hard to guess even for tasks whose creation an 
> attacker controls, unless there's an info-leak to rely on.

 Sure, I'm fine implementing it, even in such a way so as not to cause a 
code size/performance regression for X86_TSC configurations.  But is the 
calculation really unpredictable enough?  I don't feel competent enough to 
decide.  Jason, what do you think?

  Maciej

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-09 22:53   ` Maciej W. Rozycki
@ 2023-01-10 10:47     ` Ingo Molnar
  2023-01-10 13:56       ` David Laight
  0 siblings, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2023-01-10 10:47 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Jason A. Donenfeld, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, x86, linux-kernel


* Maciej W. Rozycki <macro@orcam.me.uk> wrote:

> Jason,
> 
>  Would you mind commenting on the below?
> 
> On Mon, 9 Jan 2023, Ingo Molnar wrote:
> 
> > > For x86 kernel stack offset randomization uses the RDTSC instruction, 
> > > which causes an invalid opcode exception with hardware that does not 
> > > implement this instruction:
> > 
> > > @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
> > >  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
> > >  	 * 6 (ia32) bits.
> > >  	 */
> > > -	choose_random_kstack_offset(rdtsc() & 0xFF);
> > > +	if (cpu_feature_enabled(X86_FEATURE_TSC))
> > > +		choose_random_kstack_offset(rdtsc() & 0xFF);
> > >  }
> > 
> > While this is an obscure corner case, falling back to 0 offset silently 
> > feels a bit wrong - could we at least attempt to generate some 
> > unpredictability in this case?
> > 
> > It's not genuine entropy, but we could pass in a value that varies from 
> > task to task and which is not an 'obviously known' constant value like the 
> > 0 fallback?
> > 
> > For example the lowest 8 bits of the virtual page number of the current 
> > task plus the lowest 8 bits of jiffies should vary from task to task, has 
> > some time dependence and is cheap to compute:
> > 
> > 	(((unsigned long)current >> 12) + jiffies) & 0xFF
> > 
> > This combined with the per-CPU forward storage of previous offsets:
> > 
> > #define choose_random_kstack_offset(rand) do {                          \
> >         if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, \
> >                                 &randomize_kstack_offset)) {            \
> >                 u32 offset = raw_cpu_read(kstack_offset);               \
> >                 offset ^= (rand);                                       \
> >                 raw_cpu_write(kstack_offset, offset);                   \
> >         }                                                               \
> > 
> > Should make this reasonably hard to guess for long-running tasks even if 
> > there's no TSC - and make it hard to guess even for tasks whose creation an 
> > attacker controls, unless there's an info-leak to rely on.
> 
> Sure, I'm fine implementing it, even in such a way so as not to cause a 
> code size/performance regression for X86_TSC configurations.  But is the 
> calculation really unpredictable enough? [...]

It's not binary: it's obviously not as good as a TSC, but my point is that 
'something cheap & variable' is clearly better than 'zero offset all the 
time'.

Thanks,

	Ingo

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

* RE: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-10 10:47     ` Ingo Molnar
@ 2023-01-10 13:56       ` David Laight
  0 siblings, 0 replies; 13+ messages in thread
From: David Laight @ 2023-01-10 13:56 UTC (permalink / raw)
  To: 'Ingo Molnar', Maciej W. Rozycki
  Cc: Jason A. Donenfeld, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, x86, linux-kernel

From: Ingo Molnar
> Sent: 10 January 2023 10:47
> 
> 
> * Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> 
> > Jason,
> >
> >  Would you mind commenting on the below?
> >
> > On Mon, 9 Jan 2023, Ingo Molnar wrote:
> >
> > > > For x86 kernel stack offset randomization uses the RDTSC instruction,
> > > > which causes an invalid opcode exception with hardware that does not
> > > > implement this instruction:
> > >
> > > > @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
> > > >  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
> > > >  	 * 6 (ia32) bits.
> > > >  	 */
> > > > -	choose_random_kstack_offset(rdtsc() & 0xFF);
> > > > +	if (cpu_feature_enabled(X86_FEATURE_TSC))
> > > > +		choose_random_kstack_offset(rdtsc() & 0xFF);
> > > >  }
> > >
> > > While this is an obscure corner case, falling back to 0 offset silently
> > > feels a bit wrong - could we at least attempt to generate some
> > > unpredictability in this case?
> > >
> > > It's not genuine entropy, but we could pass in a value that varies from
> > > task to task and which is not an 'obviously known' constant value like the
> > > 0 fallback?
> > >
> > > For example the lowest 8 bits of the virtual page number of the current
> > > task plus the lowest 8 bits of jiffies should vary from task to task, has
> > > some time dependence and is cheap to compute:
> > >
> > > 	(((unsigned long)current >> 12) + jiffies) & 0xFF
> > >
> > > This combined with the per-CPU forward storage of previous offsets:
> > >
> > > #define choose_random_kstack_offset(rand) do {                          \
> > >         if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, \
> > >                                 &randomize_kstack_offset)) {            \
> > >                 u32 offset = raw_cpu_read(kstack_offset);               \
> > >                 offset ^= (rand);                                       \
> > >                 raw_cpu_write(kstack_offset, offset);                   \
> > >         }                                                               \
> > >
> > > Should make this reasonably hard to guess for long-running tasks even if
> > > there's no TSC - and make it hard to guess even for tasks whose creation an
> > > attacker controls, unless there's an info-leak to rely on.
> >
> > Sure, I'm fine implementing it, even in such a way so as not to cause a
> > code size/performance regression for X86_TSC configurations.  But is the
> > calculation really unpredictable enough? [...]
> 
> It's not binary: it's obviously not as good as a TSC, but my point is that
> 'something cheap & variable' is clearly better than 'zero offset all the
> time'.

Does it really matter if running on anything as old as a real 486?
In reality they'll only be used for testing.
There are more modern 486-class cpu for embedded use, but they
almost certainly have a TSC.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-08 21:26 [PATCH v2] x86: Disable kernel stack offset randomization for !TSC Maciej W. Rozycki
  2023-01-09 10:40 ` Ingo Molnar
@ 2023-01-10 15:19 ` Jason A. Donenfeld
  2023-01-12  1:34   ` Maciej W. Rozycki
  1 sibling, 1 reply; 13+ messages in thread
From: Jason A. Donenfeld @ 2023-01-10 15:19 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, x86, linux-kernel

On Sun, Jan 08, 2023 at 09:26:11PM +0000, Maciej W. Rozycki wrote:
> For x86 kernel stack offset randomization uses the RDTSC instruction, 
> which causes an invalid opcode exception with hardware that does not 
> implement this instruction:
> 
> process '/sbin/init' started with executable stack
> invalid opcode: 0000 [#1]
> CPU: 0 PID: 1 Comm: init Not tainted 6.1.0-rc4+ #1
> EIP: exit_to_user_mode_prepare+0x90/0xe1
> Code: 30 02 00 75 ad 0f ba e3 16 73 05 e8 a7 a5 fc ff 0f ba e3 0e 73 05 e8 3e af fc ff a1 c4 c6 51 c0 85 c0 7e 13 8b 0d ac 01 53 c0 <0f> 31 0f b6 c0 31 c1 89 0d ac 01 53 c0 83 3d 30 ed 62 c0 00 75 33
> EAX: 00000001 EBX: 00004000 ECX: 00000000 EDX: 000004ff
> ESI: c10253c0 EDI: 00000000 EBP: c1027f98 ESP: c1027f8c
> DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010002
> CR0: 80050033 CR2: bfe8659b CR3: 012e0000 CR4: 00000000
> Call Trace:
>  ? rest_init+0x72/0x72
>  syscall_exit_to_user_mode+0x15/0x27
>  ret_from_fork+0x10/0x30
> EIP: 0xb7f74800
> Code: Unable to access opcode bytes at 0xb7f747d6.
> EAX: 00000000 EBX: 00000000 ECX: 00000000 EDX: 00000000
> ESI: 00000000 EDI: 00000000 EBP: 00000000 ESP: bfe864b0
> DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 007b EFLAGS: 00000200
> ---[ end trace 0000000000000000 ]---
> EIP: exit_to_user_mode_prepare+0x90/0xe1
> Code: 30 02 00 75 ad 0f ba e3 16 73 05 e8 a7 a5 fc ff 0f ba e3 0e 73 05 e8 3e af fc ff a1 c4 c6 51 c0 85 c0 7e 13 8b 0d ac 01 53 c0 <0f> 31 0f b6 c0 31 c1 89 0d ac 01 53 c0 83 3d 30 ed 62 c0 00 75 33
> EAX: 00000001 EBX: 00004000 ECX: 00000000 EDX: 000004ff
> ESI: c10253c0 EDI: 00000000 EBP: c1027f98 ESP: c1027f8c
> DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010002
> CR0: 80050033 CR2: b7f747d6 CR3: 012e0000 CR4: 00000000
> Kernel panic - not syncing: Fatal exception
> 
> Therefore do not use randomization where the CPU does not have the TSC 
> feature.
> 
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> ---
> Changes from v1:
> 
> - Disable randomization at run time rather than in configuration.
> ---
>  arch/x86/include/asm/entry-common.h |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> linux-x86-randomize-kstack-offset-tsc.diff
> Index: linux-macro/arch/x86/include/asm/entry-common.h
> ===================================================================
> --- linux-macro.orig/arch/x86/include/asm/entry-common.h
> +++ linux-macro/arch/x86/include/asm/entry-common.h
> @@ -5,6 +5,7 @@
>  #include <linux/randomize_kstack.h>
>  #include <linux/user-return-notifier.h>
>  
> +#include <asm/cpufeature.h>
>  #include <asm/nospec-branch.h>
>  #include <asm/io_bitmap.h>
>  #include <asm/fpu/api.h>
> @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
>  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
>  	 * 6 (ia32) bits.
>  	 */
> -	choose_random_kstack_offset(rdtsc() & 0xFF);
> +	if (cpu_feature_enabled(X86_FEATURE_TSC))
> +		choose_random_kstack_offset(rdtsc() & 0xFF);

What would happen if you just called `get_random_u8()` here?

Jason

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-10 15:19 ` Jason A. Donenfeld
@ 2023-01-12  1:34   ` Maciej W. Rozycki
  2023-01-12  1:53     ` H. Peter Anvin
  2023-01-13 15:33     ` Jason A. Donenfeld
  0 siblings, 2 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-12  1:34 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, x86, linux-kernel

On Tue, 10 Jan 2023, Jason A. Donenfeld wrote:

> > Index: linux-macro/arch/x86/include/asm/entry-common.h
> > ===================================================================
> > --- linux-macro.orig/arch/x86/include/asm/entry-common.h
> > +++ linux-macro/arch/x86/include/asm/entry-common.h
> > @@ -5,6 +5,7 @@
> >  #include <linux/randomize_kstack.h>
> >  #include <linux/user-return-notifier.h>
> >  
> > +#include <asm/cpufeature.h>
> >  #include <asm/nospec-branch.h>
> >  #include <asm/io_bitmap.h>
> >  #include <asm/fpu/api.h>
> > @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
> >  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
> >  	 * 6 (ia32) bits.
> >  	 */
> > -	choose_random_kstack_offset(rdtsc() & 0xFF);
> > +	if (cpu_feature_enabled(X86_FEATURE_TSC))
> > +		choose_random_kstack_offset(rdtsc() & 0xFF);
> 
> What would happen if you just called `get_random_u8()` here?

 Thank you for your input.  I've had a look at the function and it seems a 
bit heavyweight compared to a mere single CPU instruction, but I guess why 
not.  Do you have any performance figures (in terms of CPU cycles) for the 
usual cases?  Offhand I'm not sure how I could benchmark it myself.

 I have made a patch and of course it makes the system boot too, although 
it's not clear to me how I can actually verify randomisation works.  I can 
assume it does I suppose.

  Maciej

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-12  1:34   ` Maciej W. Rozycki
@ 2023-01-12  1:53     ` H. Peter Anvin
  2023-01-12 11:30       ` Borislav Petkov
  2023-01-30 20:43       ` Maciej W. Rozycki
  2023-01-13 15:33     ` Jason A. Donenfeld
  1 sibling, 2 replies; 13+ messages in thread
From: H. Peter Anvin @ 2023-01-12  1:53 UTC (permalink / raw)
  To: Maciej W. Rozycki, Jason A. Donenfeld
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	linux-kernel

On January 11, 2023 5:34:29 PM PST, "Maciej W. Rozycki" <macro@orcam.me.uk> wrote:
>On Tue, 10 Jan 2023, Jason A. Donenfeld wrote:
>
>> > Index: linux-macro/arch/x86/include/asm/entry-common.h
>> > ===================================================================
>> > --- linux-macro.orig/arch/x86/include/asm/entry-common.h
>> > +++ linux-macro/arch/x86/include/asm/entry-common.h
>> > @@ -5,6 +5,7 @@
>> >  #include <linux/randomize_kstack.h>
>> >  #include <linux/user-return-notifier.h>
>> >  
>> > +#include <asm/cpufeature.h>
>> >  #include <asm/nospec-branch.h>
>> >  #include <asm/io_bitmap.h>
>> >  #include <asm/fpu/api.h>
>> > @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
>> >  	 * Therefore, final stack offset entropy will be 5 (x86_64) or
>> >  	 * 6 (ia32) bits.
>> >  	 */
>> > -	choose_random_kstack_offset(rdtsc() & 0xFF);
>> > +	if (cpu_feature_enabled(X86_FEATURE_TSC))
>> > +		choose_random_kstack_offset(rdtsc() & 0xFF);
>> 
>> What would happen if you just called `get_random_u8()` here?
>
> Thank you for your input.  I've had a look at the function and it seems a 
>bit heavyweight compared to a mere single CPU instruction, but I guess why 
>not.  Do you have any performance figures (in terms of CPU cycles) for the 
>usual cases?  Offhand I'm not sure how I could benchmark it myself.
>
> I have made a patch and of course it makes the system boot too, although 
>it's not clear to me how I can actually verify randomisation works.  I can 
>assume it does I suppose.
>
>  Maciej

Not to mention that we could use rdrand here if it is available (although it is slower than rdtsc.)

RDTSC isn't a super fast instruction either, but what is *way* more significant is that this use of RDTSC is NOT safe: in certain power states it may very well be that stone number of lower bits of TSC contain no entropy at all.

At the very least one should do a rotating multiply with a large (32-bit) prime number.

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-12  1:53     ` H. Peter Anvin
@ 2023-01-12 11:30       ` Borislav Petkov
  2023-01-12 11:58         ` Maciej W. Rozycki
  2023-01-30 20:43       ` Maciej W. Rozycki
  1 sibling, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2023-01-12 11:30 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Maciej W. Rozycki, Jason A. Donenfeld, Thomas Gleixner,
	Ingo Molnar, Dave Hansen, x86, linux-kernel

On Wed, Jan 11, 2023 at 05:53:37PM -0800, H. Peter Anvin wrote:
> Not to mention that we could use rdrand here if it is available (although it
> is slower than rdtsc.)

Yeah, no RDRAND on 486. :)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-12 11:30       ` Borislav Petkov
@ 2023-01-12 11:58         ` Maciej W. Rozycki
  0 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-12 11:58 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, Jason A. Donenfeld, Thomas Gleixner, Ingo Molnar,
	Dave Hansen, x86, linux-kernel

On Thu, 12 Jan 2023, Borislav Petkov wrote:

> > Not to mention that we could use rdrand here if it is available (although it
> > is slower than rdtsc.)
> 
> Yeah, no RDRAND on 486. :)

 And not even in my 2009 copy of the SDM (the 2013 one does have it), so 
it must have arrived slightly later than just after the 486. ;)

  Maciej

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-12  1:34   ` Maciej W. Rozycki
  2023-01-12  1:53     ` H. Peter Anvin
@ 2023-01-13 15:33     ` Jason A. Donenfeld
  2023-01-30 20:43       ` Maciej W. Rozycki
  1 sibling, 1 reply; 13+ messages in thread
From: Jason A. Donenfeld @ 2023-01-13 15:33 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, x86, linux-kernel

Hi Maciej,

On Thu, Jan 12, 2023 at 2:34 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Tue, 10 Jan 2023, Jason A. Donenfeld wrote:
>
> > > Index: linux-macro/arch/x86/include/asm/entry-common.h
> > > ===================================================================
> > > --- linux-macro.orig/arch/x86/include/asm/entry-common.h
> > > +++ linux-macro/arch/x86/include/asm/entry-common.h
> > > @@ -5,6 +5,7 @@
> > >  #include <linux/randomize_kstack.h>
> > >  #include <linux/user-return-notifier.h>
> > >
> > > +#include <asm/cpufeature.h>
> > >  #include <asm/nospec-branch.h>
> > >  #include <asm/io_bitmap.h>
> > >  #include <asm/fpu/api.h>
> > > @@ -85,7 +86,8 @@ static inline void arch_exit_to_user_mod
> > >      * Therefore, final stack offset entropy will be 5 (x86_64) or
> > >      * 6 (ia32) bits.
> > >      */
> > > -   choose_random_kstack_offset(rdtsc() & 0xFF);
> > > +   if (cpu_feature_enabled(X86_FEATURE_TSC))
> > > +           choose_random_kstack_offset(rdtsc() & 0xFF);
> >
> > What would happen if you just called `get_random_u8()` here?
>
>  Thank you for your input.  I've had a look at the function and it seems a
> bit heavyweight compared to a mere single CPU instruction, but I guess why
> not.  Do you have any performance figures (in terms of CPU cycles) for the
> usual cases?  Offhand I'm not sure how I could benchmark it myself.

Generally it's very very fast, as most cases wind up being only a
memcpy -- in this case, a single byte copy. So by and large it should
be suitable. It's fast enough now that most networking things are able
to use it. And lots of other places where you'd want really high
performance. So I'd expect it's okay to use here too. And if it is too
slow, we should figure out how to make it faster. But I don't suspect
it'll be too slow.

Resist calls to use RDRAND directly (it's much much slower, and not
universally available) or to roll your own opencoded bespoke RNG.

Jason

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-12  1:53     ` H. Peter Anvin
  2023-01-12 11:30       ` Borislav Petkov
@ 2023-01-30 20:43       ` Maciej W. Rozycki
  1 sibling, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-30 20:43 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Jason A. Donenfeld, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, linux-kernel

On Wed, 11 Jan 2023, H. Peter Anvin wrote:

> RDTSC isn't a super fast instruction either,

 As someone recently mostly involved with RISC architectures I find it 
interesting indeed, given that the TSC is just some kind of an integer 
register (or data latch).

 E.g. with the MIPS $c0_count register, which is a free-running counter 
similar to the TSC, the "MFC0 reg, $c0_count" instruction executes just 
about as any ordinary ALU operation, such as say ADD (there is no plain 
GPR move instruction in the MIPS ISA to compare this special register move 
to).  Yes, the latency may be two clocks rather than one, but that's still 
pretty damn fast and the extra latency can be dealt with even on scalar 
microarchitectures by reordering the data consumer farther away from the 
producer.

> but what is *way* more 
> significant is that this use of RDTSC is NOT safe: in certain power 
> states it may very well be that stone number of lower bits of TSC 
> contain no entropy at all.

 I wasn't aware of this limitation; certainly at its introduction TSC was 
just a free-running counter with no special states.

 I went after Jason's suggestion to use `get_random_u8' then, which is 
both portable and the single place to make sure proper entropy is 
maintained in.  Thank you for your input.

  Maciej

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

* Re: [PATCH v2] x86: Disable kernel stack offset randomization for !TSC
  2023-01-13 15:33     ` Jason A. Donenfeld
@ 2023-01-30 20:43       ` Maciej W. Rozycki
  0 siblings, 0 replies; 13+ messages in thread
From: Maciej W. Rozycki @ 2023-01-30 20:43 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, x86, linux-kernel

Hi Jason,

> >  Thank you for your input.  I've had a look at the function and it seems a
> > bit heavyweight compared to a mere single CPU instruction, but I guess why
> > not.  Do you have any performance figures (in terms of CPU cycles) for the
> > usual cases?  Offhand I'm not sure how I could benchmark it myself.
> 
> Generally it's very very fast, as most cases wind up being only a
> memcpy -- in this case, a single byte copy. So by and large it should
> be suitable. It's fast enough now that most networking things are able
> to use it. And lots of other places where you'd want really high
> performance. So I'd expect it's okay to use here too. And if it is too
> slow, we should figure out how to make it faster. But I don't suspect
> it'll be too slow.

 Thank you for your explanation.  I have v3 ready for submission; would 
you mind if I added you with a Suggested-by: tag?

  Maciej

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

end of thread, other threads:[~2023-01-30 20:44 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-08 21:26 [PATCH v2] x86: Disable kernel stack offset randomization for !TSC Maciej W. Rozycki
2023-01-09 10:40 ` Ingo Molnar
2023-01-09 22:53   ` Maciej W. Rozycki
2023-01-10 10:47     ` Ingo Molnar
2023-01-10 13:56       ` David Laight
2023-01-10 15:19 ` Jason A. Donenfeld
2023-01-12  1:34   ` Maciej W. Rozycki
2023-01-12  1:53     ` H. Peter Anvin
2023-01-12 11:30       ` Borislav Petkov
2023-01-12 11:58         ` Maciej W. Rozycki
2023-01-30 20:43       ` Maciej W. Rozycki
2023-01-13 15:33     ` Jason A. Donenfeld
2023-01-30 20:43       ` Maciej W. Rozycki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.