| Message ID | 20240813171356.46760-3-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen: arm: Split MMU code in preparation for MPU work (part 2) | expand |
On 13.08.2024 19:13, Ayan Kumar Halder wrote: > From: Penny Zheng <penny.zheng@arm.com> > > Introduced CONFIG_VMAP which is selected by the architectures that use > MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. > > VMAP is widely used in ALTERNATIVE feature to remap a range of memory > with new memory attributes. Since this is highly dependent on virtual > address translation, we choose to fold VMAP in MMU system. > > In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it > only support in MMU system on ARM architecture. And ALTERNATIVE now > depends on VMAP. > > HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative > attacks are not possible on non MMU based systems (ie Cortex-R52, R82). > See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. While I'm not an Arm expert and hence I'm likely missing aspects, I question the one (Spectre-BHB) vulnerability there to be sufficient to draw a conclusion towards the usefulness of branch hardening. I would advise against encoding such a connection in the Kconfig dependencies. > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -1,6 +1,7 @@ > config X86_64 > def_bool y > select 64BIT > + select HAS_VMAP > > config X86 > def_bool y > @@ -31,6 +32,7 @@ config X86 > select HAS_UBSAN > select HAS_VPCI if HVM > select NEEDS_LIBELF > + select HAS_VMAP Why in two places? Also please respect alphabetic sorting here (if this hunk is kept, which may be the more consistent approach) ... > --- a/xen/common/Kconfig > +++ b/xen/common/Kconfig > @@ -77,6 +77,9 @@ config HAS_PIRQ > config HAS_PMAP > bool > > +config HAS_VMAP > + bool > + > config HAS_SCHED_GRANULARITY > bool ... and here. Jan
Hi Jan, On 14/08/2024 07:37, Jan Beulich wrote: > On 13.08.2024 19:13, Ayan Kumar Halder wrote: >> From: Penny Zheng <penny.zheng@arm.com> >> >> Introduced CONFIG_VMAP which is selected by the architectures that use >> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >> >> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >> with new memory attributes. Since this is highly dependent on virtual >> address translation, we choose to fold VMAP in MMU system. >> >> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >> only support in MMU system on ARM architecture. And ALTERNATIVE now >> depends on VMAP. >> >> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >> See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. > While I'm not an Arm expert and hence I'm likely missing aspects, I question > the one (Spectre-BHB) vulnerability there to be sufficient to draw a > conclusion towards the usefulness of branch hardening. I would advise > against encoding such a connection in the Kconfig dependencies. AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was added. See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk-PC.armlinux.org.uk/ And from https://lists.linaro.org/archives/list/linux-stable-mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ Spectre is valid on MMU based systems. Thus, I would make 'branch hardenining' valid on MMU based systems only. Let me know your thoughts. > >> --- a/xen/arch/x86/Kconfig >> +++ b/xen/arch/x86/Kconfig >> @@ -1,6 +1,7 @@ >> config X86_64 >> def_bool y >> select 64BIT >> + select HAS_VMAP >> >> config X86 >> def_bool y >> @@ -31,6 +32,7 @@ config X86 >> select HAS_UBSAN >> select HAS_VPCI if HVM >> select NEEDS_LIBELF >> + select HAS_VMAP > Why in two places? Also please respect alphabetic sorting here (if this > hunk is kept, which may be the more consistent approach) ... My mistake. I initially thought user could select one of the two. However, "vm_init()" is invoked only from xen/arch/x86/setup.c. Thus, keeping 'HAS_VMAP' under 'config X86' is dufficient. I agree that HAS_VMAP needs to go before HAS_VPCI. > >> --- a/xen/common/Kconfig >> +++ b/xen/common/Kconfig >> @@ -77,6 +77,9 @@ config HAS_PIRQ >> config HAS_PMAP >> bool >> >> +config HAS_VMAP >> + bool >> + >> config HAS_SCHED_GRANULARITY >> bool > ... and here. Yes, it needs to go after HAS_UBSAN. - Ayan > > Jan
On 14.08.2024 12:55, Ayan Kumar Halder wrote: > Hi Jan, > > On 14/08/2024 07:37, Jan Beulich wrote: >> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>> From: Penny Zheng <penny.zheng@arm.com> >>> >>> Introduced CONFIG_VMAP which is selected by the architectures that use >>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>> >>> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >>> with new memory attributes. Since this is highly dependent on virtual >>> address translation, we choose to fold VMAP in MMU system. >>> >>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>> depends on VMAP. >>> >>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >>> See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. >> While I'm not an Arm expert and hence I'm likely missing aspects, I question >> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >> conclusion towards the usefulness of branch hardening. I would advise >> against encoding such a connection in the Kconfig dependencies. > > AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was > added. > > See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk-PC.armlinux.org.uk/ > > And from > https://lists.linaro.org/archives/list/linux-stable-mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ > > Spectre is valid on MMU based systems. Since then various other issues / flavors were found. I've been focusing on the x86 side of things, but I'd be very surprised if some didn't affect other architectures as well. Plus branch hardening can be a pre- cautionary measure, too, I think. Jan
Hi Jan, On 14/08/2024 12:35, Jan Beulich wrote: > On 14.08.2024 12:55, Ayan Kumar Halder wrote: >> Hi Jan, >> >> On 14/08/2024 07:37, Jan Beulich wrote: >>> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>>> From: Penny Zheng <penny.zheng@arm.com> >>>> >>>> Introduced CONFIG_VMAP which is selected by the architectures that use >>>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>>> >>>> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >>>> with new memory attributes. Since this is highly dependent on virtual >>>> address translation, we choose to fold VMAP in MMU system. >>>> >>>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>>> depends on VMAP. >>>> >>>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>>> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >>>> See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. >>> While I'm not an Arm expert and hence I'm likely missing aspects, I question >>> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >>> conclusion towards the usefulness of branch hardening. I would advise >>> against encoding such a connection in the Kconfig dependencies. >> AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was >> added. >> >> See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk-PC.armlinux.org.uk/ >> >> And from >> https://lists.linaro.org/archives/list/linux-stable-mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ >> >> Spectre is valid on MMU based systems. > Since then various other issues / flavors were found. I've been focusing > on the x86 side of things, but I'd be very surprised if some didn't > affect other architectures as well. We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is specific to Arm. https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability covers all the flavours and it does not include Cortex-R82 or R52. It says the following :- "Cortex-R cores typically use a closed software stack. In those environments, applications or processes are strictly controlled, and therefore not exploitable" > Plus branch hardening can be a pre- > cautionary measure, too, I think. The first two Arm non MMU cores that we wish to support in the forthcoming series is Cortex-R82 and R52. As seen in https://developer.arm.com/documentation/ka005109/latest/, it explicitly states the following about R82 The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not vulnerable. The Cortex-R82 behaves differently than vulnerable A-class CPUs when speculatively executing past an instruction that authenticates PAC, and that behavior does not allow the attack software to create the "oracle". We can re-enable branch hardening if we know there is a valid non MMU Arm core which is vulnerable. Let me know if you are ok with the rationale. - Ayan > > Jan
On 14.08.2024 14:33, Ayan Kumar Halder wrote: > On 14/08/2024 12:35, Jan Beulich wrote: >> On 14.08.2024 12:55, Ayan Kumar Halder wrote: >>> On 14/08/2024 07:37, Jan Beulich wrote: >>>> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>>>> From: Penny Zheng <penny.zheng@arm.com> >>>>> >>>>> Introduced CONFIG_VMAP which is selected by the architectures that use >>>>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>>>> >>>>> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >>>>> with new memory attributes. Since this is highly dependent on virtual >>>>> address translation, we choose to fold VMAP in MMU system. >>>>> >>>>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>>>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>>>> depends on VMAP. >>>>> >>>>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>>>> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >>>>> See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. >>>> While I'm not an Arm expert and hence I'm likely missing aspects, I question >>>> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >>>> conclusion towards the usefulness of branch hardening. I would advise >>>> against encoding such a connection in the Kconfig dependencies. >>> AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was >>> added. >>> >>> See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk-PC.armlinux.org.uk/ >>> >>> And from >>> https://lists.linaro.org/archives/list/linux-stable-mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ >>> >>> Spectre is valid on MMU based systems. >> Since then various other issues / flavors were found. I've been focusing >> on the x86 side of things, but I'd be very surprised if some didn't >> affect other architectures as well. > > We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is specific > to Arm. > > https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability > covers all the flavours and it does not include Cortex-R82 or R52. > > It says the following :- > > "Cortex-R cores typically use a closed software stack. In those > environments, applications or processes are strictly controlled, and > therefore not exploitable" > >> Plus branch hardening can be a pre- >> cautionary measure, too, I think. > > The first two Arm non MMU cores that we wish to support in the > forthcoming series is Cortex-R82 and R52. > > As seen in https://developer.arm.com/documentation/ka005109/latest/, it > explicitly states the following about R82 > > The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not > vulnerable. The Cortex-R82 behaves differently than vulnerable A-class > CPUs when speculatively executing past an instruction that authenticates > PAC, and that behavior does not allow the attack software to create the > "oracle". > > We can re-enable branch hardening if we know there is a valid non MMU > Arm core which is vulnerable. > > Let me know if you are ok with the rationale. I'm okay with any rationale. As indicated, I'm likely missing (large) parts of the picture here. The connection between the two Kconfig settings merely seemed questionable to me, hence the comment. Jan
Hi Ayan, On 14/08/2024 14:33, Ayan Kumar Halder wrote: > Hi Jan, > > On 14/08/2024 12:35, Jan Beulich wrote: >> On 14.08.2024 12:55, Ayan Kumar Halder wrote: >>> Hi Jan, >>> >>> On 14/08/2024 07:37, Jan Beulich wrote: >>>> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>>>> From: Penny Zheng <penny.zheng@arm.com> >>>>> >>>>> Introduced CONFIG_VMAP which is selected by the architectures that use >>>>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>>>> >>>>> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >>>>> with new memory attributes. Since this is highly dependent on virtual >>>>> address translation, we choose to fold VMAP in MMU system. >>>>> >>>>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>>>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>>>> depends on VMAP. >>>>> >>>>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>>>> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >>>>> See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. >>>> While I'm not an Arm expert and hence I'm likely missing aspects, I question >>>> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >>>> conclusion towards the usefulness of branch hardening. I would advise >>>> against encoding such a connection in the Kconfig dependencies. >>> AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was >>> added. >>> >>> See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk-PC.armlinux.org.uk/ >>> >>> And from >>> https://lists.linaro.org/archives/list/linux-stable-mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ >>> >>> Spectre is valid on MMU based systems. >> Since then various other issues / flavors were found. I've been focusing >> on the x86 side of things, but I'd be very surprised if some didn't >> affect other architectures as well. > > We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is specific > to Arm. > > https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability > covers all the flavours and it does not include Cortex-R82 or R52. > > It says the following :- > > "Cortex-R cores typically use a closed software stack. In those > environments, applications or processes are strictly controlled, and > therefore not exploitable" > >> Plus branch hardening can be a pre- >> cautionary measure, too, I think. > > The first two Arm non MMU cores that we wish to support in the > forthcoming series is Cortex-R82 and R52. > > As seen in https://developer.arm.com/documentation/ka005109/latest/, it > explicitly states the following about R82 > > The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not > vulnerable. The Cortex-R82 behaves differently than vulnerable A-class > CPUs when speculatively executing past an instruction that authenticates > PAC, and that behavior does not allow the attack software to create the > "oracle". > > We can re-enable branch hardening if we know there is a valid non MMU > Arm core which is vulnerable. > > Let me know if you are ok with the rationale. I'm ok with your rationale. I have one question for this patch. Why can't we use CONFIG_HAS_VMAP to conditionally compile vmap.c, like: obj-$(CONFIG_HAS_VMAP) += vmap.o and get rid of VMAP_VIRT_START guard on an entire file? With this config in place, it seems strange to use VMAP_VIRT_START as a guard. ~Michal
On 16/08/2024 10:28, Michal Orzel wrote: > Hi Ayan, Hi Michal, > > On 14/08/2024 14:33, Ayan Kumar Halder wrote: >> Hi Jan, >> >> On 14/08/2024 12:35, Jan Beulich wrote: >>> On 14.08.2024 12:55, Ayan Kumar Halder wrote: >>>> Hi Jan, >>>> >>>> On 14/08/2024 07:37, Jan Beulich wrote: >>>>> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>>>>> From: Penny Zheng <penny.zheng@arm.com> >>>>>> >>>>>> Introduced CONFIG_VMAP which is selected by the architectures that use >>>>>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>>>>> >>>>>> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >>>>>> with new memory attributes. Since this is highly dependent on virtual >>>>>> address translation, we choose to fold VMAP in MMU system. >>>>>> >>>>>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>>>>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>>>>> depends on VMAP. >>>>>> >>>>>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>>>>> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >>>>>> See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. >>>>> While I'm not an Arm expert and hence I'm likely missing aspects, I question >>>>> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >>>>> conclusion towards the usefulness of branch hardening. I would advise >>>>> against encoding such a connection in the Kconfig dependencies. >>>> AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was >>>> added. >>>> >>>> See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk-PC.armlinux.org.uk/ >>>> >>>> And from >>>> https://lists.linaro.org/archives/list/linux-stable-mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ >>>> >>>> Spectre is valid on MMU based systems. >>> Since then various other issues / flavors were found. I've been focusing >>> on the x86 side of things, but I'd be very surprised if some didn't >>> affect other architectures as well. >> We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is specific >> to Arm. >> >> https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability >> covers all the flavours and it does not include Cortex-R82 or R52. >> >> It says the following :- >> >> "Cortex-R cores typically use a closed software stack. In those >> environments, applications or processes are strictly controlled, and >> therefore not exploitable" >> >>> Plus branch hardening can be a pre- >>> cautionary measure, too, I think. >> The first two Arm non MMU cores that we wish to support in the >> forthcoming series is Cortex-R82 and R52. >> >> As seen in https://developer.arm.com/documentation/ka005109/latest/, it >> explicitly states the following about R82 >> >> The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not >> vulnerable. The Cortex-R82 behaves differently than vulnerable A-class >> CPUs when speculatively executing past an instruction that authenticates >> PAC, and that behavior does not allow the attack software to create the >> "oracle". >> >> We can re-enable branch hardening if we know there is a valid non MMU >> Arm core which is vulnerable. >> >> Let me know if you are ok with the rationale. > I'm ok with your rationale. > > I have one question for this patch. Why can't we use CONFIG_HAS_VMAP to conditionally compile vmap.c, like: > obj-$(CONFIG_HAS_VMAP) += vmap.o > and get rid of VMAP_VIRT_START guard on an entire file? > With this config in place, it seems strange to use VMAP_VIRT_START as a guard. I am fine with your suggestion. However, this implies HAS_VMAP needs to be selected by "config PPC" as well (as it defines VMAP_VIRT_START in xen/arch/ppc/include/asm/config.h ). This is fine as I could see PPC uses MMU as well. - Ayan
Hi Ayan, On 14/08/2024 13:33, Ayan Kumar Halder wrote: > Hi Jan, > > On 14/08/2024 12:35, Jan Beulich wrote: >> On 14.08.2024 12:55, Ayan Kumar Halder wrote: >>> Hi Jan, >>> >>> On 14/08/2024 07:37, Jan Beulich wrote: >>>> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>>>> From: Penny Zheng <penny.zheng@arm.com> >>>>> >>>>> Introduced CONFIG_VMAP which is selected by the architectures that use >>>>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>>>> >>>>> VMAP is widely used in ALTERNATIVE feature to remap a range of memory >>>>> with new memory attributes. Since this is highly dependent on virtual >>>>> address translation, we choose to fold VMAP in MMU system. >>>>> >>>>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>>>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>>>> depends on VMAP. >>>>> >>>>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>>>> attacks are not possible on non MMU based systems (ie Cortex-R52, >>>>> R82). >>>>> See https://developer.arm.com/Arm%20Security%20Center/ >>>>> Speculative%20Processor%20Vulnerability. >>>> While I'm not an Arm expert and hence I'm likely missing aspects, I >>>> question >>>> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >>>> conclusion towards the usefulness of branch hardening. I would advise >>>> against encoding such a connection in the Kconfig dependencies. >>> AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was >>> added. >>> >>> See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk- >>> PC.armlinux.org.uk/ >>> >>> And from >>> https://lists.linaro.org/archives/list/linux-stable- >>> mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ >>> >>> Spectre is valid on MMU based systems. >> Since then various other issues / flavors were found. I've been focusing >> on the x86 side of things, but I'd be very surprised if some didn't >> affect other architectures as well. > > We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is specific > to Arm. > > https://developer.arm.com/Arm%20Security%20Center/ > Speculative%20Processor%20Vulnerability covers all the flavours and it > does not include Cortex-R82 or R52. > > It says the following :- > > "Cortex-R cores typically use a closed software stack. In those > environments, applications or processes are strictly controlled, and > therefore not exploitable" > >> Plus branch hardening can be a pre- >> cautionary measure, too, I think. > > The first two Arm non MMU cores that we wish to support in the > forthcoming series is Cortex-R82 and R52. > > As seen in https://developer.arm.com/documentation/ka005109/latest/, it > explicitly states the following about R82 > > The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not > vulnerable. The Cortex-R82 behaves differently than vulnerable A-class > CPUs when speculatively executing past an instruction that authenticates > PAC, and that behavior does not allow the attack software to create the > "oracle". I am confused. This is describing why R82 is not affected by PACMAN. But the Kconfig HARDEN_BRANCH_PREDICTOR is not for that (Xen doesn't yet use Pointer Authentification Codes). The Kconfig was introduced with XSA-254 which predates PACMAN by nearly 4 years. > > We can re-enable branch hardening if we know there is a valid non MMU > Arm core which is vulnerable. Re-quoting what you wrote earlier: "Cortex-R cores typically use a closed software stack. In those environments, applications or processes are strictly controlled, and therefore not exploitable" It is quite subtle. This wording doesn't imply the cores are not vulnerable. It says that if they are, then it would be difficult to exploit because the software should be tightly controlled. Now this would be really up to the user to decide whether they want to be extra cautious/futureproof or not. As we are at the beginning of the MPU support, then I don't think we need to resolve issue right now. And it would be fine to gate it. But maybe ARCH_VMAP was an incorrect suggestion. It might be better to gate with the !MMU (IIRC this would imply MPU). But before the feature can be marked as security support. We will need to agree on how the hypervisor is intended to be used on ARMv8-R. Maybe it would need a caveat "only trusted software can be run" which means we don't have to worry about speculation on Cortex-R. Although, it would be nice to have some defense in-depth :). Cheers,
On 16/08/2024 17:40, Julien Grall wrote: > Hi Ayan, Hi Julien, > > On 14/08/2024 13:33, Ayan Kumar Halder wrote: >> Hi Jan, >> >> On 14/08/2024 12:35, Jan Beulich wrote: >>> On 14.08.2024 12:55, Ayan Kumar Halder wrote: >>>> Hi Jan, >>>> >>>> On 14/08/2024 07:37, Jan Beulich wrote: >>>>> On 13.08.2024 19:13, Ayan Kumar Halder wrote: >>>>>> From: Penny Zheng <penny.zheng@arm.com> >>>>>> >>>>>> Introduced CONFIG_VMAP which is selected by the architectures >>>>>> that use >>>>>> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >>>>>> >>>>>> VMAP is widely used in ALTERNATIVE feature to remap a range of >>>>>> memory >>>>>> with new memory attributes. Since this is highly dependent on >>>>>> virtual >>>>>> address translation, we choose to fold VMAP in MMU system. >>>>>> >>>>>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and >>>>>> make it >>>>>> only support in MMU system on ARM architecture. And ALTERNATIVE now >>>>>> depends on VMAP. >>>>>> >>>>>> HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative >>>>>> attacks are not possible on non MMU based systems (ie Cortex-R52, >>>>>> R82). >>>>>> See https://developer.arm.com/Arm%20Security%20Center/ >>>>>> Speculative%20Processor%20Vulnerability. >>>>> While I'm not an Arm expert and hence I'm likely missing aspects, >>>>> I question >>>>> the one (Spectre-BHB) vulnerability there to be sufficient to draw a >>>>> conclusion towards the usefulness of branch hardening. I would advise >>>>> against encoding such a connection in the Kconfig dependencies. >>>> AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' >>>> was >>>> added. >>>> >>>> See https://lore.kernel.org/all/E1fNadD-0000fz-9r@rmk- >>>> PC.armlinux.org.uk/ >>>> >>>> And from >>>> https://lists.linaro.org/archives/list/linux-stable- >>>> mirror@lists.linaro.org/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/ >>>> >>>> Spectre is valid on MMU based systems. >>> Since then various other issues / flavors were found. I've been >>> focusing >>> on the x86 side of things, but I'd be very surprised if some didn't >>> affect other architectures as well. >> >> We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is >> specific to Arm. >> >> https://developer.arm.com/Arm%20Security%20Center/ >> Speculative%20Processor%20Vulnerability covers all the flavours and >> it does not include Cortex-R82 or R52. >> >> It says the following :- >> >> "Cortex-R cores typically use a closed software stack. In those >> environments, applications or processes are strictly controlled, and >> therefore not exploitable" >> >>> Plus branch hardening can be a pre- >>> cautionary measure, too, I think. >> >> The first two Arm non MMU cores that we wish to support in the >> forthcoming series is Cortex-R82 and R52. >> >> As seen in https://developer.arm.com/documentation/ka005109/latest/, >> it explicitly states the following about R82 >> >> The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not >> vulnerable. The Cortex-R82 behaves differently than vulnerable >> A-class CPUs when speculatively executing past an instruction that >> authenticates PAC, and that behavior does not allow the attack >> software to create the "oracle". > > I am confused. This is describing why R82 is not affected by PACMAN. > But the Kconfig HARDEN_BRANCH_PREDICTOR is not for that (Xen doesn't > yet use Pointer Authentification Codes). The Kconfig was introduced > with XSA-254 which predates PACMAN by nearly 4 years. Yes, you are correct. I was somehow linking PACMAN to branch prediction attacks. So, by mistake I assumed that HARDEN_BRANCH_PREDICTOR is used for protection against these attacks. > >> >> We can re-enable branch hardening if we know there is a valid non MMU >> Arm core which is vulnerable. > > Re-quoting what you wrote earlier: > > "Cortex-R cores typically use a closed software stack. In those > environments, applications or processes are strictly controlled, and > therefore not exploitable" > > It is quite subtle. This wording doesn't imply the cores are not > vulnerable. It says that if they are, then it would be difficult to > exploit because the software should be tightly controlled. > > Now this would be really up to the user to decide whether they want to > be extra cautious/futureproof or not. > > As we are at the beginning of the MPU support, then I don't think we > need to resolve issue right now. And it would be fine to gate it. But > maybe ARCH_VMAP was an incorrect suggestion. It might be better to > gate with the !MMU (IIRC this would imply MPU). I am ok with this. This has the benefit that the change can be contained within arch/arm if we do the following :- diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index cb2c0a16b8..26f7406278 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -329,7 +329,9 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, setup_mm(); +#ifdef CONFIG_MMU vm_init(); +#endif /* Parse the ACPI tables for possible boot-time configuration */ acpi_boot_table_init(); Are we ok with this ? The definition of vm_init() is in xen/include/xen/vmap.h. If I enclose it using any CONFIG_XXX (like I have done in the current patch), then I need to introduce it in common/Kconfig and define it for x86 and PPC. I would prefer to contain the change within arch/arm only if possible. > > But before the feature can be marked as security support. We will need > to agree on how the hypervisor is intended to be used on ARMv8-R. > Maybe it would need a caveat "only trusted software can be run" which > means we don't have to worry about speculation on Cortex-R. Although, > it would be nice to have some defense in-depth :). Yes, the support for ARMv8-R will be 'experimental' for some time. I am planning to add the minimal support required to just boot one or more DomUs. - Ayan
Hi Ayan, On 19/08/2024 10:45, Ayan Kumar Halder wrote: > I am ok with this. This has the benefit that the change can be contained > within arch/arm if we do the following :- > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index cb2c0a16b8..26f7406278 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -329,7 +329,9 @@ void asmlinkage __init start_xen(unsigned long > boot_phys_offset, > > setup_mm(); > > +#ifdef CONFIG_MMU > vm_init(); > +#endif > > /* Parse the ACPI tables for possible boot-time configuration */ > acpi_boot_table_init(); > > Are we ok with this ? > > The definition of vm_init() is in xen/include/xen/vmap.h. If I enclose > it using any CONFIG_XXX (like I have done in the current patch), then I > need to introduce it in common/Kconfig and define it for x86 and PPC. I > would prefer to contain the change within arch/arm only if possible. Just to clarify, are you suggesting to just protect the call vm_init(). In other word, common/vmap.c would still be included in the final binary for the MPU? If yes, then I think it would be a bit odd... Someone could still call vmap() and this would not break until runtime. So I don't see how we could get away from modifying the common code. Cheers,
On 19/08/2024 10:55, Julien Grall wrote: > Hi Ayan, > > On 19/08/2024 10:45, Ayan Kumar Halder wrote: >> I am ok with this. This has the benefit that the change can be >> contained within arch/arm if we do the following :- >> >> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >> index cb2c0a16b8..26f7406278 100644 >> --- a/xen/arch/arm/setup.c >> +++ b/xen/arch/arm/setup.c >> @@ -329,7 +329,9 @@ void asmlinkage __init start_xen(unsigned long >> boot_phys_offset, >> >> setup_mm(); >> >> +#ifdef CONFIG_MMU >> vm_init(); >> +#endif >> >> /* Parse the ACPI tables for possible boot-time configuration */ >> acpi_boot_table_init(); >> >> Are we ok with this ? >> >> The definition of vm_init() is in xen/include/xen/vmap.h. If I enclose >> it using any CONFIG_XXX (like I have done in the current patch), then >> I need to introduce it in common/Kconfig and define it for x86 and >> PPC. I would prefer to contain the change within arch/arm only if >> possible. > > Just to clarify, are you suggesting to just protect the call vm_init(). > In other word, common/vmap.c would still be included in the final binary > for the MPU? > > If yes, then I think it would be a bit odd... Someone could still call > vmap() and this would not break until runtime. > > So I don't see how we could get away from modifying the common code. Readying my previous reply again. I think the confusion comes from: > But maybe ARCH_VMAP was an incorrect suggestion. It might be better to gate with the !MMU (IIRC this would imply MPU). This was specifically referring to the branch predictor Kconfig. This was not a suggestion to avoid introducing ARCH_VMAP. Cheers,
Guys, On 19.08.2024 11:45, Ayan Kumar Halder wrote: > On 16/08/2024 17:40, Julien Grall wrote: >> On 14/08/2024 13:33, Ayan Kumar Halder wrote: mind me asking why I continue to be on the To: list of this communication between the two of you? Jan
On 19/08/2024 12:39, Jan Beulich wrote: > Guys, > > On 19.08.2024 11:45, Ayan Kumar Halder wrote: >> On 16/08/2024 17:40, Julien Grall wrote: >>> On 14/08/2024 13:33, Ayan Kumar Halder wrote: > > mind me asking why I continue to be on the To: list of this communication > between the two of you? You were involved in the review and AFAICT will still touch the common code. I am happy to remove you from the conversation if you are not interested :). Cheers,
On 19.08.2024 14:12, Julien Grall wrote: > On 19/08/2024 12:39, Jan Beulich wrote: >> Guys, >> >> On 19.08.2024 11:45, Ayan Kumar Halder wrote: >>> On 16/08/2024 17:40, Julien Grall wrote: >>>> On 14/08/2024 13:33, Ayan Kumar Halder wrote: >> >> mind me asking why I continue to be on the To: list of this communication >> between the two of you? > > You were involved in the review and AFAICT will still touch the common > code. I am happy to remove you from the conversation if you are not > interested :). That wasn't my request though. It's fine to keep me Cc-ed. Jan
Hi Jan, On 19/08/2024 13:24, Jan Beulich wrote: > On 19.08.2024 14:12, Julien Grall wrote: >> On 19/08/2024 12:39, Jan Beulich wrote: >>> Guys, >>> >>> On 19.08.2024 11:45, Ayan Kumar Halder wrote: >>>> On 16/08/2024 17:40, Julien Grall wrote: >>>>> On 14/08/2024 13:33, Ayan Kumar Halder wrote: >>> >>> mind me asking why I continue to be on the To: list of this communication >>> between the two of you? >> >> You were involved in the review and AFAICT will still touch the common >> code. I am happy to remove you from the conversation if you are not >> interested :). > > That wasn't my request though. It's fine to keep me Cc-ed. TBH, in this context, the difference between CC and TO is very blur. For me, CC is for people that may be interested but not involved in the conversation. In this case, Ayan was suggesting whether we should use ARCH_VMAP or not. Given you reviewed the rest, I feel you have your say on this decision. Hence why the TO makes sense. But I also understand why people may view it differently... Personally, I don't make any filter on "cc" vs "to" because this is a matter of opinion whether you think someone should be actively involved (or not) in the discussion. So it is best if I look at everything even if this is just having a glancing look at on the content. If you have some clear and unambiguous rules on when you want to be in the CC or TO, then I would be happy to follow them. Cheers,
Hi Julien/Jan, On 19/08/2024 10:58, Julien Grall wrote: > > > On 19/08/2024 10:55, Julien Grall wrote: >> Hi Ayan, >> >> On 19/08/2024 10:45, Ayan Kumar Halder wrote: >>> I am ok with this. This has the benefit that the change can be >>> contained within arch/arm if we do the following :- >>> >>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >>> index cb2c0a16b8..26f7406278 100644 >>> --- a/xen/arch/arm/setup.c >>> +++ b/xen/arch/arm/setup.c >>> @@ -329,7 +329,9 @@ void asmlinkage __init start_xen(unsigned long >>> boot_phys_offset, >>> >>> setup_mm(); >>> >>> +#ifdef CONFIG_MMU >>> vm_init(); >>> +#endif >>> >>> /* Parse the ACPI tables for possible boot-time configuration */ >>> acpi_boot_table_init(); >>> >>> Are we ok with this ? >>> >>> The definition of vm_init() is in xen/include/xen/vmap.h. If I >>> enclose it using any CONFIG_XXX (like I have done in the current >>> patch), then I need to introduce it in common/Kconfig and define it >>> for x86 and PPC. I would prefer to contain the change within >>> arch/arm only if possible. >> >> Just to clarify, are you suggesting to just protect the call >> vm_init(). In other word, common/vmap.c would still be included in >> the final binary for the MPU? >> >> If yes, then I think it would be a bit odd... Someone could still >> call vmap() and this would not break until runtime. >> >> So I don't see how we could get away from modifying the common code. > > Readying my previous reply again. I think the confusion comes from: > > > But maybe ARCH_VMAP was an incorrect suggestion. It might be better > to gate with the !MMU (IIRC this would imply MPU). > > This was specifically referring to the branch predictor Kconfig. This > was not a suggestion to avoid introducing ARCH_VMAP. Thanks for clarifying this. Yes, I misunderstood your previous comment. So I will do :- 1. HARDEN_BRANCH_PREDICTOR will depend on MMU. 2. ARCH_VMAP will be selected by PPC and RISCV. The reason is below. 3. xen/common/vmap.c will be conditionally compiled on ARCH_VMAP and the "#ifdef VMAP_VIRT_START .. endif" will be from removed within the file. As VMAP_VIRT_START is defined by RISCV and PPC, thus #2 is needed. Julien, Jan :- Please let me know if you are ok with #3. This was in response to Michal's comment. While his suggestion makes sense, I am not sure if extending the changes to other architectures is the correct approach. Or do you prefer keeping xen/common/vmap.c unchanged. Kind regards, Ayan > > Cheers, >
On 20.08.2024 13:48, Ayan Kumar Halder wrote: > So I will do :- > > 1. HARDEN_BRANCH_PREDICTOR will depend on MMU. > > 2. ARCH_VMAP will be selected by PPC and RISCV. The reason is below. > > 3. xen/common/vmap.c will be conditionally compiled on ARCH_VMAP and the > "#ifdef VMAP_VIRT_START .. endif" will be from removed within the file. > As VMAP_VIRT_START is defined by RISCV and PPC, thus #2 is needed. > > Julien, Jan :- Please let me know if you are ok with #3. This was in > response to Michal's comment. While his suggestion makes sense, I am not > sure if extending the changes to other architectures is the correct > approach. Or do you prefer keeping xen/common/vmap.c unchanged. No, the VMAP_VIRT_START thing was, from all I recall, merely to cover for the lack of a way to exclude building of the file from Makefile. After all this pre-dates the introduction of kconfig in Xen. And yes, suitably covering PPC and RISC-V is The Right Thing To Do (tm); their maintainers can shout if they disagree. Jan
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 21d03d9f44..e30a7da186 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -12,7 +12,7 @@ config ARM_64 config ARM def_bool y select FUNCTION_ALIGNMENT_4B - select HAS_ALTERNATIVE + select HAS_ALTERNATIVE if HAS_VMAP select HAS_DEVICE_TREE select HAS_PASSTHROUGH select HAS_UBSAN @@ -61,6 +61,7 @@ config PADDR_BITS config MMU def_bool y select HAS_PMAP + select HAS_VMAP source "arch/Kconfig" @@ -171,6 +172,7 @@ config ARM_SSBD config HARDEN_BRANCH_PREDICTOR bool "Harden the branch predictor against aliasing attacks" if EXPERT + depends on HAS_VMAP default y help Speculation attacks against some high-performance processors rely on diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index cb2c0a16b8..7f686d2cca 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -447,7 +447,9 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, * It needs to be called after do_initcalls to be able to use * stop_machine (tasklets initialized via an initcall). */ +#ifdef CONFIG_HAS_ALTERNATIVE apply_alternatives_all(); +#endif enable_errata_workarounds(); enable_cpu_features(); diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 7ef5c8bc48..32be057978 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -1,6 +1,7 @@ config X86_64 def_bool y select 64BIT + select HAS_VMAP config X86 def_bool y @@ -31,6 +32,7 @@ config X86 select HAS_UBSAN select HAS_VPCI if HVM select NEEDS_LIBELF + select HAS_VMAP config ARCH_DEFCONFIG string diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 565ceda741..188918ec5c 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -77,6 +77,9 @@ config HAS_PIRQ config HAS_PMAP bool +config HAS_VMAP + bool + config HAS_SCHED_GRANULARITY bool diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h index fdae37e950..c1dd7ac22f 100644 --- a/xen/include/xen/vmap.h +++ b/xen/include/xen/vmap.h @@ -141,7 +141,9 @@ void *arch_vmap_virt_end(void); /* Initialises the VMAP_DEFAULT virtual range */ static inline void vm_init(void) { +#ifdef CONFIG_HAS_VMAP vm_init_type(VMAP_DEFAULT, (void *)VMAP_VIRT_START, arch_vmap_virt_end()); +#endif } #endif /* __XEN_VMAP_H__ */