* [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: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-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-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.