Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:35:45PM -0800, Tim Chen wrote: > time may not provide full protection on all cpu models. All right no problem at all, it's fixed up. Until very recently the majority of microcodes wasn't available in the first place so I guess it's no big issue if in a subset of those the IBRS barrier-like behavior wasn't immediately fully leveraged in all cases. I'm just glad this detail was clarified sooner than later. The IBRS barrier-like behavior and need to be set even when it's already set, when changing mode to an higher privilege, is crystal clear now. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:35:45PM -0800, Tim Chen wrote: > time may not provide full protection on all cpu models. All right no problem at all, it's fixed up. Until very recently the majority of microcodes wasn't available in the first place so I guess it's no big issue if in a subset of those the IBRS barrier-like behavior wasn't immediately fully leveraged in all cases. I'm just glad this detail was clarified sooner than later. The IBRS barrier-like behavior and need to be set even when it's already set, when changing mode to an higher privilege, is crystal clear now. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On 01/10/2018 05:53 AM, Van De Ven, Arjan wrote: >> ibrs_enabled 2: >> >> sets IBRS always in host > > this is not secure > >> This matches the semantics described here by Tim patchset on lkml: >> >> https://marc.info/?l=linux-kernel=151520606320646 > > I will talk to Tim, it's not right. > > Yes, there's a misunderstanding on my part. Leaving IBRS=1 all the time may not provide full protection on all cpu models. Tim
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On 01/10/2018 05:53 AM, Van De Ven, Arjan wrote: >> ibrs_enabled 2: >> >> sets IBRS always in host > > this is not secure > >> This matches the semantics described here by Tim patchset on lkml: >> >> https://marc.info/?l=linux-kernel=151520606320646 > > I will talk to Tim, it's not right. > > Yes, there's a misunderstanding on my part. Leaving IBRS=1 all the time may not provide full protection on all cpu models. Tim
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 16:47 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 03:24:17PM +, David Woodhouse wrote: > > Since it achieves nothing¹ but to make userspace run slower, there's no > > need to write it again on returning to userspace. It will perform that > > function just fine without doing so. > > Ok, very glad we are on the same page now. > > Note that as far as I can tell there was no way to answer the above > question by reading the spec. The spec does, I concede, leave something to be desired. This sentence in particular — and it really is a single sentence — caused me to throw my toys out of the pram and demand verbal explanations (which is perhaps the only reason I managed to work it out): "If IBRS is set, near returns and near indirect jumps/calls will not allow their speculative target address to be controlled by code that executed in a less privileged prediction mode before the IBRS mode was last written with a value of 1 or on another logical processor so long as all RSB entries from the previous less privileged prediction mode are overwritten" But it *does* have the words "before the IBRS mode was last written with a value of 1" in there somewhere... ;) smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 16:47 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 03:24:17PM +, David Woodhouse wrote: > > Since it achieves nothing¹ but to make userspace run slower, there's no > > need to write it again on returning to userspace. It will perform that > > function just fine without doing so. > > Ok, very glad we are on the same page now. > > Note that as far as I can tell there was no way to answer the above > question by reading the spec. The spec does, I concede, leave something to be desired. This sentence in particular — and it really is a single sentence — caused me to throw my toys out of the pram and demand verbal explanations (which is perhaps the only reason I managed to work it out): "If IBRS is set, near returns and near indirect jumps/calls will not allow their speculative target address to be controlled by code that executed in a less privileged prediction mode before the IBRS mode was last written with a value of 1 or on another logical processor so long as all RSB entries from the previous less privileged prediction mode are overwritten" But it *does* have the words "before the IBRS mode was last written with a value of 1" in there somewhere... ;) smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 03:24:17PM +, David Woodhouse wrote: > Since it achieves nothing¹ but to make userspace run slower, there's no > need to write it again on returning to userspace. It will perform that > function just fine without doing so. Ok, very glad we are on the same page now. Note that as far as I can tell there was no way to answer the above question by reading the spec. You also explicitly used the word barrier in association with IBRS before, but there was no word barrier in the aforementioned specs in association with IBRS (every word barrier was always and only in association with IBPB). I hope this discussion helped clear the additional barrier semantics of IBRS in more understandable way for the current/future upstream code. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 03:24:17PM +, David Woodhouse wrote: > Since it achieves nothing¹ but to make userspace run slower, there's no > need to write it again on returning to userspace. It will perform that > function just fine without doing so. Ok, very glad we are on the same page now. Note that as far as I can tell there was no way to answer the above question by reading the spec. You also explicitly used the word barrier in association with IBRS before, but there was no word barrier in the aforementioned specs in association with IBRS (every word barrier was always and only in association with IBPB). I hope this discussion helped clear the additional barrier semantics of IBRS in more understandable way for the current/future upstream code. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 16:13 +0100, Andrea Arcangeli wrote: > > Can you also tell if IBRS must be written as a barrier to SPEC_CTRL in > return to userland (kernel exit) when ibrs_enabled 2? Generally we > wouldn't run a barrier there with ibrs_enabled 2, but absolutely > nothing is intuitive here so I need to ask explicitly. Since it achieves nothing¹ but to make userspace run slower, there's no need to write it again on returning to userspace. It will perform that function just fine without doing so. (¹ that STIBP doesn't already do, and cheaper) smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 16:13 +0100, Andrea Arcangeli wrote: > > Can you also tell if IBRS must be written as a barrier to SPEC_CTRL in > return to userland (kernel exit) when ibrs_enabled 2? Generally we > wouldn't run a barrier there with ibrs_enabled 2, but absolutely > nothing is intuitive here so I need to ask explicitly. Since it achieves nothing¹ but to make userspace run slower, there's no need to write it again on returning to userspace. It will perform that function just fine without doing so. (¹ that STIBP doesn't already do, and cheaper) smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 06:59:54AM -0800, Dave Hansen wrote: > On 01/10/2018 06:10 AM, Andrea Arcangeli wrote: > > Tim and Dave please comment too, Tim you originally wrote that code > > that leaves IBRS always on and never toggles it in the kernel entry > > point so you must know full well if Arjan is correct that you must > > toggle IBRS every time you enter kernel and in turn ibrs_enabled 2 > > isn't valid mode. > > Hi Andrea, > > The "writing IBRS=1 acts as a barrier when it is already IBRS=1" > behavior is something which I misunderstood in the past. Thanks, Arjan, > for clearing it up. "writing IBRS=1 acts as a barrier when it is already IBRS=1" would have been much clearer wording frankly. IBPB is IBP "Barrier", but also IBRS is a barrier, no problem :). So we'll add a dummy IBRS write to SPEC_CTRL in kernel entry and vmexit so that it is compliant with all released microcodes that may require it, also when ibrs_enabled is 2. Can you confirm? Can you also tell if IBRS must be written as a barrier to SPEC_CTRL in return to userland (kernel exit) when ibrs_enabled 2? Generally we wouldn't run a barrier there with ibrs_enabled 2, but absolutely nothing is intuitive here so I need to ask explicitly.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 06:59:54AM -0800, Dave Hansen wrote: > On 01/10/2018 06:10 AM, Andrea Arcangeli wrote: > > Tim and Dave please comment too, Tim you originally wrote that code > > that leaves IBRS always on and never toggles it in the kernel entry > > point so you must know full well if Arjan is correct that you must > > toggle IBRS every time you enter kernel and in turn ibrs_enabled 2 > > isn't valid mode. > > Hi Andrea, > > The "writing IBRS=1 acts as a barrier when it is already IBRS=1" > behavior is something which I misunderstood in the past. Thanks, Arjan, > for clearing it up. "writing IBRS=1 acts as a barrier when it is already IBRS=1" would have been much clearer wording frankly. IBPB is IBP "Barrier", but also IBRS is a barrier, no problem :). So we'll add a dummy IBRS write to SPEC_CTRL in kernel entry and vmexit so that it is compliant with all released microcodes that may require it, also when ibrs_enabled is 2. Can you confirm? Can you also tell if IBRS must be written as a barrier to SPEC_CTRL in return to userland (kernel exit) when ibrs_enabled 2? Generally we wouldn't run a barrier there with ibrs_enabled 2, but absolutely nothing is intuitive here so I need to ask explicitly.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On 01/10/2018 06:10 AM, Andrea Arcangeli wrote: > Tim and Dave please comment too, Tim you originally wrote that code > that leaves IBRS always on and never toggles it in the kernel entry > point so you must know full well if Arjan is correct that you must > toggle IBRS every time you enter kernel and in turn ibrs_enabled 2 > isn't valid mode. Hi Andrea, The "writing IBRS=1 acts as a barrier when it is already IBRS=1" behavior is something which I misunderstood in the past. Thanks, Arjan, for clearing it up.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On 01/10/2018 06:10 AM, Andrea Arcangeli wrote: > Tim and Dave please comment too, Tim you originally wrote that code > that leaves IBRS always on and never toggles it in the kernel entry > point so you must know full well if Arjan is correct that you must > toggle IBRS every time you enter kernel and in turn ibrs_enabled 2 > isn't valid mode. Hi Andrea, The "writing IBRS=1 acts as a barrier when it is already IBRS=1" behavior is something which I misunderstood in the past. Thanks, Arjan, for clearing it up.
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> Hello, > > On Wed, Jan 10, 2018 at 02:46:22PM +0100, Thomas Gleixner wrote: > > So here is the simple list of questions all to be answered with YES or > > NO. I don't want to see any of the 'but, though ...'. We all know by now > > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > > future CPUs. So get your act together and tell a clear YES or NO. > > Other comments/code from Tim Chen, and Dave Hansen and most important > the ibrs_enabled 2 description and implementation on lkml, makes me > still wonder if even Arjan may have misunderstood some detail about > IBRS semantics too. I spent the better part of the last 6 months in dungeons with CPU designers trying to to figure out what we could and could not do. I'm pretty darn sure I know the details.
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> Hello, > > On Wed, Jan 10, 2018 at 02:46:22PM +0100, Thomas Gleixner wrote: > > So here is the simple list of questions all to be answered with YES or > > NO. I don't want to see any of the 'but, though ...'. We all know by now > > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > > future CPUs. So get your act together and tell a clear YES or NO. > > Other comments/code from Tim Chen, and Dave Hansen and most important > the ibrs_enabled 2 description and implementation on lkml, makes me > still wonder if even Arjan may have misunderstood some detail about > IBRS semantics too. I spent the better part of the last 6 months in dungeons with CPU designers trying to to figure out what we could and could not do. I'm pretty darn sure I know the details.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
Hello, On Wed, Jan 10, 2018 at 02:46:22PM +0100, Thomas Gleixner wrote: > So here is the simple list of questions all to be answered with YES or > NO. I don't want to see any of the 'but, though ...'. We all know by now > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > future CPUs. So get your act together and tell a clear YES or NO. Other comments/code from Tim Chen, and Dave Hansen and most important the ibrs_enabled 2 description and implementation on lkml, makes me still wonder if even Arjan may have misunderstood some detail about IBRS semantics too. Tim and Dave please comment too, Tim you originally wrote that code that leaves IBRS always on and never toggles it in the kernel entry point so you must know full well if Arjan is correct that you must toggle IBRS every time you enter kernel and in turn ibrs_enabled 2 isn't valid mode. I can answer my understanding of IBRS so far that would make perfect sense if that code posted made sense in the first place... https://marc.info/?l=linux-kernel=151520606320646 > > 1) Does IBRS=1 when set once act as a set-and-forget option ? Yes it can. Once set makes spectre variant#2 go away but only for IBP BTB, stuff_RSB still needed and register hygiene too. > > 1a) If the answer to #1 is yes, is it more secure than toggling it? It's equivalent, not more secure nor less secure. Simply IBRS only protects the code that runs with IBRS set. So you if you leave it always set, you protect more code. IBRS set is equivalent to the code having been compiled with reptoline on CPUS where reptoline is equaivalent to IBRS. The act of setting IBRS does nothing, it's not a barrier (IBPB is a barrier and flushes the CPU internal state). After IBRS is set the code is like if it's built with reptoline. IBRS may reduce the performance measurably and this is why it has to be disabled in userland and that is the motivation for using ibrs_enabled 1 the default. The toggling at kernel entry is purely required so userland keeps running fast with IBRS not set by default. But it would be more secure to leave IBRS always on which is what ibrs_enabled 2, it's just incredibly slower with current silicon in some microcode implementations, which is why it's not the default. > > 1b) If the answer to #1 is yes, is retpoline required ? reptoline is definitely not required whenever IBRS is set. IBRS set if something is more secure than repotline as it's sure thing variant#2 is fixed and the microcode was applied if IBRS could be set. > > 1c) If the answer to #1 is yes, is RSB stuffing required ? RBS is totally different piece of the CPU not covered by IBRS AFIK. I'm not sure about this though but doing also stuff_RSB wouldn't hurt if the CPU has no microcode update applied. SMEP set in cr4 obviates the need of stuff_RSB in kernel entry but not in vmexit. > 2) Does toggle mode of IBRS require retpoline ? Toggle mode means nothing in terms of added security. Toggle mode simply protects only kernel space. If you use the toggle mode (ibrs_enabled 1 default) only kernel is protected by IBRS and in turn you don't have to build the kernel with reptoline. If you leave IBRS always on it's effectively like having built everything (kernel and all userland) with reptoline. > 3) Does toggle mode of IBRS require RSB stuffing ? Yes, again IBRS does nothing to prevent the RSB stuffing. Only SMEP helps there if set in cr4 but only for kernel entry, vmexit still needs stuff_RSB. The stuff_RSB part I'm no certain but again it won't hurt to do it and we do it even with IBRS on. It's not measurable though. > 4) Exist CPUs which require IBRS to be selected automatically ? > >4b) If yes, provide the list as a separate answer please Not sure if I get the question so not answering this one. Future CPUs will have a new CPUID bit where enabling IBRS will perform better than toggling it at kernel entry, so in those CPUs the most secure mode (ibrs_enabled 2) that leaves IBRS always on, will perform even better than the current default (ibrs_enabled 1). So in presence of such future CPUID bit we should prefer ibrs_enabled 2 as default and leave IBRS always on as default. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
Hello, On Wed, Jan 10, 2018 at 02:46:22PM +0100, Thomas Gleixner wrote: > So here is the simple list of questions all to be answered with YES or > NO. I don't want to see any of the 'but, though ...'. We all know by now > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > future CPUs. So get your act together and tell a clear YES or NO. Other comments/code from Tim Chen, and Dave Hansen and most important the ibrs_enabled 2 description and implementation on lkml, makes me still wonder if even Arjan may have misunderstood some detail about IBRS semantics too. Tim and Dave please comment too, Tim you originally wrote that code that leaves IBRS always on and never toggles it in the kernel entry point so you must know full well if Arjan is correct that you must toggle IBRS every time you enter kernel and in turn ibrs_enabled 2 isn't valid mode. I can answer my understanding of IBRS so far that would make perfect sense if that code posted made sense in the first place... https://marc.info/?l=linux-kernel=151520606320646 > > 1) Does IBRS=1 when set once act as a set-and-forget option ? Yes it can. Once set makes spectre variant#2 go away but only for IBP BTB, stuff_RSB still needed and register hygiene too. > > 1a) If the answer to #1 is yes, is it more secure than toggling it? It's equivalent, not more secure nor less secure. Simply IBRS only protects the code that runs with IBRS set. So you if you leave it always set, you protect more code. IBRS set is equivalent to the code having been compiled with reptoline on CPUS where reptoline is equaivalent to IBRS. The act of setting IBRS does nothing, it's not a barrier (IBPB is a barrier and flushes the CPU internal state). After IBRS is set the code is like if it's built with reptoline. IBRS may reduce the performance measurably and this is why it has to be disabled in userland and that is the motivation for using ibrs_enabled 1 the default. The toggling at kernel entry is purely required so userland keeps running fast with IBRS not set by default. But it would be more secure to leave IBRS always on which is what ibrs_enabled 2, it's just incredibly slower with current silicon in some microcode implementations, which is why it's not the default. > > 1b) If the answer to #1 is yes, is retpoline required ? reptoline is definitely not required whenever IBRS is set. IBRS set if something is more secure than repotline as it's sure thing variant#2 is fixed and the microcode was applied if IBRS could be set. > > 1c) If the answer to #1 is yes, is RSB stuffing required ? RBS is totally different piece of the CPU not covered by IBRS AFIK. I'm not sure about this though but doing also stuff_RSB wouldn't hurt if the CPU has no microcode update applied. SMEP set in cr4 obviates the need of stuff_RSB in kernel entry but not in vmexit. > 2) Does toggle mode of IBRS require retpoline ? Toggle mode means nothing in terms of added security. Toggle mode simply protects only kernel space. If you use the toggle mode (ibrs_enabled 1 default) only kernel is protected by IBRS and in turn you don't have to build the kernel with reptoline. If you leave IBRS always on it's effectively like having built everything (kernel and all userland) with reptoline. > 3) Does toggle mode of IBRS require RSB stuffing ? Yes, again IBRS does nothing to prevent the RSB stuffing. Only SMEP helps there if set in cr4 but only for kernel entry, vmexit still needs stuff_RSB. The stuff_RSB part I'm no certain but again it won't hurt to do it and we do it even with IBRS on. It's not measurable though. > 4) Exist CPUs which require IBRS to be selected automatically ? > >4b) If yes, provide the list as a separate answer please Not sure if I get the question so not answering this one. Future CPUs will have a new CPUID bit where enabling IBRS will perform better than toggling it at kernel entry, so in those CPUs the most secure mode (ibrs_enabled 2) that leaves IBRS always on, will perform even better than the current default (ibrs_enabled 1). So in presence of such future CPUID bit we should prefer ibrs_enabled 2 as default and leave IBRS always on as default. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 14:46 +0100, Thomas Gleixner wrote: > > So here is the simple list of questions all to be answered with YES or > NO. I don't want to see any of the 'but, though ...'. We all know by now > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > future CPUs. So get your act together and tell a clear YES or NO. This is actually covered by the documentation. Someone really should send you a copy. > 1) Does IBRS=1 when set once act as a set-and-forget option ? Never on current hardware. In future with IBRS_ATT, yes. > 1a) If the answer to #1 is yes, is it more secure than toggling it? No, just faster. > 1b) If the answer to #1 is yes, is retpoline required ? No. We'll ALTERNATIVE it away if we have IBRS_ATT. > 1c) If the answer to #1 is yes, is RSB stuffing required ? Not for IBRS_ATT, with weasel words about requiring SMEP. > 2) Does toggle mode of IBRS require retpoline ? No. Retpoline is an *alternative* to IBRS, for protecting the kernel. > 3) Does toggle mode of IBRS require RSB stuffing ? Yes for kernel entry if you have no SMEP. And yes on vmexit. > 4) Exist CPUs which require IBRS to be selected automatically ? > > 4b) If yes, provide the list as a separate answer please You mean CPUs on which retpoline isn't sufficient and thus the kernel should prefer IBRS "automatically" without a command line option? As discussed, yes on Skylake and anything after it that doesn't have IBRS_ATT, because there are tiny theoretical gaps that retpoline doesn't handle. But the option of sacrificing goats may be perfectly acceptable. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 14:46 +0100, Thomas Gleixner wrote: > > So here is the simple list of questions all to be answered with YES or > NO. I don't want to see any of the 'but, though ...'. We all know by now > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > future CPUs. So get your act together and tell a clear YES or NO. This is actually covered by the documentation. Someone really should send you a copy. > 1) Does IBRS=1 when set once act as a set-and-forget option ? Never on current hardware. In future with IBRS_ATT, yes. > 1a) If the answer to #1 is yes, is it more secure than toggling it? No, just faster. > 1b) If the answer to #1 is yes, is retpoline required ? No. We'll ALTERNATIVE it away if we have IBRS_ATT. > 1c) If the answer to #1 is yes, is RSB stuffing required ? Not for IBRS_ATT, with weasel words about requiring SMEP. > 2) Does toggle mode of IBRS require retpoline ? No. Retpoline is an *alternative* to IBRS, for protecting the kernel. > 3) Does toggle mode of IBRS require RSB stuffing ? Yes for kernel entry if you have no SMEP. And yes on vmexit. > 4) Exist CPUs which require IBRS to be selected automatically ? > > 4b) If yes, provide the list as a separate answer please You mean CPUs on which retpoline isn't sufficient and thus the kernel should prefer IBRS "automatically" without a command line option? As discussed, yes on Skylake and anything after it that doesn't have IBRS_ATT, because there are tiny theoretical gaps that retpoline doesn't handle. But the option of sacrificing goats may be perfectly acceptable. smime.p7s Description: S/MIME cryptographic signature
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> ibrs_enabled 2: > > sets IBRS always in host this is not secure > This matches the semantics described here by Tim patchset on lkml: > > https://marc.info/?l=linux-kernel=151520606320646 I will talk to Tim, it's not right. > I can tell in practice it works as I described in all microcodes > I tested. have you tested all 80 ?
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> ibrs_enabled 2: > > sets IBRS always in host this is not secure > This matches the semantics described here by Tim patchset on lkml: > > https://marc.info/?l=linux-kernel=151520606320646 I will talk to Tim, it's not right. > I can tell in practice it works as I described in all microcodes > I tested. have you tested all 80 ?
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, Van De Ven, Arjan wrote: > > So here is the simple list of questions all to be answered with YES or > > NO. I don't want to see any of the 'but, though ...'. We all know by now > > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > > future CPUs. So get your act together and tell a clear YES or NO. > > 2) Does toggle mode of IBRS require retpoline ? > > NO > > > > > 3) Does toggle mode of IBRS require RSB stuffing ? > > Only for the VM exit case Ok. > > > > 4) Exist CPUs which require IBRS to be selected automatically ? > > I do not understand your question exactly Whether the kernel should enable IBRS by default on certain models. And if yes, which ones. Thanks, tglx
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, Van De Ven, Arjan wrote: > > So here is the simple list of questions all to be answered with YES or > > NO. I don't want to see any of the 'but, though ...'. We all know by now > > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > > future CPUs. So get your act together and tell a clear YES or NO. > > 2) Does toggle mode of IBRS require retpoline ? > > NO > > > > > 3) Does toggle mode of IBRS require RSB stuffing ? > > Only for the VM exit case Ok. > > > > 4) Exist CPUs which require IBRS to be selected automatically ? > > I do not understand your question exactly Whether the kernel should enable IBRS by default on certain models. And if yes, which ones. Thanks, tglx
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:45:52PM +, Van De Ven, Arjan wrote: > > > Andrea, what you're saying is directly contradicting what I've heard > > from Intel. > > > > The documentation already distinguishes between IBRS on current > > hardware, and IBRS_ATT on future hardware. If it was the case that IBRS > > on current hardware is a set-and-forget option and completely disables > > branch prediction, then they would say that. Rather than explicitly > > saying the *opposite*, specifically for the case of current hardware, > > as they do. > > > > Rather than continuing to debate it, perhaps it's best just to wake for > > the US to wake up, and Intel to give a definitive answer. > > On current hardware, you cannot just set IBRS always. ibrs_enabled 1: sets IBRS at vmexit and at kernel entry. clears IBRS at kernel exit (return to usermode) restores whatever IBRS value the guest was using at vmenter ibrs_enabled 2: sets IBRS always in host sets IBRS if it wasn't already set by the guest in vmexit restores whatever IBRS valeu the guest was using at vmenter This matches the semantics described here by Tim patchset on lkml: https://marc.info/?l=linux-kernel=151520606320646 If you what you say is correct, then you should discuss with Tim what "echo 2 > /sys/kernel/debug/x86/ibrs_enabled will turn on IBRS in both userspace and kernel" is doing or is supposed to achieve. > (In practice, on some you might get lucky if you try. Intel does not > guarantee it. Intel does not test it. The model is to write the msr on > privilege change, e.g. ring transition) ibrs_enabled 1 is the default always with SPEC_CTRL in cpuid. The question is ibrs_enabled 2 optional mode but what is implemented matches the semantics of the above patchset in link from Tim so again you should talk with Tim and adjourn on the status of leaving IBRS always on with current silicon. I can tell in practice it works as I described in all microcodes I tested. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:45:52PM +, Van De Ven, Arjan wrote: > > > Andrea, what you're saying is directly contradicting what I've heard > > from Intel. > > > > The documentation already distinguishes between IBRS on current > > hardware, and IBRS_ATT on future hardware. If it was the case that IBRS > > on current hardware is a set-and-forget option and completely disables > > branch prediction, then they would say that. Rather than explicitly > > saying the *opposite*, specifically for the case of current hardware, > > as they do. > > > > Rather than continuing to debate it, perhaps it's best just to wake for > > the US to wake up, and Intel to give a definitive answer. > > On current hardware, you cannot just set IBRS always. ibrs_enabled 1: sets IBRS at vmexit and at kernel entry. clears IBRS at kernel exit (return to usermode) restores whatever IBRS value the guest was using at vmenter ibrs_enabled 2: sets IBRS always in host sets IBRS if it wasn't already set by the guest in vmexit restores whatever IBRS valeu the guest was using at vmenter This matches the semantics described here by Tim patchset on lkml: https://marc.info/?l=linux-kernel=151520606320646 If you what you say is correct, then you should discuss with Tim what "echo 2 > /sys/kernel/debug/x86/ibrs_enabled will turn on IBRS in both userspace and kernel" is doing or is supposed to achieve. > (In practice, on some you might get lucky if you try. Intel does not > guarantee it. Intel does not test it. The model is to write the msr on > privilege change, e.g. ring transition) ibrs_enabled 1 is the default always with SPEC_CTRL in cpuid. The question is ibrs_enabled 2 optional mode but what is implemented matches the semantics of the above patchset in link from Tim so again you should talk with Tim and adjourn on the status of leaving IBRS always on with current silicon. I can tell in practice it works as I described in all microcodes I tested. Thanks, Andrea
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> So here is the simple list of questions all to be answered with YES or > NO. I don't want to see any of the 'but, though ...'. We all know by now > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > future CPUs. So get your act together and tell a clear YES or NO. I will answer for current cpus, so those not having IBRS_ATT > > 1) Does IBRS=1 when set once act as a set-and-forget option ? NO > 1a) If the answer to #1 is yes, is it more secure than toggling it? NO > > 1b) If the answer to #1 is yes, is retpoline required ? NO - Once you have IBRS done PROPERLY (toggled) you don't need retpoline > > 1c) If the answer to #1 is yes, is RSB stuffing required ? Only for the VM exit case > > 2) Does toggle mode of IBRS require retpoline ? NO > > 3) Does toggle mode of IBRS require RSB stuffing ? Only for the VM exit case > > 4) Exist CPUs which require IBRS to be selected automatically ? I do not understand your question exactly
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> So here is the simple list of questions all to be answered with YES or > NO. I don't want to see any of the 'but, though ...'. We all know by now > that it's CPU dependent and slow and whatever and that IBRS_ATT will be in > future CPUs. So get your act together and tell a clear YES or NO. I will answer for current cpus, so those not having IBRS_ATT > > 1) Does IBRS=1 when set once act as a set-and-forget option ? NO > 1a) If the answer to #1 is yes, is it more secure than toggling it? NO > > 1b) If the answer to #1 is yes, is retpoline required ? NO - Once you have IBRS done PROPERLY (toggled) you don't need retpoline > > 1c) If the answer to #1 is yes, is RSB stuffing required ? Only for the VM exit case > > 2) Does toggle mode of IBRS require retpoline ? NO > > 3) Does toggle mode of IBRS require RSB stuffing ? Only for the VM exit case > > 4) Exist CPUs which require IBRS to be selected automatically ? I do not understand your question exactly
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, David Woodhouse wrote: > Andrea, what you're saying is directly contradicting what I've heard > from Intel. > > The documentation already distinguishes between IBRS on current > hardware, and IBRS_ATT on future hardware. If it was the case that IBRS > on current hardware is a set-and-forget option and completely disables > branch prediction, then they would say that. Rather than explicitly > saying the *opposite*, specifically for the case of current hardware, > as they do. > > Rather than continuing to debate it, perhaps it's best just to wake for > the US to wake up, and Intel to give a definitive answer. So here is the simple list of questions all to be answered with YES or NO. I don't want to see any of the 'but, though ...'. We all know by now that it's CPU dependent and slow and whatever and that IBRS_ATT will be in future CPUs. So get your act together and tell a clear YES or NO. 1) Does IBRS=1 when set once act as a set-and-forget option ? 1a) If the answer to #1 is yes, is it more secure than toggling it? 1b) If the answer to #1 is yes, is retpoline required ? 1c) If the answer to #1 is yes, is RSB stuffing required ? 2) Does toggle mode of IBRS require retpoline ? 3) Does toggle mode of IBRS require RSB stuffing ? 4) Exist CPUs which require IBRS to be selected automatically ? 4b) If yes, provide the list as a separate answer please Thanks, tglx
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, David Woodhouse wrote: > Andrea, what you're saying is directly contradicting what I've heard > from Intel. > > The documentation already distinguishes between IBRS on current > hardware, and IBRS_ATT on future hardware. If it was the case that IBRS > on current hardware is a set-and-forget option and completely disables > branch prediction, then they would say that. Rather than explicitly > saying the *opposite*, specifically for the case of current hardware, > as they do. > > Rather than continuing to debate it, perhaps it's best just to wake for > the US to wake up, and Intel to give a definitive answer. So here is the simple list of questions all to be answered with YES or NO. I don't want to see any of the 'but, though ...'. We all know by now that it's CPU dependent and slow and whatever and that IBRS_ATT will be in future CPUs. So get your act together and tell a clear YES or NO. 1) Does IBRS=1 when set once act as a set-and-forget option ? 1a) If the answer to #1 is yes, is it more secure than toggling it? 1b) If the answer to #1 is yes, is retpoline required ? 1c) If the answer to #1 is yes, is RSB stuffing required ? 2) Does toggle mode of IBRS require retpoline ? 3) Does toggle mode of IBRS require RSB stuffing ? 4) Exist CPUs which require IBRS to be selected automatically ? 4b) If yes, provide the list as a separate answer please Thanks, tglx
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> Andrea, what you're saying is directly contradicting what I've heard > from Intel. > > The documentation already distinguishes between IBRS on current > hardware, and IBRS_ATT on future hardware. If it was the case that IBRS > on current hardware is a set-and-forget option and completely disables > branch prediction, then they would say that. Rather than explicitly > saying the *opposite*, specifically for the case of current hardware, > as they do. > > Rather than continuing to debate it, perhaps it's best just to wake for > the US to wake up, and Intel to give a definitive answer. On current hardware, you cannot just set IBRS always. (In practice, on some you might get lucky if you try. Intel does not guarantee it. Intel does not test it. The model is to write the msr on privilege change, e.g. ring transition)
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> Andrea, what you're saying is directly contradicting what I've heard > from Intel. > > The documentation already distinguishes between IBRS on current > hardware, and IBRS_ATT on future hardware. If it was the case that IBRS > on current hardware is a set-and-forget option and completely disables > branch prediction, then they would say that. Rather than explicitly > saying the *opposite*, specifically for the case of current hardware, > as they do. > > Rather than continuing to debate it, perhaps it's best just to wake for > the US to wake up, and Intel to give a definitive answer. On current hardware, you cannot just set IBRS always. (In practice, on some you might get lucky if you try. Intel does not guarantee it. Intel does not test it. The model is to write the msr on privilege change, e.g. ring transition)
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> On Wed, Jan 10, 2018 at 12:12:53PM +, David Woodhouse wrote: > > IBRS is like a barrier. You must write it between the 'problematic' > > loading of the branch targets, and the kernel code which might be > > affected. > > > > You cannot, on current hardware, merely set it once and forget about > > it. That is not sufficient. > > I think you've got it all wrong... Andrea: David is right. The specification draft that you have also makes this clear. You can't just set IBRS once and call it good; you do need to write it on entering the kernel.
RE: [patch RFC 5/5] x86/speculation: Add basic speculation control code
> On Wed, Jan 10, 2018 at 12:12:53PM +, David Woodhouse wrote: > > IBRS is like a barrier. You must write it between the 'problematic' > > loading of the branch targets, and the kernel code which might be > > affected. > > > > You cannot, on current hardware, merely set it once and forget about > > it. That is not sufficient. > > I think you've got it all wrong... Andrea: David is right. The specification draft that you have also makes this clear. You can't just set IBRS once and call it good; you do need to write it on entering the kernel.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 02:10:10PM +0100, Andrea Arcangeli wrote: > It's still incredibly faster to shutdown part of the CPU temporarily > than to flush its internal state as a whole with IBPB. If it wouldn't > be the case ibrs_enabled 0 ibpb_enabled 2 special mode would perform > better (but that's only enabled by default if SPEC_CTRL is not > available and only IBPB_SUPPORT is). Yet another bit of perhaps useful info: if you run 100% kernel intensive computation (i.e. only interrupts on idle task or a kernel driver doing all in kernel) ibrs_enabled 0 ibpb_enabled 2 of course is much faster than ibrs_enabled 1 ibpb_enabled 1. As long as you don't have frequent ring change, ibrs_enabled 0 ibpb_enabled 2 is faster.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 02:10:10PM +0100, Andrea Arcangeli wrote: > It's still incredibly faster to shutdown part of the CPU temporarily > than to flush its internal state as a whole with IBPB. If it wouldn't > be the case ibrs_enabled 0 ibpb_enabled 2 special mode would perform > better (but that's only enabled by default if SPEC_CTRL is not > available and only IBPB_SUPPORT is). Yet another bit of perhaps useful info: if you run 100% kernel intensive computation (i.e. only interrupts on idle task or a kernel driver doing all in kernel) ibrs_enabled 0 ibpb_enabled 2 of course is much faster than ibrs_enabled 1 ibpb_enabled 1. As long as you don't have frequent ring change, ibrs_enabled 0 ibpb_enabled 2 is faster.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > Which in current silicon IBP speculation is turned off always, So this is clearly the source of confusion. If that's the case, what you are saying makes sense. Where is this information coming from? It definitely is not in the Intel documentation I've seen. Thanks, -- Jiri Kosina SUSE Labs
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > Which in current silicon IBP speculation is turned off always, So this is clearly the source of confusion. If that's the case, what you are saying makes sense. Where is this information coming from? It definitely is not in the Intel documentation I've seen. Thanks, -- Jiri Kosina SUSE Labs
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 02:05:51PM +0100, Andrea Arcangeli wrote: > Also note, the slowdown of setting IBRS varies with older CPUs being To give a further detail, older CPUs to provide IBRS semantics have to do something even less finegrined that doesn't just restricts speculation across IBRS less privileged prediction level, that doesn't just restrict speculation across indirect branches, that doesn't just restrict indirect branch prediction, but that shuts down a whole block of the CPU to achieve those super finegrined theoretical IBRS semantics that matches future silicon. IBRS is doing a lot more than what it would be needed because there was no other way to achieve it on some microcode implementations. Which is why ibrs_enabled 1 is needed to achieve better performance as you don't want to run slower while in user mode, much better to pay the cost of setting IBRS and shutting down that part of the CPU. It's still incredibly faster to shutdown part of the CPU temporarily than to flush its internal state as a whole with IBPB. If it wouldn't be the case ibrs_enabled 0 ibpb_enabled 2 special mode would perform better (but that's only enabled by default if SPEC_CTRL is not available and only IBPB_SUPPORT is). Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 02:05:51PM +0100, Andrea Arcangeli wrote: > Also note, the slowdown of setting IBRS varies with older CPUs being To give a further detail, older CPUs to provide IBRS semantics have to do something even less finegrined that doesn't just restricts speculation across IBRS less privileged prediction level, that doesn't just restrict speculation across indirect branches, that doesn't just restrict indirect branch prediction, but that shuts down a whole block of the CPU to achieve those super finegrined theoretical IBRS semantics that matches future silicon. IBRS is doing a lot more than what it would be needed because there was no other way to achieve it on some microcode implementations. Which is why ibrs_enabled 1 is needed to achieve better performance as you don't want to run slower while in user mode, much better to pay the cost of setting IBRS and shutting down that part of the CPU. It's still incredibly faster to shutdown part of the CPU temporarily than to flush its internal state as a whole with IBPB. If it wouldn't be the case ibrs_enabled 0 ibpb_enabled 2 special mode would perform better (but that's only enabled by default if SPEC_CTRL is not available and only IBPB_SUPPORT is). Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:57 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 01:47:22PM +0100, Jiri Kosina wrote: > > > > On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > > > > > > > > Perhaps the confusing come from "less privileged prediction mode" and > > > you thought that meant "less privileged ring mode". It says "predction > > > mode" not ring 3. > > Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX > > non-root", with obvious ordering of privileges. > > > > So if IBRS is set, branch predictor will not allow the predicted target to > > be influenced by code that executed in less privileged prediction mode > > before value of '1' IBRS mode was last written to, and that's pretty much > > it. > Which in current silicon IBP speculation is turned off always, and the > above specification really is to provide more finegrined semantics > for future silicon where it'll perform best to leave it always on and > it'll be still as secure as it is now despite the IBP speculation may > not always be turned off like it happens right now. > > With all the prediction modes ordered right for the respective > guest/ring and CPUID will tell us when it's higher perf to enable > ibrs_enabled 2 ibpb_enabled 1 by default. > > Again I see zero issues with leaving IBRS always on in current and > future silicon and I see absolutely zero problems in setting IBRS in > vmexit to prevent the whole guest mode to attack the kernel memory, > and in fact ibrs_enabled 2 will even more secure and it'll prevent the > gust mode userland even to attack the host qemu userland through > spectre variant#2. > > As long as the "with obvious ordering of privileges" is maintained > when IBRS is not a total turn off of IBP speculation, everything works > as intended. Andrea, what you're saying is directly contradicting what I've heard from Intel. The documentation already distinguishes between IBRS on current hardware, and IBRS_ATT on future hardware. If it was the case that IBRS on current hardware is a set-and-forget option and completely disables branch prediction, then they would say that. Rather than explicitly saying the *opposite*, specifically for the case of current hardware, as they do. Rather than continuing to debate it, perhaps it's best just to wake for the US to wake up, and Intel to give a definitive answer. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:57 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 01:47:22PM +0100, Jiri Kosina wrote: > > > > On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > > > > > > > > Perhaps the confusing come from "less privileged prediction mode" and > > > you thought that meant "less privileged ring mode". It says "predction > > > mode" not ring 3. > > Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX > > non-root", with obvious ordering of privileges. > > > > So if IBRS is set, branch predictor will not allow the predicted target to > > be influenced by code that executed in less privileged prediction mode > > before value of '1' IBRS mode was last written to, and that's pretty much > > it. > Which in current silicon IBP speculation is turned off always, and the > above specification really is to provide more finegrined semantics > for future silicon where it'll perform best to leave it always on and > it'll be still as secure as it is now despite the IBP speculation may > not always be turned off like it happens right now. > > With all the prediction modes ordered right for the respective > guest/ring and CPUID will tell us when it's higher perf to enable > ibrs_enabled 2 ibpb_enabled 1 by default. > > Again I see zero issues with leaving IBRS always on in current and > future silicon and I see absolutely zero problems in setting IBRS in > vmexit to prevent the whole guest mode to attack the kernel memory, > and in fact ibrs_enabled 2 will even more secure and it'll prevent the > gust mode userland even to attack the host qemu userland through > spectre variant#2. > > As long as the "with obvious ordering of privileges" is maintained > when IBRS is not a total turn off of IBP speculation, everything works > as intended. Andrea, what you're saying is directly contradicting what I've heard from Intel. The documentation already distinguishes between IBRS on current hardware, and IBRS_ATT on future hardware. If it was the case that IBRS on current hardware is a set-and-forget option and completely disables branch prediction, then they would say that. Rather than explicitly saying the *opposite*, specifically for the case of current hardware, as they do. Rather than continuing to debate it, perhaps it's best just to wake for the US to wake up, and Intel to give a definitive answer. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 02:02:02PM +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 12:51:13PM +, David Woodhouse wrote: > > If it worked as Andrea suggests, then there would be absolutely no > > point in the patches we've seen which add the IBRS-frobbing on syscall > > entry and vmexit. > > This is perhaps what you're missing I think, there is a huge point: > performance. And note, if you run getppid in a loop you're perfectly right ibrs_enabled 2 will perform better than ibrs_enabled 1, but we run multiple set of macro benchmarks and ibrs_enabled 1 ibpb_enabled 1 always wins. So ibrs_enabled 2 ibpb_enabled 1 is left as an optional "paranoid" security mode, more secure but even slower. We draw a line for the default to stop the PoC which we know can be mounted (at least in theory :) and is practical attack. Also note, the slowdown of setting IBRS varies with older CPUs being more affected, but for all current silicon ibrs_enabled 1 ibpb_enabled 1 is faster in 99% of real life workloads. The future silicon will set the CPUID that will tell us set "ibrs_enabled 2" by default, but you can do it right and it's even more secure as it completely solves any HT effect in userland and it prevents the KVM guest kernel to be attacked by guest userland through a spectre variant#2 attack from guest userland to qemu host userland. Hope this helps, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 02:02:02PM +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 12:51:13PM +, David Woodhouse wrote: > > If it worked as Andrea suggests, then there would be absolutely no > > point in the patches we've seen which add the IBRS-frobbing on syscall > > entry and vmexit. > > This is perhaps what you're missing I think, there is a huge point: > performance. And note, if you run getppid in a loop you're perfectly right ibrs_enabled 2 will perform better than ibrs_enabled 1, but we run multiple set of macro benchmarks and ibrs_enabled 1 ibpb_enabled 1 always wins. So ibrs_enabled 2 ibpb_enabled 1 is left as an optional "paranoid" security mode, more secure but even slower. We draw a line for the default to stop the PoC which we know can be mounted (at least in theory :) and is practical attack. Also note, the slowdown of setting IBRS varies with older CPUs being more affected, but for all current silicon ibrs_enabled 1 ibpb_enabled 1 is faster in 99% of real life workloads. The future silicon will set the CPUID that will tell us set "ibrs_enabled 2" by default, but you can do it right and it's even more secure as it completely solves any HT effect in userland and it prevents the KVM guest kernel to be attacked by guest userland through a spectre variant#2 attack from guest userland to qemu host userland. Hope this helps, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:51:13PM +, David Woodhouse wrote: > If it worked as Andrea suggests, then there would be absolutely no > point in the patches we've seen which add the IBRS-frobbing on syscall > entry and vmexit. This is perhaps what you're missing I think, there is a huge point: performance. ibrs_enabled 1 ibpb_enabled 1 is much faster than ibrs_enabled 2 ibpb_enabled 1. The current IBRS is disabling IBP, it can't do the "finegrined" thing that leaves IBP speculation enabled. Kernel is not a 100% C++ virtual template, so for kernel IBRS enabled is not as a big deal as userland with IBRS enabled. Once the new CPUid bit will be set it'll tell us IBRS is really restricting speculation in a finegrined way with all privilege levels ordered, so it simply tells is ibrs_enabled 2 ibpb_enabled 1 will be faster than ibrs_enabled 1 ibpb_enabled 1, so the only thing we've to do with the new CPUID bit you're talking about from the future, is to change the default to ibrs_enabled 2 and it'll perform even better with zero overhead in kernel entry points. We already optimized for the future silicon, we've only to add a tweak to detect cpuid and set the default ibrs_enabled to 2. > The "IBRS all the time" feature is something we get on *future* > hardware, not current hardware. No, for current hardware it's the most secure option available and required in fact if you want to fix guest userland attack on host userland with math certainty which can matter for some customer.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:51:13PM +, David Woodhouse wrote: > If it worked as Andrea suggests, then there would be absolutely no > point in the patches we've seen which add the IBRS-frobbing on syscall > entry and vmexit. This is perhaps what you're missing I think, there is a huge point: performance. ibrs_enabled 1 ibpb_enabled 1 is much faster than ibrs_enabled 2 ibpb_enabled 1. The current IBRS is disabling IBP, it can't do the "finegrined" thing that leaves IBP speculation enabled. Kernel is not a 100% C++ virtual template, so for kernel IBRS enabled is not as a big deal as userland with IBRS enabled. Once the new CPUid bit will be set it'll tell us IBRS is really restricting speculation in a finegrined way with all privilege levels ordered, so it simply tells is ibrs_enabled 2 ibpb_enabled 1 will be faster than ibrs_enabled 1 ibpb_enabled 1, so the only thing we've to do with the new CPUID bit you're talking about from the future, is to change the default to ibrs_enabled 2 and it'll perform even better with zero overhead in kernel entry points. We already optimized for the future silicon, we've only to add a tweak to detect cpuid and set the default ibrs_enabled to 2. > The "IBRS all the time" feature is something we get on *future* > hardware, not current hardware. No, for current hardware it's the most secure option available and required in fact if you want to fix guest userland attack on host userland with math certainty which can matter for some customer.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:47:22PM +0100, Jiri Kosina wrote: > On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > > > Perhaps the confusing come from "less privileged prediction mode" and > > you thought that meant "less privileged ring mode". It says "predction > > mode" not ring 3. > > Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX > non-root", with obvious ordering of privileges. > > So if IBRS is set, branch predictor will not allow the predicted target to > be influenced by code that executed in less privileged prediction mode > before value of '1' IBRS mode was last written to, and that's pretty much > it. Which in current silicon IBP speculation is turned off always, and the above specification really is to provide more finegrined semantics for future silicon where it'll perform best to leave it always on and it'll be still as secure as it is now despite the IBP speculation may not always be turned off like it happens right now. With all the prediction modes ordered right for the respective guest/ring and CPUID will tell us when it's higher perf to enable ibrs_enabled 2 ibpb_enabled 1 by default. Again I see zero issues with leaving IBRS always on in current and future silicon and I see absolutely zero problems in setting IBRS in vmexit to prevent the whole guest mode to attack the kernel memory, and in fact ibrs_enabled 2 will even more secure and it'll prevent the gust mode userland even to attack the host qemu userland through spectre variant#2. As long as the "with obvious ordering of privileges" is maintained when IBRS is not a total turn off of IBP speculation, everything works as intended.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:47:22PM +0100, Jiri Kosina wrote: > On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > > > Perhaps the confusing come from "less privileged prediction mode" and > > you thought that meant "less privileged ring mode". It says "predction > > mode" not ring 3. > > Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX > non-root", with obvious ordering of privileges. > > So if IBRS is set, branch predictor will not allow the predicted target to > be influenced by code that executed in less privileged prediction mode > before value of '1' IBRS mode was last written to, and that's pretty much > it. Which in current silicon IBP speculation is turned off always, and the above specification really is to provide more finegrined semantics for future silicon where it'll perform best to leave it always on and it'll be still as secure as it is now despite the IBP speculation may not always be turned off like it happens right now. With all the prediction modes ordered right for the respective guest/ring and CPUID will tell us when it's higher perf to enable ibrs_enabled 2 ibpb_enabled 1 by default. Again I see zero issues with leaving IBRS always on in current and future silicon and I see absolutely zero problems in setting IBRS in vmexit to prevent the whole guest mode to attack the kernel memory, and in fact ibrs_enabled 2 will even more secure and it'll prevent the gust mode userland even to attack the host qemu userland through spectre variant#2. As long as the "with obvious ordering of privileges" is maintained when IBRS is not a total turn off of IBP speculation, everything works as intended.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:47 +0100, Jiri Kosina wrote: > On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > > > Perhaps the confusing come from "less privileged prediction mode" and > > you thought that meant "less privileged ring mode". It says "predction > > mode" not ring 3. > > Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX > non-root", with obvious ordering of privileges. > > So if IBRS is set, branch predictor will not allow the predicted target to > be influenced by code that executed in less privileged prediction mode > before value of '1' IBRS mode was last written to, and that's pretty much > it. The operative words in that sentence being, "before the IBRS mode was last written with a value of 1". If it worked as Andrea suggests, then there would be absolutely no point in the patches we've seen which add the IBRS-frobbing on syscall entry and vmexit. The "IBRS all the time" feature is something we get on *future* hardware, not current hardware. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:47 +0100, Jiri Kosina wrote: > On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > > > Perhaps the confusing come from "less privileged prediction mode" and > > you thought that meant "less privileged ring mode". It says "predction > > mode" not ring 3. > > Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX > non-root", with obvious ordering of privileges. > > So if IBRS is set, branch predictor will not allow the predicted target to > be influenced by code that executed in less privileged prediction mode > before value of '1' IBRS mode was last written to, and that's pretty much > it. The operative words in that sentence being, "before the IBRS mode was last written with a value of 1". If it worked as Andrea suggests, then there would be absolutely no point in the patches we've seen which add the IBRS-frobbing on syscall entry and vmexit. The "IBRS all the time" feature is something we get on *future* hardware, not current hardware. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > Perhaps the confusing come from "less privileged prediction mode" and > you thought that meant "less privileged ring mode". It says "predction > mode" not ring 3. Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX non-root", with obvious ordering of privileges. So if IBRS is set, branch predictor will not allow the predicted target to be influenced by code that executed in less privileged prediction mode before value of '1' IBRS mode was last written to, and that's pretty much it. -- Jiri Kosina SUSE Labs
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, Andrea Arcangeli wrote: > Perhaps the confusing come from "less privileged prediction mode" and > you thought that meant "less privileged ring mode". It says "predction > mode" not ring 3. Well, prediction mode is defined by "CPL3 vs CPL0-2" and "VMX root vs VMX non-root", with obvious ordering of privileges. So if IBRS is set, branch predictor will not allow the predicted target to be influenced by code that executed in less privileged prediction mode before value of '1' IBRS mode was last written to, and that's pretty much it. -- Jiri Kosina SUSE Labs
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:29:44PM +, David Woodhouse wrote: > On Wed, 2018-01-10 at 13:17 +0100, Andrea Arcangeli wrote: > > On Wed, Jan 10, 2018 at 12:09:34PM +, David Woodhouse wrote: > > > That is not consistent with the documentation I've seen, which Intel > > > have so far utterly failed to publish AFAICT. > > > > > > "a near indirect jump/call/return may be affected by code in a less > > > privileged > > > prediction mode that executed AFTER IBRS mode was last written with a > > > value of 1" > > > > You must have misunderstood the context there, or the above text is > > wrong to begin with. > > That's a quote from the Intel documentation for the IBRS feature. > Go read it, please. Perhaps the confusing come from "less privileged prediction mode" and you thought that meant "less privileged ring mode". It says "predction mode" not ring 3. Frankly I found that documentation very confusing like the inversion of IBRS enabled really meaning IBP speculation disabled like Ingo pointed out. It's clear all done in a rush but we've to live with it. After all the electric current also really flows in the opposite direction of the arrow.. > Andrea, what part of this whole fucking mess isn't entirely batshit > insane to start with? :) Absolutely :). > I think you are confused with the future IBRS_ATT option which will > exist on new hardware. No, the only difference is that such option will perform best. This is why the current default ibrs_enabled 1 ibpb_enabled 1. That future CPUID bit will simply make the kernel boot by default with ibrs_enabled 2 ibpb_enabled 1 and it'll perform best as it won't have to write to SPEC_CTRL in kernel entry kernel exit vmenter/vmexit like it commonly has to do with ibrs_enabled 1. The only difference is that ibrs_enabled 2 will perform best, while currently ibrs_enabled 1 performs best. > Right now, IBRS works as I described it because that's the best they > could do with microcode. It works as I described but instead of arguing about specs above, Intel should clarify that IBRS can already be set 100% of the time, be left alone and set always, with all CPUs with SPEC_CTRL, and it's the most secure mode available and matches the IBRS patchset with ibrs_enabled 2. Hope this helps clarify and of course this is so complex it's perfectly normal to misunderstand something, but I'm confident it's not me who misunderstood because if I did, the whole ibrs_enabled 2 that was just posted would make zero sense, that is for current CPUs and it's the most secure option available (not less secure as you seem to think). Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:29:44PM +, David Woodhouse wrote: > On Wed, 2018-01-10 at 13:17 +0100, Andrea Arcangeli wrote: > > On Wed, Jan 10, 2018 at 12:09:34PM +, David Woodhouse wrote: > > > That is not consistent with the documentation I've seen, which Intel > > > have so far utterly failed to publish AFAICT. > > > > > > "a near indirect jump/call/return may be affected by code in a less > > > privileged > > > prediction mode that executed AFTER IBRS mode was last written with a > > > value of 1" > > > > You must have misunderstood the context there, or the above text is > > wrong to begin with. > > That's a quote from the Intel documentation for the IBRS feature. > Go read it, please. Perhaps the confusing come from "less privileged prediction mode" and you thought that meant "less privileged ring mode". It says "predction mode" not ring 3. Frankly I found that documentation very confusing like the inversion of IBRS enabled really meaning IBP speculation disabled like Ingo pointed out. It's clear all done in a rush but we've to live with it. After all the electric current also really flows in the opposite direction of the arrow.. > Andrea, what part of this whole fucking mess isn't entirely batshit > insane to start with? :) Absolutely :). > I think you are confused with the future IBRS_ATT option which will > exist on new hardware. No, the only difference is that such option will perform best. This is why the current default ibrs_enabled 1 ibpb_enabled 1. That future CPUID bit will simply make the kernel boot by default with ibrs_enabled 2 ibpb_enabled 1 and it'll perform best as it won't have to write to SPEC_CTRL in kernel entry kernel exit vmenter/vmexit like it commonly has to do with ibrs_enabled 1. The only difference is that ibrs_enabled 2 will perform best, while currently ibrs_enabled 1 performs best. > Right now, IBRS works as I described it because that's the best they > could do with microcode. It works as I described but instead of arguing about specs above, Intel should clarify that IBRS can already be set 100% of the time, be left alone and set always, with all CPUs with SPEC_CTRL, and it's the most secure mode available and matches the IBRS patchset with ibrs_enabled 2. Hope this helps clarify and of course this is so complex it's perfectly normal to misunderstand something, but I'm confident it's not me who misunderstood because if I did, the whole ibrs_enabled 2 that was just posted would make zero sense, that is for current CPUs and it's the most secure option available (not less secure as you seem to think). Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:17 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 12:09:34PM +, David Woodhouse wrote: > > That is not consistent with the documentation I've seen, which Intel > > have so far utterly failed to publish AFAICT. > > > > "a near indirect jump/call/return may be affected by code in a less > > privileged > > prediction mode that executed AFTER IBRS mode was last written with a value > > of 1" > > You must have misunderstood the context there, or the above text is > wrong to begin with. That's a quote from the Intel documentation for the IBRS feature. Go read it, please. > > The kernel is only protected from branch targets set in userspace > > *BEFORE* the IBRS mode was last set to 1. If you set it to 1, then > > leave it like that while you run userspace and then kernel code again, > > you are not protected. > > I'm sure you've got it wrong, that would be crazy if it would be the > case. Andrea, what part of this whole fucking mess isn't entirely batshit insane to start with? :) I think you are confused with the future IBRS_ATT option which will exist on new hardware. Right now, IBRS works as I described it because that's the best they could do with microcode. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:17 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 12:09:34PM +, David Woodhouse wrote: > > That is not consistent with the documentation I've seen, which Intel > > have so far utterly failed to publish AFAICT. > > > > "a near indirect jump/call/return may be affected by code in a less > > privileged > > prediction mode that executed AFTER IBRS mode was last written with a value > > of 1" > > You must have misunderstood the context there, or the above text is > wrong to begin with. That's a quote from the Intel documentation for the IBRS feature. Go read it, please. > > The kernel is only protected from branch targets set in userspace > > *BEFORE* the IBRS mode was last set to 1. If you set it to 1, then > > leave it like that while you run userspace and then kernel code again, > > you are not protected. > > I'm sure you've got it wrong, that would be crazy if it would be the > case. Andrea, what part of this whole fucking mess isn't entirely batshit insane to start with? :) I think you are confused with the future IBRS_ATT option which will exist on new hardware. Right now, IBRS works as I described it because that's the best they could do with microcode. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:20:45PM +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 12:12:53PM +, David Woodhouse wrote: > > IBRS is like a barrier. You must write it between the 'problematic' > > loading of the branch targets, and the kernel code which might be > > affected. > > > > You cannot, on current hardware, merely set it once and forget about > > it. That is not sufficient. > > I think you've got it all wrong... On a side note, I hope once you get it right, the debugfs tunable will suddenly look less horrid.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:20:45PM +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 12:12:53PM +, David Woodhouse wrote: > > IBRS is like a barrier. You must write it between the 'problematic' > > loading of the branch targets, and the kernel code which might be > > affected. > > > > You cannot, on current hardware, merely set it once and forget about > > it. That is not sufficient. > > I think you've got it all wrong... On a side note, I hope once you get it right, the debugfs tunable will suddenly look less horrid.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:12:53PM +, David Woodhouse wrote: > IBRS is like a barrier. You must write it between the 'problematic' > loading of the branch targets, and the kernel code which might be > affected. > > You cannot, on current hardware, merely set it once and forget about > it. That is not sufficient. I think you've got it all wrong...
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:12:53PM +, David Woodhouse wrote: > IBRS is like a barrier. You must write it between the 'problematic' > loading of the branch targets, and the kernel code which might be > affected. > > You cannot, on current hardware, merely set it once and forget about > it. That is not sufficient. I think you've got it all wrong...
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:09:34PM +, David Woodhouse wrote: > That is not consistent with the documentation I've seen, which Intel > have so far utterly failed to publish AFAICT. > > "a near indirect jump/call/return may be affected by code in a less privileged > prediction mode that executed AFTER IBRS mode was last written with a value > of 1" You must have misunderstood the context there, or the above text is wrong to begin with. > The kernel is only protected from branch targets set in userspace > *BEFORE* the IBRS mode was last set to 1. If you set it to 1, then > leave it like that while you run userspace and then kernel code again, > you are not protected. I'm sure you've got it wrong, that would be crazy if it would be the case. Even from an implementation standpoint IBRS just means stop speculating through branch prediction, there's no way they can add this separation between ring 0 and ring 3 when it didn't exist to begin with. RSB had it with SMEP and in fact there's no need to do stuff_RSB with SMEP enabled in cr4 and we alternative it out in kernel entry on host and guest, but we still have to do that in vmexit. IBP/BTB had nothing like, which is why user ring 3 can attack host ring 0. It will have it and also separating guest from host but in the future. Plus IBRS can be set at will by guest mode, if guest kernel is malicious it will let guest userland run with IBRS on. You can't trust the guest kernel to begin with. So it would make IBRS totally useless for KVM to set in vmexit if you were right about that. In any case, we've a ibrs_enabled 0 ibpb_enabled 2 that would achieve full security if you were right, but I think you got it all wrong about IBRS and rings. Second: IBRS means Indirect Branch Restricted Speculation, the speculation through branch prediction is restricted while it's set. It's inconceivable that they add ring knowledge to a part that is couldn't be helped by SMEP in the first place that had at least some ring separation when enabled.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 12:09:34PM +, David Woodhouse wrote: > That is not consistent with the documentation I've seen, which Intel > have so far utterly failed to publish AFAICT. > > "a near indirect jump/call/return may be affected by code in a less privileged > prediction mode that executed AFTER IBRS mode was last written with a value > of 1" You must have misunderstood the context there, or the above text is wrong to begin with. > The kernel is only protected from branch targets set in userspace > *BEFORE* the IBRS mode was last set to 1. If you set it to 1, then > leave it like that while you run userspace and then kernel code again, > you are not protected. I'm sure you've got it wrong, that would be crazy if it would be the case. Even from an implementation standpoint IBRS just means stop speculating through branch prediction, there's no way they can add this separation between ring 0 and ring 3 when it didn't exist to begin with. RSB had it with SMEP and in fact there's no need to do stuff_RSB with SMEP enabled in cr4 and we alternative it out in kernel entry on host and guest, but we still have to do that in vmexit. IBP/BTB had nothing like, which is why user ring 3 can attack host ring 0. It will have it and also separating guest from host but in the future. Plus IBRS can be set at will by guest mode, if guest kernel is malicious it will let guest userland run with IBRS on. You can't trust the guest kernel to begin with. So it would make IBRS totally useless for KVM to set in vmexit if you were right about that. In any case, we've a ibrs_enabled 0 ibpb_enabled 2 that would achieve full security if you were right, but I think you got it all wrong about IBRS and rings. Second: IBRS means Indirect Branch Restricted Speculation, the speculation through branch prediction is restricted while it's set. It's inconceivable that they add ring knowledge to a part that is couldn't be helped by SMEP in the first place that had at least some ring separation when enabled.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:07 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 01:01:58PM +0100, Andrea Arcangeli wrote: > > On Wed, Jan 10, 2018 at 11:58:54AM +, David Woodhouse wrote: > > > On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > > > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > I don't know why you're calling that 'IBRS=2'; are you getting > > > > confused > > > > > by Andrea's distro horridness? > > > > > > > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > > > > set in SPEC_CTLR 100% of the time, except in guest mode. > > > > > > On all current hardware, if you only set IBRS when you exit a guest, > > > then you are not protecting yourself from userspace at all. IBRS acts > > > as a *barrier* in all current hardware. > > > > Kernel memory is 100% protected if you set only IBRS at vmexit. > > > > Once IBRS is set, there's no way any userland (nor host nor guest) can > > attack the kernel memory through spectre variant#2. > > > > What is not protected is host userland from guest userland which is > > point 3 in the email I posted earlier and I already provided all > > details there on how to fix that purely theoretical issue not part of > > the PoC with the provided debugfs tunables, so I won't repeat here. > > Also I read in another email you thought IBRS is a barrier or > something, it's not, it's purely temporarily preventing the CPU to > speculate through IBP BTB whatever, No. IBRS is like a barrier. You must write it between the 'problematic' loading of the branch targets, and the kernel code which might be affected. You cannot, on current hardware, merely set it once and forget about it. That is not sufficient. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:07 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 01:01:58PM +0100, Andrea Arcangeli wrote: > > On Wed, Jan 10, 2018 at 11:58:54AM +, David Woodhouse wrote: > > > On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > > > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > I don't know why you're calling that 'IBRS=2'; are you getting > > > > confused > > > > > by Andrea's distro horridness? > > > > > > > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > > > > set in SPEC_CTLR 100% of the time, except in guest mode. > > > > > > On all current hardware, if you only set IBRS when you exit a guest, > > > then you are not protecting yourself from userspace at all. IBRS acts > > > as a *barrier* in all current hardware. > > > > Kernel memory is 100% protected if you set only IBRS at vmexit. > > > > Once IBRS is set, there's no way any userland (nor host nor guest) can > > attack the kernel memory through spectre variant#2. > > > > What is not protected is host userland from guest userland which is > > point 3 in the email I posted earlier and I already provided all > > details there on how to fix that purely theoretical issue not part of > > the PoC with the provided debugfs tunables, so I won't repeat here. > > Also I read in another email you thought IBRS is a barrier or > something, it's not, it's purely temporarily preventing the CPU to > speculate through IBP BTB whatever, No. IBRS is like a barrier. You must write it between the 'problematic' loading of the branch targets, and the kernel code which might be affected. You cannot, on current hardware, merely set it once and forget about it. That is not sufficient. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:01 +0100, Andrea Arcangeli wrote: > > > On all current hardware, if you only set IBRS when you exit a guest, > > then you are not protecting yourself from userspace at all. IBRS acts > > as a *barrier* in all current hardware. > > Kernel memory is 100% protected if you set only IBRS at vmexit. > > Once IBRS is set, there's no way any userland (nor host nor guest) can > attack the kernel memory through spectre variant#2. That is not consistent with the documentation I've seen, which Intel have so far utterly failed to publish AFAICT. "a near indirect jump/call/return may be affected by code in a less privileged prediction mode that executed AFTER IBRS mode was last written with a value of 1" The kernel is only protected from branch targets set in userspace *BEFORE* the IBRS mode was last set to 1. If you set it to 1, then leave it like that while you run userspace and then kernel code again, you are not protected. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 13:01 +0100, Andrea Arcangeli wrote: > > > On all current hardware, if you only set IBRS when you exit a guest, > > then you are not protecting yourself from userspace at all. IBRS acts > > as a *barrier* in all current hardware. > > Kernel memory is 100% protected if you set only IBRS at vmexit. > > Once IBRS is set, there's no way any userland (nor host nor guest) can > attack the kernel memory through spectre variant#2. That is not consistent with the documentation I've seen, which Intel have so far utterly failed to publish AFAICT. "a near indirect jump/call/return may be affected by code in a less privileged prediction mode that executed AFTER IBRS mode was last written with a value of 1" The kernel is only protected from branch targets set in userspace *BEFORE* the IBRS mode was last set to 1. If you set it to 1, then leave it like that while you run userspace and then kernel code again, you are not protected. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:01:58PM +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 11:58:54AM +, David Woodhouse wrote: > > On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > I don't know why you're calling that 'IBRS=2'; are you getting > > > confused > > > > by Andrea's distro horridness? > > > > > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > > > set in SPEC_CTLR 100% of the time, except in guest mode. > > > > On all current hardware, if you only set IBRS when you exit a guest, > > then you are not protecting yourself from userspace at all. IBRS acts > > as a *barrier* in all current hardware. > > Kernel memory is 100% protected if you set only IBRS at vmexit. > > Once IBRS is set, there's no way any userland (nor host nor guest) can > attack the kernel memory through spectre variant#2. > > What is not protected is host userland from guest userland which is > point 3 in the email I posted earlier and I already provided all > details there on how to fix that purely theoretical issue not part of > the PoC with the provided debugfs tunables, so I won't repeat here. Also I read in another email you thought IBRS is a barrier or something, it's not, it's purely temporarily preventing the CPU to speculate through IBP BTB whatever, it will do the least possible amount of work without altering the internal state that can still be poisoned by the attacker but it's no problem because kernel mode won't make any use of that "poison". IBRS won't flush anything, the poison stays there, it's not barrier, and IBPB is the only barrier that flushes the content. IBPB is expensive but after IBPB you return running at core full speed but you can't be influenced by any poison somebody put in the CPU to influence your runtime. IBRS as opposed is not expensive to execute (relatively speaking) but then the CPU runs slower as it can't use those parts of the CPU that may have been poisoned by the attacker. So you can also flush the whole content of IBP/BTB at vmexit with IBPB instead of setting SPECT_CTRL to IBRS. Same thing at kernel entry. If you do that and you don't care about SMT/HT effect it's equivalent to setting IBRS at vmexit/kernelentry and in fact it'll also protect host userland from guest userland. Hope this clarifies what those IBRS IBPB features achieve. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 01:01:58PM +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 11:58:54AM +, David Woodhouse wrote: > > On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > I don't know why you're calling that 'IBRS=2'; are you getting > > > confused > > > > by Andrea's distro horridness? > > > > > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > > > set in SPEC_CTLR 100% of the time, except in guest mode. > > > > On all current hardware, if you only set IBRS when you exit a guest, > > then you are not protecting yourself from userspace at all. IBRS acts > > as a *barrier* in all current hardware. > > Kernel memory is 100% protected if you set only IBRS at vmexit. > > Once IBRS is set, there's no way any userland (nor host nor guest) can > attack the kernel memory through spectre variant#2. > > What is not protected is host userland from guest userland which is > point 3 in the email I posted earlier and I already provided all > details there on how to fix that purely theoretical issue not part of > the PoC with the provided debugfs tunables, so I won't repeat here. Also I read in another email you thought IBRS is a barrier or something, it's not, it's purely temporarily preventing the CPU to speculate through IBP BTB whatever, it will do the least possible amount of work without altering the internal state that can still be poisoned by the attacker but it's no problem because kernel mode won't make any use of that "poison". IBRS won't flush anything, the poison stays there, it's not barrier, and IBPB is the only barrier that flushes the content. IBPB is expensive but after IBPB you return running at core full speed but you can't be influenced by any poison somebody put in the CPU to influence your runtime. IBRS as opposed is not expensive to execute (relatively speaking) but then the CPU runs slower as it can't use those parts of the CPU that may have been poisoned by the attacker. So you can also flush the whole content of IBP/BTB at vmexit with IBPB instead of setting SPECT_CTRL to IBRS. Same thing at kernel entry. If you do that and you don't care about SMT/HT effect it's equivalent to setting IBRS at vmexit/kernelentry and in fact it'll also protect host userland from guest userland. Hope this clarifies what those IBRS IBPB features achieve. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 11:58:54AM +, David Woodhouse wrote: > On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > I don't know why you're calling that 'IBRS=2'; are you getting > > confused > > > by Andrea's distro horridness? > > > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > > set in SPEC_CTLR 100% of the time, except in guest mode. > > On all current hardware, if you only set IBRS when you exit a guest, > then you are not protecting yourself from userspace at all. IBRS acts > as a *barrier* in all current hardware. Kernel memory is 100% protected if you set only IBRS at vmexit. Once IBRS is set, there's no way any userland (nor host nor guest) can attack the kernel memory through spectre variant#2. What is not protected is host userland from guest userland which is point 3 in the email I posted earlier and I already provided all details there on how to fix that purely theoretical issue not part of the PoC with the provided debugfs tunables, so I won't repeat here.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 11:58:54AM +, David Woodhouse wrote: > On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > I don't know why you're calling that 'IBRS=2'; are you getting > > confused > > > by Andrea's distro horridness? > > > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > > set in SPEC_CTLR 100% of the time, except in guest mode. > > On all current hardware, if you only set IBRS when you exit a guest, > then you are not protecting yourself from userspace at all. IBRS acts > as a *barrier* in all current hardware. Kernel memory is 100% protected if you set only IBRS at vmexit. Once IBRS is set, there's no way any userland (nor host nor guest) can attack the kernel memory through spectre variant#2. What is not protected is host userland from guest userland which is point 3 in the email I posted earlier and I already provided all details there on how to fix that purely theoretical issue not part of the PoC with the provided debugfs tunables, so I won't repeat here.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > I don't know why you're calling that 'IBRS=2'; are you getting > confused > > by Andrea's distro horridness? > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > set in SPEC_CTLR 100% of the time, except in guest mode. On all current hardware, if you only set IBRS when you exit a guest, then you are not protecting yourself from userspace at all. IBRS acts as a *barrier* in all current hardware. Future CPUs will have a new feature where you *can* do something like this, but this is not available yet. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 12:54 +0100, Andrea Arcangeli wrote: > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > I don't know why you're calling that 'IBRS=2'; are you getting > confused > > by Andrea's distro horridness? > > Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS > set in SPEC_CTLR 100% of the time, except in guest mode. On all current hardware, if you only set IBRS when you exit a guest, then you are not protecting yourself from userspace at all. IBRS acts as a *barrier* in all current hardware. Future CPUs will have a new feature where you *can* do something like this, but this is not available yet. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > I don't know why you're calling that 'IBRS=2'; are you getting confused > by Andrea's distro horridness? Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS set in SPEC_CTLR 100% of the time, except in guest mode. IBRS is the "1" value to write in SPEC_CTRL MSR, SPEC_CTRL = 2 is not IBRS. "ibrs_enabled ibpb_enabled" semantics are practically the only thing we didn't change from the code that we've been presented with (aside from adding ibrs_enable 0 ibpb_enabled 2 new mode to fix spectre variant#2 on CPUs with no SPEC_CTRL/IBRS and only with IBPB_SUPPORT, that otherwise would have no kernel protection at all [of course modulo HT/SMT but that's not the PoC and ibrs_enabled 1 ibpb_enabled 1 also won't protect guest/user vs guest/user for HT/SMT and CPU pinning/isolation or ibrs 2 or HT/SMT disablement is needed for that]). NOTE: we moved ibrs_enabled ibpb_enable to debugfs, they were in more problematic location before we moved them there. We could have done something entirely different, but ibrs_enabled ibpb_enabled semantics as they were presented, looked as good as it could get to be able to achieve all the below on 0day: 1) ship a full fix for all CPUs (including those CPUs that cannot be fixed with reptolines) that protected the host kernel memory 100% from spectre variant#2 by default 2) to be able to get as much performance back as possible for all those cases where these attacks are irrelevant and performance is critical by setting ibrs_enabled 0 ibpb_enabled 0 (pti_enabled 0 is available too in the same debug/x86 location as well in fact). If you see complains on ibrs_enabled 1 slowing down ffmpeg or something compared to ibrs_enabled 0, well it's because we shipped those tunables in the first place that precisely allows so easy comparison, and that makes it so easy to set ibrs_enabled 0 and run at full CPU power in kernel mode too if spectre variant#2 is not a concern. Compare that to having to reboot the system and modify grub every time before running a test... that would have been truly horrid as a solution to get the all performance back where spectre variant#2 is no concern. If you're rendering a movie or you're transcoding with ffmpeg and nothing else and you're behind several level of firewalls or even disconnected from the network, it can all be disabled (including setting pti_enabled 0 if that's the only app in guest or host, i.e. a single memcached microservice running in a KVM guest). 3) allow to optionally make it impossible for a KVM guest userland to read KVM guest kernel memory by attacking qemu host userland, by simply setting ibrs_enabled 2 ibpb_enabled 1 for a subset of customers that may be especially concerned about that theoretical issue. Also allow to fix other theoretical HT/SMT attacks affecting guest/user mode vs guest/user mode with the same ibrs_enabled 2 ibpb_enabled 1. We don't know if those issues are practical but ibrs_enabled 2 will provide math guarantee for those as well. For the KVM guest userland attack on qemu userland not even SMEP helps as kindly confirmed by Dave. For that issue also the ibrs_enabled 0 ibpb_enabled 2 on host or even only in guest is enough, but likely it would perform worse than ibrs_enabled 2 ibpb_enabled 1, but then it depends on the CPU which is again why tunables are so handy so you can optimize for the best tradeoff. Not exactly sure how you could have accommodated for all 3 points above in an even more flexible way by removing those three debugfs tunables. If you don't want to deal with them because it's very complex to understand exactly what they do, there's a simple solution too: ignore them knowing it boots secure by default if either SPEC_CTRL/IBPB_SUPPORT are present in boot "dmesg" logs and to disable it all to get the performance back: echo 0 >ibrs_enabled echo 0 >ibpb_enabled. Even Dave just said that even with reptolines optimization patched in by default at boot, he'd still like to be able to enable the equivalent of ibrs_enabled 1, let alone those CPUs where reptolines can't fix spectre variant#2. Furthermore ibrs_enabled 2 is not obsoleted at all by kernel repotlines unless you rebuild the whole userland including all qemu dependencies and glibc with reptolines which would have been impossible to achieve on 0day no matter what. Retpolines are an optimization for later for a subset of CPUs so that we can stop setting IBRS in SPEC_CTRL while in kernel mode. On some CPUs IBRS or in general disabling the IBP is so fast I wouldn't even bother to enable reptolines at boot in if they don't even work safe to begin with. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > I don't know why you're calling that 'IBRS=2'; are you getting confused > by Andrea's distro horridness? Eh, yes he's got confused. ibrs_enabled 2 simply means to leave IBRS set in SPEC_CTLR 100% of the time, except in guest mode. IBRS is the "1" value to write in SPEC_CTRL MSR, SPEC_CTRL = 2 is not IBRS. "ibrs_enabled ibpb_enabled" semantics are practically the only thing we didn't change from the code that we've been presented with (aside from adding ibrs_enable 0 ibpb_enabled 2 new mode to fix spectre variant#2 on CPUs with no SPEC_CTRL/IBRS and only with IBPB_SUPPORT, that otherwise would have no kernel protection at all [of course modulo HT/SMT but that's not the PoC and ibrs_enabled 1 ibpb_enabled 1 also won't protect guest/user vs guest/user for HT/SMT and CPU pinning/isolation or ibrs 2 or HT/SMT disablement is needed for that]). NOTE: we moved ibrs_enabled ibpb_enable to debugfs, they were in more problematic location before we moved them there. We could have done something entirely different, but ibrs_enabled ibpb_enabled semantics as they were presented, looked as good as it could get to be able to achieve all the below on 0day: 1) ship a full fix for all CPUs (including those CPUs that cannot be fixed with reptolines) that protected the host kernel memory 100% from spectre variant#2 by default 2) to be able to get as much performance back as possible for all those cases where these attacks are irrelevant and performance is critical by setting ibrs_enabled 0 ibpb_enabled 0 (pti_enabled 0 is available too in the same debug/x86 location as well in fact). If you see complains on ibrs_enabled 1 slowing down ffmpeg or something compared to ibrs_enabled 0, well it's because we shipped those tunables in the first place that precisely allows so easy comparison, and that makes it so easy to set ibrs_enabled 0 and run at full CPU power in kernel mode too if spectre variant#2 is not a concern. Compare that to having to reboot the system and modify grub every time before running a test... that would have been truly horrid as a solution to get the all performance back where spectre variant#2 is no concern. If you're rendering a movie or you're transcoding with ffmpeg and nothing else and you're behind several level of firewalls or even disconnected from the network, it can all be disabled (including setting pti_enabled 0 if that's the only app in guest or host, i.e. a single memcached microservice running in a KVM guest). 3) allow to optionally make it impossible for a KVM guest userland to read KVM guest kernel memory by attacking qemu host userland, by simply setting ibrs_enabled 2 ibpb_enabled 1 for a subset of customers that may be especially concerned about that theoretical issue. Also allow to fix other theoretical HT/SMT attacks affecting guest/user mode vs guest/user mode with the same ibrs_enabled 2 ibpb_enabled 1. We don't know if those issues are practical but ibrs_enabled 2 will provide math guarantee for those as well. For the KVM guest userland attack on qemu userland not even SMEP helps as kindly confirmed by Dave. For that issue also the ibrs_enabled 0 ibpb_enabled 2 on host or even only in guest is enough, but likely it would perform worse than ibrs_enabled 2 ibpb_enabled 1, but then it depends on the CPU which is again why tunables are so handy so you can optimize for the best tradeoff. Not exactly sure how you could have accommodated for all 3 points above in an even more flexible way by removing those three debugfs tunables. If you don't want to deal with them because it's very complex to understand exactly what they do, there's a simple solution too: ignore them knowing it boots secure by default if either SPEC_CTRL/IBPB_SUPPORT are present in boot "dmesg" logs and to disable it all to get the performance back: echo 0 >ibrs_enabled echo 0 >ibpb_enabled. Even Dave just said that even with reptolines optimization patched in by default at boot, he'd still like to be able to enable the equivalent of ibrs_enabled 1, let alone those CPUs where reptolines can't fix spectre variant#2. Furthermore ibrs_enabled 2 is not obsoleted at all by kernel repotlines unless you rebuild the whole userland including all qemu dependencies and glibc with reptolines which would have been impossible to achieve on 0day no matter what. Retpolines are an optimization for later for a subset of CPUs so that we can stop setting IBRS in SPEC_CTRL while in kernel mode. On some CPUs IBRS or in general disabling the IBP is so fast I wouldn't even bother to enable reptolines at boot in if they don't even work safe to begin with. Thanks, Andrea
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 11:22:53AM +, David Woodhouse wrote: > On Wed, 2018-01-10 at 11:03 +0100, Peter Zijlstra wrote: > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > > > > > > > The only question I have is if retpoline works at all on SKL (with ucode > > > > update); BDW needs the ucode update for retpoline to work because of the > > > > RSB fallback. > > > As I understand it, Skylake is never getting the IBRS_ATT (all the > > > time) feature. That is for future CPUs only. > > > > Ah, so then I got confused by your earlier email. So retpoline will work > > on SKL, but it will, like BDW need the ucode update? > > Er, ... I'm not sure if SKL does need the µcode update. As I understand > it, the µcode update for BDW stops it from taking predictions for 'ret' > from anything but the RSB, which means that RSB-stuffing is sufficient. OK, that is what I understood as well. > On SKL I don't think there *is* a way to stop it doing that, which is > why we were originally saying we'd use IBRS on SKL instead. Ah, so we need the dynamic IBRS cruft for SKL :/ I really detest the IBRS 'feature' it would be ever so much better if we can otherwise fix that RSB issue, but I suppose people have looked at that already. > > Yes, continuation of the nomenclature of that earlier thread. > > Except I don't think it is; I think the various IBRS= options there > were about pointlessly fine-tuning when you frob the *existing* IBRS > bit, not about the future IBRS_ATT bit which really is set-and-forget. IBRS=1 was the dynamic mumbo jumbo, IBRS=2 set it once at CPU bringup and leave it on (or rather, that was the intention, patch had holes) IIRC. > > Still, point remains, do we ever need that MSR fiddling dynamic IBRS > > stuff now that we have retpolines? I would prefer to not have it if > > there is no compelling reason for it. > > We do want IBPB if we want to protect userspace processes (and VM > guests) from each other. Although the attack mode there is relatively > hard so *perhaps* we can resort to prayer instead? Sure, not arguing about IBPB, I understand that bit. > For IBRS though... yeah, good question. It *does* give additional > protection on SKL where 'ret' might get mispredicted and the RSB- > stuffing is not always guaranteed to be sufficient, but appealing to > the deity of your choice might also be an acceptable mitigation there? I'm just trying to understand the various undocumented options here. As pjt [1] put it nicely: "The majority of the confusion does not stem from this being complicated. It's that there's been great reluctance to document the details" There just isn't clear and coherent information on this stuff around, so everybody gets confused. [1] https://lkml.kernel.org/r/capm31rkv-p40p0yuy7qjvswt7sr2wurbzqj_ma1trajnibw...@mail.gmail.com
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 11:22:53AM +, David Woodhouse wrote: > On Wed, 2018-01-10 at 11:03 +0100, Peter Zijlstra wrote: > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > > > > > > > The only question I have is if retpoline works at all on SKL (with ucode > > > > update); BDW needs the ucode update for retpoline to work because of the > > > > RSB fallback. > > > As I understand it, Skylake is never getting the IBRS_ATT (all the > > > time) feature. That is for future CPUs only. > > > > Ah, so then I got confused by your earlier email. So retpoline will work > > on SKL, but it will, like BDW need the ucode update? > > Er, ... I'm not sure if SKL does need the µcode update. As I understand > it, the µcode update for BDW stops it from taking predictions for 'ret' > from anything but the RSB, which means that RSB-stuffing is sufficient. OK, that is what I understood as well. > On SKL I don't think there *is* a way to stop it doing that, which is > why we were originally saying we'd use IBRS on SKL instead. Ah, so we need the dynamic IBRS cruft for SKL :/ I really detest the IBRS 'feature' it would be ever so much better if we can otherwise fix that RSB issue, but I suppose people have looked at that already. > > Yes, continuation of the nomenclature of that earlier thread. > > Except I don't think it is; I think the various IBRS= options there > were about pointlessly fine-tuning when you frob the *existing* IBRS > bit, not about the future IBRS_ATT bit which really is set-and-forget. IBRS=1 was the dynamic mumbo jumbo, IBRS=2 set it once at CPU bringup and leave it on (or rather, that was the intention, patch had holes) IIRC. > > Still, point remains, do we ever need that MSR fiddling dynamic IBRS > > stuff now that we have retpolines? I would prefer to not have it if > > there is no compelling reason for it. > > We do want IBPB if we want to protect userspace processes (and VM > guests) from each other. Although the attack mode there is relatively > hard so *perhaps* we can resort to prayer instead? Sure, not arguing about IBPB, I understand that bit. > For IBRS though... yeah, good question. It *does* give additional > protection on SKL where 'ret' might get mispredicted and the RSB- > stuffing is not always guaranteed to be sufficient, but appealing to > the deity of your choice might also be an acceptable mitigation there? I'm just trying to understand the various undocumented options here. As pjt [1] put it nicely: "The majority of the confusion does not stem from this being complicated. It's that there's been great reluctance to document the details" There just isn't clear and coherent information on this stuff around, so everybody gets confused. [1] https://lkml.kernel.org/r/capm31rkv-p40p0yuy7qjvswt7sr2wurbzqj_ma1trajnibw...@mail.gmail.com
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, David Woodhouse wrote: > On Wed, 2018-01-10 at 11:03 +0100, Peter Zijlstra wrote: > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > > > > > > > The only question I have is if retpoline works at all on SKL (with ucode > > > > update); BDW needs the ucode update for retpoline to work because of the > > > > RSB fallback. > > > As I understand it, Skylake is never getting the IBRS_ATT (all the > > > time) feature. That is for future CPUs only. > > > > Ah, so then I got confused by your earlier email. So retpoline will work > > on SKL, but it will, like BDW need the ucode update? > > Er, ... I'm not sure if SKL does need the µcode update. As I understand > it, the µcode update for BDW stops it from taking predictions for 'ret' > from anything but the RSB, which means that RSB-stuffing is sufficient. > > On SKL I don't think there *is* a way to stop it doing that, which is > why we were originally saying we'd use IBRS on SKL instead. > > Do ask someone who works at Intel for confirmation of the above :) To get how many contradicting answers? I'm really fed up with the desinformation politics in this matter. > > > I don't know why you're calling that 'IBRS=2'; are you getting confused > > > by Andrea's distro horridness? > > Yes, continuation of the nomenclature of that earlier thread. > > Except I don't think it is; I think the various IBRS= options there > were about pointlessly fine-tuning when you frob the *existing* IBRS > bit, not about the future IBRS_ATT bit which really is set-and-forget. So we can offer the spectre_v2=ibrs option now, which just enforces it and be done with it. The auto mode will not select IBRS. The question remains, whether retpoline needs to be still in place even with IBRS enabled. I'd say yes, unless the IBRS always on mechanics fully works as intended. But that's not a question we have to answer today. As Ingo stated, we also can require to have microcode early, which is in fact a single initrd/reboot required. With the flood of reboots we've seen in the last week and which will continue a while that really should not matter at all. But we can avoid all the complexity of late switching and other crap. > > Still, point remains, do we ever need that MSR fiddling dynamic IBRS > > stuff now that we have retpolines? I would prefer to not have it if > > there is no compelling reason for it. > > We do want IBPB if we want to protect userspace processes (and VM > guests) from each other. IBPB is an orthogonal feature and that's the one which is also implemented by AMD, who did not bother with the IBRS voodoo, if my limited knowledge (READ: Where is the damned documentation (both Intel and AMD)?) is correct. > Although the attack mode there is relatively hard so *perhaps* we can > resort to prayer instead? That's the only thing we can do anyway due to lack of authorative statements. That lack is either caused by lawyer weaseling or by fundamental lack of confidence that the operation principles of the CPU are fully understood. Maybe both. Not very useful in any case. > For IBRS though... yeah, good question. It *does* give additional > protection on SKL where 'ret' might get mispredicted and the RSB- > stuffing is not always guaranteed to be sufficient, but appealing to > the deity of your choice might also be an acceptable mitigation there? So the right thing to do is: if (spectre_v2_commandline == "ibrs" || !retpoline) ibrs_enable_if_ucode_supports(); if (retpoline) retpoline_select(); We definitely should add spectre_v2=pray as a valid boot option choice. Thanks, tglx
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 10 Jan 2018, David Woodhouse wrote: > On Wed, 2018-01-10 at 11:03 +0100, Peter Zijlstra wrote: > > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > > > > > > > The only question I have is if retpoline works at all on SKL (with ucode > > > > update); BDW needs the ucode update for retpoline to work because of the > > > > RSB fallback. > > > As I understand it, Skylake is never getting the IBRS_ATT (all the > > > time) feature. That is for future CPUs only. > > > > Ah, so then I got confused by your earlier email. So retpoline will work > > on SKL, but it will, like BDW need the ucode update? > > Er, ... I'm not sure if SKL does need the µcode update. As I understand > it, the µcode update for BDW stops it from taking predictions for 'ret' > from anything but the RSB, which means that RSB-stuffing is sufficient. > > On SKL I don't think there *is* a way to stop it doing that, which is > why we were originally saying we'd use IBRS on SKL instead. > > Do ask someone who works at Intel for confirmation of the above :) To get how many contradicting answers? I'm really fed up with the desinformation politics in this matter. > > > I don't know why you're calling that 'IBRS=2'; are you getting confused > > > by Andrea's distro horridness? > > Yes, continuation of the nomenclature of that earlier thread. > > Except I don't think it is; I think the various IBRS= options there > were about pointlessly fine-tuning when you frob the *existing* IBRS > bit, not about the future IBRS_ATT bit which really is set-and-forget. So we can offer the spectre_v2=ibrs option now, which just enforces it and be done with it. The auto mode will not select IBRS. The question remains, whether retpoline needs to be still in place even with IBRS enabled. I'd say yes, unless the IBRS always on mechanics fully works as intended. But that's not a question we have to answer today. As Ingo stated, we also can require to have microcode early, which is in fact a single initrd/reboot required. With the flood of reboots we've seen in the last week and which will continue a while that really should not matter at all. But we can avoid all the complexity of late switching and other crap. > > Still, point remains, do we ever need that MSR fiddling dynamic IBRS > > stuff now that we have retpolines? I would prefer to not have it if > > there is no compelling reason for it. > > We do want IBPB if we want to protect userspace processes (and VM > guests) from each other. IBPB is an orthogonal feature and that's the one which is also implemented by AMD, who did not bother with the IBRS voodoo, if my limited knowledge (READ: Where is the damned documentation (both Intel and AMD)?) is correct. > Although the attack mode there is relatively hard so *perhaps* we can > resort to prayer instead? That's the only thing we can do anyway due to lack of authorative statements. That lack is either caused by lawyer weaseling or by fundamental lack of confidence that the operation principles of the CPU are fully understood. Maybe both. Not very useful in any case. > For IBRS though... yeah, good question. It *does* give additional > protection on SKL where 'ret' might get mispredicted and the RSB- > stuffing is not always guaranteed to be sufficient, but appealing to > the deity of your choice might also be an acceptable mitigation there? So the right thing to do is: if (spectre_v2_commandline == "ibrs" || !retpoline) ibrs_enable_if_ucode_supports(); if (retpoline) retpoline_select(); We definitely should add spectre_v2=pray as a valid boot option choice. Thanks, tglx
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 11:03 +0100, Peter Zijlstra wrote: > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > > > > The only question I have is if retpoline works at all on SKL (with ucode > > > update); BDW needs the ucode update for retpoline to work because of the > > > RSB fallback. > > As I understand it, Skylake is never getting the IBRS_ATT (all the > > time) feature. That is for future CPUs only. > > Ah, so then I got confused by your earlier email. So retpoline will work > on SKL, but it will, like BDW need the ucode update? Er, ... I'm not sure if SKL does need the µcode update. As I understand it, the µcode update for BDW stops it from taking predictions for 'ret' from anything but the RSB, which means that RSB-stuffing is sufficient. On SKL I don't think there *is* a way to stop it doing that, which is why we were originally saying we'd use IBRS on SKL instead. Do ask someone who works at Intel for confirmation of the above :) > > I don't know why you're calling that 'IBRS=2'; are you getting confused > > by Andrea's distro horridness? > Yes, continuation of the nomenclature of that earlier thread. Except I don't think it is; I think the various IBRS= options there were about pointlessly fine-tuning when you frob the *existing* IBRS bit, not about the future IBRS_ATT bit which really is set-and-forget. > Still, point remains, do we ever need that MSR fiddling dynamic IBRS > stuff now that we have retpolines? I would prefer to not have it if > there is no compelling reason for it. We do want IBPB if we want to protect userspace processes (and VM guests) from each other. Although the attack mode there is relatively hard so *perhaps* we can resort to prayer instead? For IBRS though... yeah, good question. It *does* give additional protection on SKL where 'ret' might get mispredicted and the RSB- stuffing is not always guaranteed to be sufficient, but appealing to the deity of your choice might also be an acceptable mitigation there? When IBRS came first and retpoline was purely a performance optimisation, it seemed to be a no-brainer: performance optimisations don't get to say "well, it's only a *little* bit less secure but yay it's faster!". Now with retpoline working first, it does seem *psychologically* to be a slightly different question, but really it's the same choice. I don't have a real answer for precisely how hard it would be to exploit the theoretical loopholes that retpoline opens on SKL. One observation is that for an attacker to find a gadget is slightly easier than it used to be, in some ways. Previously, if you found gadgets you wanted to use, they had to *work*. They had to return to some sane address and not just crash the system. That's not true in speculation. They can leak the data and then crash or go off into the weeds, but the crash will never really happen. The leak will. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 11:03 +0100, Peter Zijlstra wrote: > On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > > > > > > > The only question I have is if retpoline works at all on SKL (with ucode > > > update); BDW needs the ucode update for retpoline to work because of the > > > RSB fallback. > > As I understand it, Skylake is never getting the IBRS_ATT (all the > > time) feature. That is for future CPUs only. > > Ah, so then I got confused by your earlier email. So retpoline will work > on SKL, but it will, like BDW need the ucode update? Er, ... I'm not sure if SKL does need the µcode update. As I understand it, the µcode update for BDW stops it from taking predictions for 'ret' from anything but the RSB, which means that RSB-stuffing is sufficient. On SKL I don't think there *is* a way to stop it doing that, which is why we were originally saying we'd use IBRS on SKL instead. Do ask someone who works at Intel for confirmation of the above :) > > I don't know why you're calling that 'IBRS=2'; are you getting confused > > by Andrea's distro horridness? > Yes, continuation of the nomenclature of that earlier thread. Except I don't think it is; I think the various IBRS= options there were about pointlessly fine-tuning when you frob the *existing* IBRS bit, not about the future IBRS_ATT bit which really is set-and-forget. > Still, point remains, do we ever need that MSR fiddling dynamic IBRS > stuff now that we have retpolines? I would prefer to not have it if > there is no compelling reason for it. We do want IBPB if we want to protect userspace processes (and VM guests) from each other. Although the attack mode there is relatively hard so *perhaps* we can resort to prayer instead? For IBRS though... yeah, good question. It *does* give additional protection on SKL where 'ret' might get mispredicted and the RSB- stuffing is not always guaranteed to be sufficient, but appealing to the deity of your choice might also be an acceptable mitigation there? When IBRS came first and retpoline was purely a performance optimisation, it seemed to be a no-brainer: performance optimisations don't get to say "well, it's only a *little* bit less secure but yay it's faster!". Now with retpoline working first, it does seem *psychologically* to be a slightly different question, but really it's the same choice. I don't have a real answer for precisely how hard it would be to exploit the theoretical loopholes that retpoline opens on SKL. One observation is that for an attacker to find a gadget is slightly easier than it used to be, in some ways. Previously, if you found gadgets you wanted to use, they had to *work*. They had to return to some sane address and not just crash the system. That's not true in speculation. They can leak the data and then crash or go off into the weeds, but the crash will never really happen. The leak will. smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > The only question I have is if retpoline works at all on SKL (with ucode > > update); BDW needs the ucode update for retpoline to work because of the > > RSB fallback. > > As I understand it, Skylake is never getting the IBRS_ATT (all the > time) feature. That is for future CPUs only. Ah, so then I got confused by your earlier email. So retpoline will work on SKL, but it will, like BDW need the ucode update? > I don't know why you're calling that 'IBRS=2'; are you getting confused > by Andrea's distro horridness? Yes, continuation of the nomenclature of that earlier thread. Still, point remains, do we ever need that MSR fiddling dynamic IBRS stuff now that we have retpolines? I would prefer to not have it if there is no compelling reason for it.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, Jan 10, 2018 at 09:27:59AM +, David Woodhouse wrote: > > The only question I have is if retpoline works at all on SKL (with ucode > > update); BDW needs the ucode update for retpoline to work because of the > > RSB fallback. > > As I understand it, Skylake is never getting the IBRS_ATT (all the > time) feature. That is for future CPUs only. Ah, so then I got confused by your earlier email. So retpoline will work on SKL, but it will, like BDW need the ucode update? > I don't know why you're calling that 'IBRS=2'; are you getting confused > by Andrea's distro horridness? Yes, continuation of the nomenclature of that earlier thread. Still, point remains, do we ever need that MSR fiddling dynamic IBRS stuff now that we have retpolines? I would prefer to not have it if there is no compelling reason for it.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 10:22 +0100, Peter Zijlstra wrote: > On Tue, Jan 09, 2018 at 06:02:53PM -0800, Dave Hansen wrote: > > > > On 01/09/2018 05:06 PM, Thomas Gleixner wrote: > > > > > > --- a/arch/x86/kernel/cpu/bugs.c > > > +++ b/arch/x86/kernel/cpu/bugs.c > > > @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { > > > SPECTRE_V2_CMD_RETPOLINE, > > > SPECTRE_V2_CMD_RETPOLINE_GENERIC, > > > SPECTRE_V2_CMD_RETPOLINE_AMD, > > > + SPECTRE_V2_CMD_IBRS, > > > }; > > A few nits on this: > > > > IBRS should not default on anywhere, which goes double when retpolines > > are available. > > > > I think I'd also prefer that we separate the IBRS and retpoline enabling > > so that you can do both if you want. They do nearly the same thing in > > practice, but I can't convince myself that you never ever need IBRS once > > retpolines are in place. > As per: > > https://lkml.kernel.org/r/1515460999.4423.104.ca...@amazon.co.uk > > IBRS=2 (always on) is preferred for SKL+ over retpoline. > > And from what I gather IBRS=1 is never better than retpoline, IBRS=1 is > both slower and covers less AFAIU (please educate if I'm wrong). > > From this point, I would prefer to not even have the IBRS=1 code. > > The only question I have is if retpoline works at all on SKL (with ucode > update); BDW needs the ucode update for retpoline to work because of the > RSB fallback. As I understand it, Skylake is never getting the IBRS_ATT (all the time) feature. That is for future CPUs only. I don't know why you're calling that 'IBRS=2'; are you getting confused by Andrea's distro horridness? smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Wed, 2018-01-10 at 10:22 +0100, Peter Zijlstra wrote: > On Tue, Jan 09, 2018 at 06:02:53PM -0800, Dave Hansen wrote: > > > > On 01/09/2018 05:06 PM, Thomas Gleixner wrote: > > > > > > --- a/arch/x86/kernel/cpu/bugs.c > > > +++ b/arch/x86/kernel/cpu/bugs.c > > > @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { > > > SPECTRE_V2_CMD_RETPOLINE, > > > SPECTRE_V2_CMD_RETPOLINE_GENERIC, > > > SPECTRE_V2_CMD_RETPOLINE_AMD, > > > + SPECTRE_V2_CMD_IBRS, > > > }; > > A few nits on this: > > > > IBRS should not default on anywhere, which goes double when retpolines > > are available. > > > > I think I'd also prefer that we separate the IBRS and retpoline enabling > > so that you can do both if you want. They do nearly the same thing in > > practice, but I can't convince myself that you never ever need IBRS once > > retpolines are in place. > As per: > > https://lkml.kernel.org/r/1515460999.4423.104.ca...@amazon.co.uk > > IBRS=2 (always on) is preferred for SKL+ over retpoline. > > And from what I gather IBRS=1 is never better than retpoline, IBRS=1 is > both slower and covers less AFAIU (please educate if I'm wrong). > > From this point, I would prefer to not even have the IBRS=1 code. > > The only question I have is if retpoline works at all on SKL (with ucode > update); BDW needs the ucode update for retpoline to work because of the > RSB fallback. As I understand it, Skylake is never getting the IBRS_ATT (all the time) feature. That is for future CPUs only. I don't know why you're calling that 'IBRS=2'; are you getting confused by Andrea's distro horridness? smime.p7s Description: S/MIME cryptographic signature
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Tue, Jan 09, 2018 at 06:02:53PM -0800, Dave Hansen wrote: > On 01/09/2018 05:06 PM, Thomas Gleixner wrote: > > --- a/arch/x86/kernel/cpu/bugs.c > > +++ b/arch/x86/kernel/cpu/bugs.c > > @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { > > SPECTRE_V2_CMD_RETPOLINE, > > SPECTRE_V2_CMD_RETPOLINE_GENERIC, > > SPECTRE_V2_CMD_RETPOLINE_AMD, > > + SPECTRE_V2_CMD_IBRS, > > }; > > A few nits on this: > > IBRS should not default on anywhere, which goes double when retpolines > are available. > > I think I'd also prefer that we separate the IBRS and retpoline enabling > so that you can do both if you want. They do nearly the same thing in > practice, but I can't convince myself that you never ever need IBRS once > retpolines are in place. As per: https://lkml.kernel.org/r/1515460999.4423.104.ca...@amazon.co.uk IBRS=2 (always on) is preferred for SKL+ over retpoline. And from what I gather IBRS=1 is never better than retpoline, IBRS=1 is both slower and covers less AFAIU (please educate if I'm wrong). >From this point, I would prefer to not even have the IBRS=1 code. The only question I have is if retpoline works at all on SKL (with ucode update); BDW needs the ucode update for retpoline to work because of the RSB fallback.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Tue, Jan 09, 2018 at 06:02:53PM -0800, Dave Hansen wrote: > On 01/09/2018 05:06 PM, Thomas Gleixner wrote: > > --- a/arch/x86/kernel/cpu/bugs.c > > +++ b/arch/x86/kernel/cpu/bugs.c > > @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { > > SPECTRE_V2_CMD_RETPOLINE, > > SPECTRE_V2_CMD_RETPOLINE_GENERIC, > > SPECTRE_V2_CMD_RETPOLINE_AMD, > > + SPECTRE_V2_CMD_IBRS, > > }; > > A few nits on this: > > IBRS should not default on anywhere, which goes double when retpolines > are available. > > I think I'd also prefer that we separate the IBRS and retpoline enabling > so that you can do both if you want. They do nearly the same thing in > practice, but I can't convince myself that you never ever need IBRS once > retpolines are in place. As per: https://lkml.kernel.org/r/1515460999.4423.104.ca...@amazon.co.uk IBRS=2 (always on) is preferred for SKL+ over retpoline. And from what I gather IBRS=1 is never better than retpoline, IBRS=1 is both slower and covers less AFAIU (please educate if I'm wrong). >From this point, I would prefer to not even have the IBRS=1 code. The only question I have is if retpoline works at all on SKL (with ucode update); BDW needs the ucode update for retpoline to work because of the RSB fallback.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Tue, Jan 9, 2018 at 8:02 PM, Dave Hansenwrote: > On 01/09/2018 05:06 PM, Thomas Gleixner wrote: >> --- a/arch/x86/kernel/cpu/bugs.c >> +++ b/arch/x86/kernel/cpu/bugs.c >> @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { >> SPECTRE_V2_CMD_RETPOLINE, >> SPECTRE_V2_CMD_RETPOLINE_GENERIC, >> SPECTRE_V2_CMD_RETPOLINE_AMD, >> + SPECTRE_V2_CMD_IBRS, >> }; > > A few nits on this: > > IBRS should not default on anywhere, which goes double when retpolines > are available. > > I think I'd also prefer that we separate the IBRS and retpoline enabling > so that you can do both if you want. They do nearly the same thing in > practice, but I can't convince myself that you never ever need IBRS once > retpolines are in place. Fairly strong agreement here. IBRS being separately configurable gives us an option for the paranoid, and allows distros to ship with it off by default.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On Tue, Jan 9, 2018 at 8:02 PM, Dave Hansen wrote: > On 01/09/2018 05:06 PM, Thomas Gleixner wrote: >> --- a/arch/x86/kernel/cpu/bugs.c >> +++ b/arch/x86/kernel/cpu/bugs.c >> @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { >> SPECTRE_V2_CMD_RETPOLINE, >> SPECTRE_V2_CMD_RETPOLINE_GENERIC, >> SPECTRE_V2_CMD_RETPOLINE_AMD, >> + SPECTRE_V2_CMD_IBRS, >> }; > > A few nits on this: > > IBRS should not default on anywhere, which goes double when retpolines > are available. > > I think I'd also prefer that we separate the IBRS and retpoline enabling > so that you can do both if you want. They do nearly the same thing in > practice, but I can't convince myself that you never ever need IBRS once > retpolines are in place. Fairly strong agreement here. IBRS being separately configurable gives us an option for the paranoid, and allows distros to ship with it off by default.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On 01/09/2018 05:06 PM, Thomas Gleixner wrote: > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { > SPECTRE_V2_CMD_RETPOLINE, > SPECTRE_V2_CMD_RETPOLINE_GENERIC, > SPECTRE_V2_CMD_RETPOLINE_AMD, > + SPECTRE_V2_CMD_IBRS, > }; A few nits on this: IBRS should not default on anywhere, which goes double when retpolines are available. I think I'd also prefer that we separate the IBRS and retpoline enabling so that you can do both if you want. They do nearly the same thing in practice, but I can't convince myself that you never ever need IBRS once retpolines are in place.
Re: [patch RFC 5/5] x86/speculation: Add basic speculation control code
On 01/09/2018 05:06 PM, Thomas Gleixner wrote: > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd { > SPECTRE_V2_CMD_RETPOLINE, > SPECTRE_V2_CMD_RETPOLINE_GENERIC, > SPECTRE_V2_CMD_RETPOLINE_AMD, > + SPECTRE_V2_CMD_IBRS, > }; A few nits on this: IBRS should not default on anywhere, which goes double when retpolines are available. I think I'd also prefer that we separate the IBRS and retpoline enabling so that you can do both if you want. They do nearly the same thing in practice, but I can't convince myself that you never ever need IBRS once retpolines are in place.