* [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
@ 2019-11-13 19:33 Waiman Long
2019-11-14 17:45 ` Borislav Petkov
2019-11-14 20:12 ` Pawan Gupta
0 siblings, 2 replies; 12+ messages in thread
From: Waiman Long @ 2019-11-13 19:33 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86
Cc: linux-kernel, Josh Poimboeuf, Greg Kroah-Hartman, Pawan Gupta,
Mark Gross, Tony Luck, Waiman Long
For MDS vulnerable processors with TSX support, enabling either MDS
or TAA mitigations will enable the use of VERW to flush internal
processor buffers at the right code path. IOW, they are either both
mitigated or both not mitigated. However, if the command line options
are inconsistent, the vulnerabilites sysfs files may not report the
mitigation status correctly.
For example, with only the "mds=off" option:
vulnerabilities/mds:Vulnerable; SMT vulnerable
vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
The mds vulnerabilities file has wrong status in this case.
Change taa_select_mitigation() to sync up the two mitigation status
and have them turned off if both "mds=off" and "tsx_async_abort=off"
are present.
Signed-off-by: Waiman Long <longman@redhat.com>
---
arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4c7b0fa15a19..418d41c1fd0d 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
return;
}
- /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
- if (taa_mitigation == TAA_MITIGATION_OFF)
+ /*
+ * TAA mitigation via VERW is turned off if both
+ * tsx_async_abort=off and mds=off are specified.
+ */
+ if (taa_mitigation == TAA_MITIGATION_OFF &&
+ mds_mitigation == MDS_MITIGATION_OFF)
goto out;
if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
@@ -339,6 +343,15 @@ static void __init taa_select_mitigation(void)
if (taa_nosmt || cpu_mitigations_auto_nosmt())
cpu_smt_disable(false);
+ /*
+ * Update MDS mitigation, if necessary, as the mds_user_clear is
+ * now enabled for TAA mitigation.
+ */
+ if (mds_mitigation == MDS_MITIGATION_OFF &&
+ boot_cpu_has_bug(X86_BUG_MDS)) {
+ mds_mitigation = MDS_MITIGATION_FULL;
+ mds_select_mitigation();
+ }
out:
pr_info("%s\n", taa_strings[taa_mitigation]);
}
--
2.18.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-13 19:33 [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status Waiman Long
@ 2019-11-14 17:45 ` Borislav Petkov
2019-11-14 17:53 ` Waiman Long
2019-11-14 20:12 ` Pawan Gupta
1 sibling, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2019-11-14 17:45 UTC (permalink / raw)
To: Waiman Long
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
Josh Poimboeuf, Greg Kroah-Hartman, Pawan Gupta, Mark Gross,
Tony Luck
On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
> For MDS vulnerable processors with TSX support, enabling either MDS
> or TAA mitigations will enable the use of VERW to flush internal
> processor buffers at the right code path. IOW, they are either both
> mitigated or both not mitigated. However, if the command line options
> are inconsistent, the vulnerabilites sysfs files may not report the
> mitigation status correctly.
>
> For example, with only the "mds=off" option:
>
> vulnerabilities/mds:Vulnerable; SMT vulnerable
> vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
>
> The mds vulnerabilities file has wrong status in this case.
>
> Change taa_select_mitigation() to sync up the two mitigation status
> and have them turned off if both "mds=off" and "tsx_async_abort=off"
> are present.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 4c7b0fa15a19..418d41c1fd0d 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
> return;
> }
>
> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
> - if (taa_mitigation == TAA_MITIGATION_OFF)
> + /*
> + * TAA mitigation via VERW is turned off if both
> + * tsx_async_abort=off and mds=off are specified.
> + */
So this changes the dependency of switches so if anything, it should be
properly documented first in all three:
Documentation/admin-guide/hw-vuln/tsx_async_abort.rst
Documentation/x86/tsx_async_abort.rst
Documentation/admin-guide/kernel-parameters.txt
However, before we do that, we need to agree on functionality:
Will the mitigations be disabled only with *both* =off supplied on the
command line or should the mitigations be disabled when *any* of the two
=off is supplied?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 17:45 ` Borislav Petkov
@ 2019-11-14 17:53 ` Waiman Long
2019-11-14 18:48 ` Thomas Gleixner
0 siblings, 1 reply; 12+ messages in thread
From: Waiman Long @ 2019-11-14 17:53 UTC (permalink / raw)
To: Borislav Petkov
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
Josh Poimboeuf, Greg Kroah-Hartman, Pawan Gupta, Mark Gross,
Tony Luck
On 11/14/19 12:45 PM, Borislav Petkov wrote:
> On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
>> For MDS vulnerable processors with TSX support, enabling either MDS
>> or TAA mitigations will enable the use of VERW to flush internal
>> processor buffers at the right code path. IOW, they are either both
>> mitigated or both not mitigated. However, if the command line options
>> are inconsistent, the vulnerabilites sysfs files may not report the
>> mitigation status correctly.
>>
>> For example, with only the "mds=off" option:
>>
>> vulnerabilities/mds:Vulnerable; SMT vulnerable
>> vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
>>
>> The mds vulnerabilities file has wrong status in this case.
>>
>> Change taa_select_mitigation() to sync up the two mitigation status
>> and have them turned off if both "mds=off" and "tsx_async_abort=off"
>> are present.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>> arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>> index 4c7b0fa15a19..418d41c1fd0d 100644
>> --- a/arch/x86/kernel/cpu/bugs.c
>> +++ b/arch/x86/kernel/cpu/bugs.c
>> @@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
>> return;
>> }
>>
>> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
>> - if (taa_mitigation == TAA_MITIGATION_OFF)
>> + /*
>> + * TAA mitigation via VERW is turned off if both
>> + * tsx_async_abort=off and mds=off are specified.
>> + */
> So this changes the dependency of switches so if anything, it should be
> properly documented first in all three:
>
> Documentation/admin-guide/hw-vuln/tsx_async_abort.rst
> Documentation/x86/tsx_async_abort.rst
> Documentation/admin-guide/kernel-parameters.txt
>
> However, before we do that, we need to agree on functionality:
I agree that the documentation needs to be updated. I am going to do
that once we have a consensus of what is the right thing to do.
> Will the mitigations be disabled only with *both* =off supplied on the
> command line or should the mitigations be disabled when *any* of the two
> =off is supplied?
The mitigation is disabled only with BOTH =off supplied or
"mitigations=off". This is the current behavior. This patch is just to
make sure that vulnerabilities files reflect the actual behavior. Of
course, we can change it to disable mitigation with either =off if this
is what the consensus turn out to be.
Cheers,
Longman
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 17:53 ` Waiman Long
@ 2019-11-14 18:48 ` Thomas Gleixner
2019-11-14 19:11 ` Waiman Long
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2019-11-14 18:48 UTC (permalink / raw)
To: Waiman Long
Cc: Borislav Petkov, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
Josh Poimboeuf, Greg Kroah-Hartman, Pawan Gupta, Mark Gross,
Tony Luck
On Thu, 14 Nov 2019, Waiman Long wrote:
> On 11/14/19 12:45 PM, Borislav Petkov wrote:
> >> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
> >> - if (taa_mitigation == TAA_MITIGATION_OFF)
> >> + /*
> >> + * TAA mitigation via VERW is turned off if both
> >> + * tsx_async_abort=off and mds=off are specified.
> >> + */
> > So this changes the dependency of switches so if anything, it should be
> > properly documented first in all three:
> >
> > Documentation/admin-guide/hw-vuln/tsx_async_abort.rst
> > Documentation/x86/tsx_async_abort.rst
> > Documentation/admin-guide/kernel-parameters.txt
> >
> > However, before we do that, we need to agree on functionality:
> I agree that the documentation needs to be updated. I am going to do
> that once we have a consensus of what is the right thing to do.
> > Will the mitigations be disabled only with *both* =off supplied on the
> > command line or should the mitigations be disabled when *any* of the two
> > =off is supplied?
>
> The mitigation is disabled only with BOTH =off supplied or
> "mitigations=off". This is the current behavior. This patch is just to
> make sure that vulnerabilities files reflect the actual behavior. Of
> course, we can change it to disable mitigation with either =off if this
> is what the consensus turn out to be.
I think the current behaviour is correct. It's just a coincidence that both
issues happen to use the same mitigation technology in the exactly same
places. So if you leave one on then the other gets mitigated as a side
effect and the sysfs file should reflect that.
Thanks,
tglx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 18:48 ` Thomas Gleixner
@ 2019-11-14 19:11 ` Waiman Long
0 siblings, 0 replies; 12+ messages in thread
From: Waiman Long @ 2019-11-14 19:11 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Borislav Petkov, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
Josh Poimboeuf, Greg Kroah-Hartman, Pawan Gupta, Mark Gross,
Tony Luck
On 11/14/19 1:48 PM, Thomas Gleixner wrote:
> On Thu, 14 Nov 2019, Waiman Long wrote:
>> On 11/14/19 12:45 PM, Borislav Petkov wrote:
>>>> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
>>>> - if (taa_mitigation == TAA_MITIGATION_OFF)
>>>> + /*
>>>> + * TAA mitigation via VERW is turned off if both
>>>> + * tsx_async_abort=off and mds=off are specified.
>>>> + */
>>> So this changes the dependency of switches so if anything, it should be
>>> properly documented first in all three:
>>>
>>> Documentation/admin-guide/hw-vuln/tsx_async_abort.rst
>>> Documentation/x86/tsx_async_abort.rst
>>> Documentation/admin-guide/kernel-parameters.txt
>>>
>>> However, before we do that, we need to agree on functionality:
>> I agree that the documentation needs to be updated. I am going to do
>> that once we have a consensus of what is the right thing to do.
>>> Will the mitigations be disabled only with *both* =off supplied on the
>>> command line or should the mitigations be disabled when *any* of the two
>>> =off is supplied?
>> The mitigation is disabled only with BOTH =off supplied or
>> "mitigations=off". This is the current behavior. This patch is just to
>> make sure that vulnerabilities files reflect the actual behavior. Of
>> course, we can change it to disable mitigation with either =off if this
>> is what the consensus turn out to be.
> I think the current behaviour is correct. It's just a coincidence that both
> issues happen to use the same mitigation technology in the exactly same
> places. So if you leave one on then the other gets mitigated as a side
> effect and the sysfs file should reflect that.
>
> Thanks,
>
> tglx
>
Good to hear that. I will send a v2 patch with document update.
Cheers,
Longman
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-13 19:33 [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status Waiman Long
2019-11-14 17:45 ` Borislav Petkov
@ 2019-11-14 20:12 ` Pawan Gupta
2019-11-14 20:35 ` Josh Poimboeuf
2019-11-14 21:58 ` Waiman Long
1 sibling, 2 replies; 12+ messages in thread
From: Pawan Gupta @ 2019-11-14 20:12 UTC (permalink / raw)
To: Waiman Long
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86, linux-kernel, Josh Poimboeuf, Greg Kroah-Hartman,
Mark Gross, Tony Luck
On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
> For MDS vulnerable processors with TSX support, enabling either MDS
> or TAA mitigations will enable the use of VERW to flush internal
> processor buffers at the right code path. IOW, they are either both
> mitigated or both not mitigated. However, if the command line options
> are inconsistent, the vulnerabilites sysfs files may not report the
> mitigation status correctly.
>
> For example, with only the "mds=off" option:
>
> vulnerabilities/mds:Vulnerable; SMT vulnerable
> vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
>
> The mds vulnerabilities file has wrong status in this case.
>
> Change taa_select_mitigation() to sync up the two mitigation status
> and have them turned off if both "mds=off" and "tsx_async_abort=off"
> are present.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 4c7b0fa15a19..418d41c1fd0d 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
> return;
> }
>
> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
> - if (taa_mitigation == TAA_MITIGATION_OFF)
> + /*
> + * TAA mitigation via VERW is turned off if both
> + * tsx_async_abort=off and mds=off are specified.
> + */
> + if (taa_mitigation == TAA_MITIGATION_OFF &&
> + mds_mitigation == MDS_MITIGATION_OFF)
> goto out;
>
> if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
> @@ -339,6 +343,15 @@ static void __init taa_select_mitigation(void)
> if (taa_nosmt || cpu_mitigations_auto_nosmt())
> cpu_smt_disable(false);
>
> + /*
> + * Update MDS mitigation, if necessary, as the mds_user_clear is
> + * now enabled for TAA mitigation.
> + */
> + if (mds_mitigation == MDS_MITIGATION_OFF &&
> + boot_cpu_has_bug(X86_BUG_MDS)) {
> + mds_mitigation = MDS_MITIGATION_FULL;
> + mds_select_mitigation();
This will cause a confusing print in dmesg from previous and this call
to mds_select_mitigation().
"MDS: Vulnerable"
"MDS: Mitigation: Clear CPU buffers"
Maybe delay MDS mitigation print till TAA is evaluated.
Thanks,
Pawan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 20:12 ` Pawan Gupta
@ 2019-11-14 20:35 ` Josh Poimboeuf
2019-11-14 21:48 ` Waiman Long
2019-11-14 21:58 ` Waiman Long
1 sibling, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2019-11-14 20:35 UTC (permalink / raw)
To: Pawan Gupta
Cc: Waiman Long, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, x86, linux-kernel, Greg Kroah-Hartman,
Mark Gross, Tony Luck
On Thu, Nov 14, 2019 at 12:12:58PM -0800, Pawan Gupta wrote:
> On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
> > For MDS vulnerable processors with TSX support, enabling either MDS
> > or TAA mitigations will enable the use of VERW to flush internal
> > processor buffers at the right code path. IOW, they are either both
> > mitigated or both not mitigated. However, if the command line options
> > are inconsistent, the vulnerabilites sysfs files may not report the
> > mitigation status correctly.
> >
> > For example, with only the "mds=off" option:
> >
> > vulnerabilities/mds:Vulnerable; SMT vulnerable
> > vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
> >
> > The mds vulnerabilities file has wrong status in this case.
> >
> > Change taa_select_mitigation() to sync up the two mitigation status
> > and have them turned off if both "mds=off" and "tsx_async_abort=off"
> > are present.
> >
> > Signed-off-by: Waiman Long <longman@redhat.com>
> > ---
> > arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
> > 1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 4c7b0fa15a19..418d41c1fd0d 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
> > return;
> > }
> >
> > - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
> > - if (taa_mitigation == TAA_MITIGATION_OFF)
> > + /*
> > + * TAA mitigation via VERW is turned off if both
> > + * tsx_async_abort=off and mds=off are specified.
> > + */
> > + if (taa_mitigation == TAA_MITIGATION_OFF &&
> > + mds_mitigation == MDS_MITIGATION_OFF)
> > goto out;
> >
> > if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
> > @@ -339,6 +343,15 @@ static void __init taa_select_mitigation(void)
> > if (taa_nosmt || cpu_mitigations_auto_nosmt())
> > cpu_smt_disable(false);
> >
> > + /*
> > + * Update MDS mitigation, if necessary, as the mds_user_clear is
> > + * now enabled for TAA mitigation.
> > + */
> > + if (mds_mitigation == MDS_MITIGATION_OFF &&
> > + boot_cpu_has_bug(X86_BUG_MDS)) {
> > + mds_mitigation = MDS_MITIGATION_FULL;
> > + mds_select_mitigation();
>
> This will cause a confusing print in dmesg from previous and this call
> to mds_select_mitigation().
>
> "MDS: Vulnerable"
> "MDS: Mitigation: Clear CPU buffers"
>
> Maybe delay MDS mitigation print till TAA is evaluated.
Since they're so intertwined it might make sense to just combine the two
mitigations into a single function.
--
Josh
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 20:35 ` Josh Poimboeuf
@ 2019-11-14 21:48 ` Waiman Long
0 siblings, 0 replies; 12+ messages in thread
From: Waiman Long @ 2019-11-14 21:48 UTC (permalink / raw)
To: Josh Poimboeuf, Pawan Gupta
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86, linux-kernel, Greg Kroah-Hartman, Mark Gross, Tony Luck
On 11/14/19 3:35 PM, Josh Poimboeuf wrote:
> On Thu, Nov 14, 2019 at 12:12:58PM -0800, Pawan Gupta wrote:
>> On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
>>> For MDS vulnerable processors with TSX support, enabling either MDS
>>> or TAA mitigations will enable the use of VERW to flush internal
>>> processor buffers at the right code path. IOW, they are either both
>>> mitigated or both not mitigated. However, if the command line options
>>> are inconsistent, the vulnerabilites sysfs files may not report the
>>> mitigation status correctly.
>>>
>>> For example, with only the "mds=off" option:
>>>
>>> vulnerabilities/mds:Vulnerable; SMT vulnerable
>>> vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
>>>
>>> The mds vulnerabilities file has wrong status in this case.
>>>
>>> Change taa_select_mitigation() to sync up the two mitigation status
>>> and have them turned off if both "mds=off" and "tsx_async_abort=off"
>>> are present.
>>>
>>> Signed-off-by: Waiman Long <longman@redhat.com>
>>> ---
>>> arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
>>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>>> index 4c7b0fa15a19..418d41c1fd0d 100644
>>> --- a/arch/x86/kernel/cpu/bugs.c
>>> +++ b/arch/x86/kernel/cpu/bugs.c
>>> @@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
>>> return;
>>> }
>>>
>>> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
>>> - if (taa_mitigation == TAA_MITIGATION_OFF)
>>> + /*
>>> + * TAA mitigation via VERW is turned off if both
>>> + * tsx_async_abort=off and mds=off are specified.
>>> + */
>>> + if (taa_mitigation == TAA_MITIGATION_OFF &&
>>> + mds_mitigation == MDS_MITIGATION_OFF)
>>> goto out;
>>>
>>> if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
>>> @@ -339,6 +343,15 @@ static void __init taa_select_mitigation(void)
>>> if (taa_nosmt || cpu_mitigations_auto_nosmt())
>>> cpu_smt_disable(false);
>>>
>>> + /*
>>> + * Update MDS mitigation, if necessary, as the mds_user_clear is
>>> + * now enabled for TAA mitigation.
>>> + */
>>> + if (mds_mitigation == MDS_MITIGATION_OFF &&
>>> + boot_cpu_has_bug(X86_BUG_MDS)) {
>>> + mds_mitigation = MDS_MITIGATION_FULL;
>>> + mds_select_mitigation();
>> This will cause a confusing print in dmesg from previous and this call
>> to mds_select_mitigation().
>>
>> "MDS: Vulnerable"
>> "MDS: Mitigation: Clear CPU buffers"
>>
>> Maybe delay MDS mitigation print till TAA is evaluated.
> Since they're so intertwined it might make sense to just combine the two
> mitigations into a single function.
>
They are intertwined mainly for non-MDS_NO processors with TSX. The
mds_select_mitigation() function is pretty simple. Merging the two
together will make the MDS part harder to read. Also the pr_fmt() macro
has to be different for MDS and TAA.
Cheers,
Longman
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 20:12 ` Pawan Gupta
2019-11-14 20:35 ` Josh Poimboeuf
@ 2019-11-14 21:58 ` Waiman Long
2019-11-14 22:09 ` Borislav Petkov
2019-11-14 23:03 ` Thomas Gleixner
1 sibling, 2 replies; 12+ messages in thread
From: Waiman Long @ 2019-11-14 21:58 UTC (permalink / raw)
To: Pawan Gupta
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86, linux-kernel, Josh Poimboeuf, Greg Kroah-Hartman,
Mark Gross, Tony Luck
On 11/14/19 3:12 PM, Pawan Gupta wrote:
> On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
>> For MDS vulnerable processors with TSX support, enabling either MDS
>> or TAA mitigations will enable the use of VERW to flush internal
>> processor buffers at the right code path. IOW, they are either both
>> mitigated or both not mitigated. However, if the command line options
>> are inconsistent, the vulnerabilites sysfs files may not report the
>> mitigation status correctly.
>>
>> For example, with only the "mds=off" option:
>>
>> vulnerabilities/mds:Vulnerable; SMT vulnerable
>> vulnerabilities/tsx_async_abort:Mitigation: Clear CPU buffers; SMT vulnerable
>>
>> The mds vulnerabilities file has wrong status in this case.
>>
>> Change taa_select_mitigation() to sync up the two mitigation status
>> and have them turned off if both "mds=off" and "tsx_async_abort=off"
>> are present.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>> arch/x86/kernel/cpu/bugs.c | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>> index 4c7b0fa15a19..418d41c1fd0d 100644
>> --- a/arch/x86/kernel/cpu/bugs.c
>> +++ b/arch/x86/kernel/cpu/bugs.c
>> @@ -304,8 +304,12 @@ static void __init taa_select_mitigation(void)
>> return;
>> }
>>
>> - /* TAA mitigation is turned off on the cmdline (tsx_async_abort=off) */
>> - if (taa_mitigation == TAA_MITIGATION_OFF)
>> + /*
>> + * TAA mitigation via VERW is turned off if both
>> + * tsx_async_abort=off and mds=off are specified.
>> + */
>> + if (taa_mitigation == TAA_MITIGATION_OFF &&
>> + mds_mitigation == MDS_MITIGATION_OFF)
>> goto out;
>>
>> if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
>> @@ -339,6 +343,15 @@ static void __init taa_select_mitigation(void)
>> if (taa_nosmt || cpu_mitigations_auto_nosmt())
>> cpu_smt_disable(false);
>>
>> + /*
>> + * Update MDS mitigation, if necessary, as the mds_user_clear is
>> + * now enabled for TAA mitigation.
>> + */
>> + if (mds_mitigation == MDS_MITIGATION_OFF &&
>> + boot_cpu_has_bug(X86_BUG_MDS)) {
>> + mds_mitigation = MDS_MITIGATION_FULL;
>> + mds_select_mitigation();
> This will cause a confusing print in dmesg from previous and this call
> to mds_select_mitigation().
>
> "MDS: Vulnerable"
> "MDS: Mitigation: Clear CPU buffers"
Yes, that is the side effect of this patch. It is the last message that
is relevant. We saw this kind of messages all the time with early
loading of microcode. A message showing a hardware vulnerability as
vulnerable and then another message showing it as mitigated after the
loading of microcode.
>
> Maybe delay MDS mitigation print till TAA is evaluated.
I will see what can be done about that. However, this is not a critical
issue and I may not change it if there is no easy solution.
Cheers,
Longman
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 21:58 ` Waiman Long
@ 2019-11-14 22:09 ` Borislav Petkov
2019-11-15 14:40 ` Waiman Long
2019-11-14 23:03 ` Thomas Gleixner
1 sibling, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2019-11-14 22:09 UTC (permalink / raw)
To: Waiman Long
Cc: Pawan Gupta, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-kernel, Josh Poimboeuf, Greg Kroah-Hartman, Mark Gross,
Tony Luck
On Thu, Nov 14, 2019 at 04:58:24PM -0500, Waiman Long wrote:
> > Maybe delay MDS mitigation print till TAA is evaluated.
>
> I will see what can be done about that. However, this is not a critical
> issue and I may not change it if there is no easy solution.
Swapping the two mitigation selection calls in check_bugs() might
work...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 21:58 ` Waiman Long
2019-11-14 22:09 ` Borislav Petkov
@ 2019-11-14 23:03 ` Thomas Gleixner
1 sibling, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2019-11-14 23:03 UTC (permalink / raw)
To: Waiman Long
Cc: Pawan Gupta, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86,
linux-kernel, Josh Poimboeuf, Greg Kroah-Hartman, Mark Gross,
Tony Luck
On Thu, 14 Nov 2019, Waiman Long wrote:
> On 11/14/19 3:12 PM, Pawan Gupta wrote:
> > On Wed, Nov 13, 2019 at 02:33:50PM -0500, Waiman Long wrote:
Folks, please trim your replies ....
> >> + /*
> >> + * Update MDS mitigation, if necessary, as the mds_user_clear is
> >> + * now enabled for TAA mitigation.
> >> + */
> >> + if (mds_mitigation == MDS_MITIGATION_OFF &&
> >> + boot_cpu_has_bug(X86_BUG_MDS)) {
> >> + mds_mitigation = MDS_MITIGATION_FULL;
> >> + mds_select_mitigation();
> > This will cause a confusing print in dmesg from previous and this call
> > to mds_select_mitigation().
> >
> > "MDS: Vulnerable"
> > "MDS: Mitigation: Clear CPU buffers"
>
> Yes, that is the side effect of this patch. It is the last message that
> is relevant. We saw this kind of messages all the time with early
> loading of microcode. A message showing a hardware vulnerability as
> vulnerable and then another message showing it as mitigated after the
> loading of microcode.
> >
> > Maybe delay MDS mitigation print till TAA is evaluated.
>
> I will see what can be done about that. However, this is not a critical
> issue and I may not change it if there is no easy solution.
Right. There is nothing wrong with these two messages coming after each
other. They are both correct and due to the ordering they also make sense.
> > "MDS: Vulnerable"
> > "MDS: Mitigation: Clear CPU buffers"
CPU is vulnerable and then the next printk tells that mitigation is in
effect. So really nothing to worry about.
The important part is that the ordering of these messages is correct which
is the case and that the sysfs file corresponds with the last printk.
We really have more urgent problems than bikeshed painting these printks.
Thanks,
tglx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status
2019-11-14 22:09 ` Borislav Petkov
@ 2019-11-15 14:40 ` Waiman Long
0 siblings, 0 replies; 12+ messages in thread
From: Waiman Long @ 2019-11-15 14:40 UTC (permalink / raw)
To: Borislav Petkov
Cc: Pawan Gupta, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-kernel, Josh Poimboeuf, Greg Kroah-Hartman, Mark Gross,
Tony Luck
On 11/14/19 5:09 PM, Borislav Petkov wrote:
> On Thu, Nov 14, 2019 at 04:58:24PM -0500, Waiman Long wrote:
>>> Maybe delay MDS mitigation print till TAA is evaluated.
>> I will see what can be done about that. However, this is not a critical
>> issue and I may not change it if there is no easy solution.
> Swapping the two mitigation selection calls in check_bugs() might
> work...
Not really. If taa_select_mitigation() goes first and with
"tsx_async_abort=off" only, the mds file is correct, but the taa file
will be wrong.
Cheers,
Longman
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-15 14:41 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-13 19:33 [PATCH] x86/speculation: Fix incorrect MDS/TAA mitigation status Waiman Long
2019-11-14 17:45 ` Borislav Petkov
2019-11-14 17:53 ` Waiman Long
2019-11-14 18:48 ` Thomas Gleixner
2019-11-14 19:11 ` Waiman Long
2019-11-14 20:12 ` Pawan Gupta
2019-11-14 20:35 ` Josh Poimboeuf
2019-11-14 21:48 ` Waiman Long
2019-11-14 21:58 ` Waiman Long
2019-11-14 22:09 ` Borislav Petkov
2019-11-15 14:40 ` Waiman Long
2019-11-14 23:03 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®