On Fri, Sep 03, 2021 at 07:52:43PM -0500, Moger, Babu wrote:
> > BTW, is the list of PSF-affected CPUs the same as the list of
> > SSB-affected CPUs? If there might be PSF CPUs which don't have SSB,
> > then more logic will need to be added to ensure a sensible default.
> I can't think of a scenario where it is not same on a system.
To clarify, I'm asking about CPU capabilities. Are there any AMD CPUs
with the PSF feature, which don't have SSB?
> > On a related note, is there a realistic, non-hypothetical need to have
> > separate policies and cmdline options for both SSB and PSF? i.e. is
> > there a real-world scenario where a user needs to disable PSF while
> > leaving SSB enabled?
>
> https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf <https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf>
> There are some examples in the document. Probably it is too soon to tell if
> those are real real-world scenarios as this feature is very new.
I didn't see any actual examples. Are you referring to this sentence?
"PSFD may be desirable for software which is concerned with the
speculative behavior of PSF but desires a smaller performance impact
than setting SSBD."
> > Because trying to give them separate interfaces, when PSF disable is
> > intertwined with SSB disable in hardware, is awkward and confusing. And
> > the idea of adding another double-negative interface (disable=off!),
> > just because a vulnerability is considered to be a CPU "feature", isn't
> > very appetizing.
> >
> > So instead of adding a new double-negative interface, which only *half*
> > works due to the ssb_disable dependency, and which is guaranteed to
> > further confuse users, and which not even be used in the real world
> > except possibly by confused users...
> >
> > I'm wondering if we can just start out with the simplest possible
> > approach: don't change any code and instead just document the fact that
> > "spec_store_bypass_disable=" also affects PSF.
> >
> > Then, later on, if a real-world need is demonstrated, actual code could
> > be added to support disabling PSF independently (but of course it would
> > never be fully independent since PSF disable is forced by SSB disable).
>
> Do you mean for now keep only 'on' and 'auto' and remove "off"?
No, since PSF can already be mitigated with SSBD today, I'm suggesting
that all code be removed from the patch and instead just update the
documentation.
--
Josh
On 9/4/21 12:23 PM, Josh Poimboeuf wrote:
> On Fri, Sep 03, 2021 at 07:52:43PM -0500, Moger, Babu wrote:
>>> BTW, is the list of PSF-affected CPUs the same as the list of
>>> SSB-affected CPUs? If there might be PSF CPUs which don't have SSB,
>>> then more logic will need to be added to ensure a sensible default.
>> I can't think of a scenario where it is not same on a system.
>
> To clarify, I'm asking about CPU capabilities. Are there any AMD CPUs
> with the PSF feature, which don't have SSB?
No. That combination is not there. It is always SSB + PSF.
>
>>> On a related note, is there a realistic, non-hypothetical need to have
>>> separate policies and cmdline options for both SSB and PSF? i.e. is
>>> there a real-world scenario where a user needs to disable PSF while
>>> leaving SSB enabled?
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2Fdocuments%2Fsecurity-analysis-predictive-store-forwarding.pdf&data=04%7C01%7CBabu.Moger%40amd.com%7Cfcbc2781e8b54ed6ca6b08d96fc8bdf2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637663730522361553%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eI8DisfM4gpj%2B4AOfOUFGOFRZP6zqhSsTJIwINHK5GY%3D&reserved=0 <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2Fdocuments%2Fsecurity-analysis-predictive-store-forwarding.pdf&data=04%7C01%7CBabu.Moger%40amd.com%7Cfcbc2781e8b54ed6ca6b08d96fc8bdf2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637663730522361553%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eI8DisfM4gpj%2B4AOfOUFGOFRZP6zqhSsTJIwINHK5GY%3D&reserved=0>
>> There are some examples in the document. Probably it is too soon to tell if
>> those are real real-world scenarios as this feature is very new.
>
> I didn't see any actual examples. Are you referring to this sentence?
Agree. It is not an actual example. There is also this text about where it
can be useful.
"AMD believes that for most applications, the security risk of PSF is
likely low and where isolation is required, techniques such as address
space isolation are preferred over software sandboxing."
>
> "PSFD may be desirable for software which is concerned with the
> speculative behavior of PSF but desires a smaller performance impact
> than setting SSBD."
>
>>> Because trying to give them separate interfaces, when PSF disable is
>>> intertwined with SSB disable in hardware, is awkward and confusing. And
>>> the idea of adding another double-negative interface (disable=off!),
>>> just because a vulnerability is considered to be a CPU "feature", isn't
>>> very appetizing.
>>>
>>> So instead of adding a new double-negative interface, which only *half*
>>> works due to the ssb_disable dependency, and which is guaranteed to
>>> further confuse users, and which not even be used in the real world
>>> except possibly by confused users...
>>>
>>> I'm wondering if we can just start out with the simplest possible
>>> approach: don't change any code and instead just document the fact that
>>> "spec_store_bypass_disable=" also affects PSF.
>>>
>>> Then, later on, if a real-world need is demonstrated, actual code could
>>> be added to support disabling PSF independently (but of course it would
>>> never be fully independent since PSF disable is forced by SSB disable).
>>
>> Do you mean for now keep only 'on' and 'auto' and remove "off"?
>
> No, since PSF can already be mitigated with SSBD today, I'm suggesting
> that all code be removed from the patch and instead just update the
> documentation.
>
Hmm Interesting..
Just updating the documentation and without giving interface to enable or
disable will not be a much of a value add. In the earlier
discussion, the direction was to go with simple "on" and "off".
https://lore.kernel.org/lkml/202105101508.BC6CC99FAD@keescook/
But, I am not sure right now.
thanks
Babu
On Tue, Sep 07, 2021 at 06:15:53PM -0500, Babu Moger wrote:
> >>> Because trying to give them separate interfaces, when PSF disable is
> >>> intertwined with SSB disable in hardware, is awkward and confusing. And
> >>> the idea of adding another double-negative interface (disable=off!),
> >>> just because a vulnerability is considered to be a CPU "feature", isn't
> >>> very appetizing.
> >>>
> >>> So instead of adding a new double-negative interface, which only *half*
> >>> works due to the ssb_disable dependency, and which is guaranteed to
> >>> further confuse users, and which not even be used in the real world
> >>> except possibly by confused users...
> >>>
> >>> I'm wondering if we can just start out with the simplest possible
> >>> approach: don't change any code and instead just document the fact that
> >>> "spec_store_bypass_disable=" also affects PSF.
> >>>
> >>> Then, later on, if a real-world need is demonstrated, actual code could
> >>> be added to support disabling PSF independently (but of course it would
> >>> never be fully independent since PSF disable is forced by SSB disable).
> >>
> >> Do you mean for now keep only 'on' and 'auto' and remove "off"?
> >
> > No, since PSF can already be mitigated with SSBD today, I'm suggesting
> > that all code be removed from the patch and instead just update the
> > documentation.
> >
>
> Hmm Interesting..
> Just updating the documentation and without giving interface to enable or
> disable will not be a much of a value add.
It's also not a value add to create controls and added complexity for a
feature which nobody needs. There's no harm in starting out with the
simplest possible solution, which is no code at all.
Code can always be added later if really needed...
--
Josh
Josh Poimboeuf <[email protected]> writes:
> On Fri, Sep 03, 2021 at 07:52:43PM -0500, Moger, Babu wrote:
>> > BTW, is the list of PSF-affected CPUs the same as the list of
>> > SSB-affected CPUs? If there might be PSF CPUs which don't have SSB,
>> > then more logic will need to be added to ensure a sensible default.
>> I can't think of a scenario where it is not same on a system.
>
> To clarify, I'm asking about CPU capabilities. Are there any AMD CPUs
> with the PSF feature, which don't have SSB?
>
>> > On a related note, is there a realistic, non-hypothetical need to have
>> > separate policies and cmdline options for both SSB and PSF? i.e. is
>> > there a real-world scenario where a user needs to disable PSF while
>> > leaving SSB enabled?
>>
>> https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf <https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf>
>> There are some examples in the document. Probably it is too soon to tell if
>> those are real real-world scenarios as this feature is very new.
>
> I didn't see any actual examples. Are you referring to this sentence?
>
> "PSFD may be desirable for software which is concerned with the
> speculative behavior of PSF but desires a smaller performance impact
> than setting SSBD."
>
Sounds reasonable. It would have been good if the whitepaper mentioned
any real examples which could benefit from selectively disabling psf.
Generally speaking, as a user, I would either want to turn speculation
entirely off or on which is what ssbd already does.
>> > Because trying to give them separate interfaces, when PSF disable is
>> > intertwined with SSB disable in hardware, is awkward and confusing. And
>> > the idea of adding another double-negative interface (disable=off!),
>> > just because a vulnerability is considered to be a CPU "feature", isn't
>> > very appetizing.
>> >
>> > So instead of adding a new double-negative interface, which only *half*
>> > works due to the ssb_disable dependency, and which is guaranteed to
>> > further confuse users, and which not even be used in the real world
>> > except possibly by confused users...
>> >
>> > I'm wondering if we can just start out with the simplest possible
>> > approach: don't change any code and instead just document the fact that
>> > "spec_store_bypass_disable=" also affects PSF.
>> >
>> > Then, later on, if a real-world need is demonstrated, actual code could
>> > be added to support disabling PSF independently (but of course it would
>> > never be fully independent since PSF disable is forced by SSB disable).
>>
>> Do you mean for now keep only 'on' and 'auto' and remove "off"?
>
> No, since PSF can already be mitigated with SSBD today, I'm suggesting
> that all code be removed from the patch and instead just update the
> documentation.
On 9/8/21 1:20 PM, Josh Poimboeuf wrote:
> On Tue, Sep 07, 2021 at 06:15:53PM -0500, Babu Moger wrote:
>>>>> Because trying to give them separate interfaces, when PSF disable is
>>>>> intertwined with SSB disable in hardware, is awkward and confusing. And
>>>>> the idea of adding another double-negative interface (disable=off!),
>>>>> just because a vulnerability is considered to be a CPU "feature", isn't
>>>>> very appetizing.
>>>>>
>>>>> So instead of adding a new double-negative interface, which only *half*
>>>>> works due to the ssb_disable dependency, and which is guaranteed to
>>>>> further confuse users, and which not even be used in the real world
>>>>> except possibly by confused users...
>>>>>
>>>>> I'm wondering if we can just start out with the simplest possible
>>>>> approach: don't change any code and instead just document the fact that
>>>>> "spec_store_bypass_disable=" also affects PSF.
>>>>>
>>>>> Then, later on, if a real-world need is demonstrated, actual code could
>>>>> be added to support disabling PSF independently (but of course it would
>>>>> never be fully independent since PSF disable is forced by SSB disable).
>>>>
>>>> Do you mean for now keep only 'on' and 'auto' and remove "off"?
>>>
>>> No, since PSF can already be mitigated with SSBD today, I'm suggesting
>>> that all code be removed from the patch and instead just update the
>>> documentation.
>>>
>>
>> Hmm Interesting..
>> Just updating the documentation and without giving interface to enable or
>> disable will not be a much of a value add.
>
> It's also not a value add to create controls and added complexity for a
> feature which nobody needs. There's no harm in starting out with the
> simplest possible solution, which is no code at all.
>
> Code can always be added later if really needed...
>
Alright. Lets revisit this later when it seems reasonable to add this in
the kernel.
For now, I will focus on exposing this feature in KVM where guests can
make use of it. It appears straight forward. Will send those patches soon.
thanks
Babu