[PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation

Julien Grall posted 2 patches 3 years, 1 month ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210313160611.18665-1-julien@xen.org
There is a newer version of this series
xen/arch/arm/Makefile                |  2 +-
xen/arch/arm/arm32/entry.S           |  2 +-
xen/arch/arm/arm32/head.S            |  1 -
xen/arch/arm/arm32/lib/lib1funcs.S   |  1 +
xen/arch/arm/arm32/proc-v7.S         |  1 -
xen/arch/arm/arm64/debug-cadence.inc |  1 -
xen/arch/arm/arm64/debug-pl011.inc   |  2 --
xen/arch/arm/arm64/entry.S           |  2 --
xen/arch/arm/arm64/head.S            |  2 --
xen/arch/arm/arm64/smc.S             |  3 ---
xen/include/asm-arm/arm64/macros.h   |  6 ++++++
xen/include/asm-arm/config.h         |  6 ++++++
xen/include/asm-arm/macros.h         | 18 +++++++++---------
13 files changed, 24 insertions(+), 23 deletions(-)
[PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation
Posted by Julien Grall 3 years, 1 month ago
From: Julien Grall <jgrall@amazon.com>

Hi all,

Last year, Arm released a whitepaper about a new category of speculation.
(see [1] and [2]). In short, a processor may be able to speculate past
some of the unconditional control flow instructions (e.g eret, smc, br).

In some of the cases, the registers will contain values controlled by
the guest. While there is no known gadget afterwards, we still want to
prevent any leakage in the future.

The mitigation is planned in two parts:
   1) Arm provided patches for both GCC and LLVM to add speculation barrier
   and remove problematic code sequence.
   2) Inspection of assembly code and call to higher level (e.g smc in our case).

I still haven't looked at 1) and how to mitigate properly Arm32 (see
patch #1) and SMC call. So this issue is not fully addressed.

Note that the ERET instruction was already addressed as part of XSA-312.

Cheers,

[1] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability
[2] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability/downloads/straight-line-speculation

Julien Grall (2):
  xen/arm: Include asm/asm-offsets.h and asm/macros.h on every assembly
    files
  xen/arm64: Place a speculation barrier following an ret instruction

 xen/arch/arm/Makefile                |  2 +-
 xen/arch/arm/arm32/entry.S           |  2 +-
 xen/arch/arm/arm32/head.S            |  1 -
 xen/arch/arm/arm32/lib/lib1funcs.S   |  1 +
 xen/arch/arm/arm32/proc-v7.S         |  1 -
 xen/arch/arm/arm64/debug-cadence.inc |  1 -
 xen/arch/arm/arm64/debug-pl011.inc   |  2 --
 xen/arch/arm/arm64/entry.S           |  2 --
 xen/arch/arm/arm64/head.S            |  2 --
 xen/arch/arm/arm64/smc.S             |  3 ---
 xen/include/asm-arm/arm64/macros.h   |  6 ++++++
 xen/include/asm-arm/config.h         |  6 ++++++
 xen/include/asm-arm/macros.h         | 18 +++++++++---------
 13 files changed, 24 insertions(+), 23 deletions(-)

-- 
2.17.1


Re: [PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation
Posted by Bertrand Marquis 3 years, 1 month ago
Hi Julien,

> On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Hi all,
> 
> Last year, Arm released a whitepaper about a new category of speculation.
> (see [1] and [2]). In short, a processor may be able to speculate past
> some of the unconditional control flow instructions (e.g eret, smc, br).
> 
> In some of the cases, the registers will contain values controlled by
> the guest. While there is no known gadget afterwards, we still want to
> prevent any leakage in the future.
> 
> The mitigation is planned in two parts:
>   1) Arm provided patches for both GCC and LLVM to add speculation barrier
>   and remove problematic code sequence.
>   2) Inspection of assembly code and call to higher level (e.g smc in our case).
> 
> I still haven't looked at 1) and how to mitigate properly Arm32 (see
> patch #1) and SMC call. So this issue is not fully addressed.
> 
> Note that the ERET instruction was already addressed as part of XSA-312.

On my tests, this serie is breaking the arm64 build:
| aarch64-poky-linux-ld --sysroot=/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/recipe-sysroot         -EL  --fix-cortex-a53-843419 --fix-cortex-a53-843419 -r -o built_in.o memcpy.o memcmp.o memmove.o memset.o memchr.o clear_page.o bitops.o find_next_bit.o strchr.o strcmp.o strlen.o strncmp.o strnlen.o strrchr.o
| arm64/head.S: Assembler messages:
| arm64/head.S:305: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Boot CPU booting -\r\n")'
| arm64/head.S:331: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Ready -\r\n")'
| arm64/head.S:365: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- CPU ")'
| make[6]: Leaving directory '/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/local-xen/xen/xen/arch/arm/arm64/lib'
| arm64/head.S:367: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, " booting -\r\n")'
| arm64/head.S:398: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Ready -\r\n")'
| arm64/head.S:412: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Current EL ")'
| arm64/head.S:415: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, " -\r\n")'
| arm64/head.S:424: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Xen must be entered in NS EL2 mode -\r\n")'
| arm64/head.S:425: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Please update the bootloader -\r\n")'
| arm64/head.S:441: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Zero BSS -\r\n")'
| arm64/head.S:459: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Initialize CPU -\r\n")'
| arm64/head.S:654: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Unable to build boot page tables - virt and phys addresses clash. -\r\n")'
| arm64/head.S:666: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Turning on paging -\r\n")'
| arm64/head.S:800: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Boot failed -\r\n")'
| arm64/head.S:848: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- UART enabled -\r\n")'
| {standard input}: Error: local label `"98" (instance number 1 of a fb label)' is not defined
| /home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/local-xen/xen/xen/Rules.mk:204: recipe for target 'arm64/head.o' failed

This was done adding your 2 patches on top of current staging.

Cheers
Bertrand

> 
> Cheers,
> 
> [1] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability
> [2] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability/downloads/straight-line-speculation
> 
> Julien Grall (2):
>  xen/arm: Include asm/asm-offsets.h and asm/macros.h on every assembly
>    files
>  xen/arm64: Place a speculation barrier following an ret instruction
> 
> xen/arch/arm/Makefile                |  2 +-
> xen/arch/arm/arm32/entry.S           |  2 +-
> xen/arch/arm/arm32/head.S            |  1 -
> xen/arch/arm/arm32/lib/lib1funcs.S   |  1 +
> xen/arch/arm/arm32/proc-v7.S         |  1 -
> xen/arch/arm/arm64/debug-cadence.inc |  1 -
> xen/arch/arm/arm64/debug-pl011.inc   |  2 --
> xen/arch/arm/arm64/entry.S           |  2 --
> xen/arch/arm/arm64/head.S            |  2 --
> xen/arch/arm/arm64/smc.S             |  3 ---
> xen/include/asm-arm/arm64/macros.h   |  6 ++++++
> xen/include/asm-arm/config.h         |  6 ++++++
> xen/include/asm-arm/macros.h         | 18 +++++++++---------
> 13 files changed, 24 insertions(+), 23 deletions(-)
> 
> -- 
> 2.17.1
> 


Re: [PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation
Posted by Julien Grall 3 years, 1 month ago

On 15/03/2021 13:32, Bertrand Marquis wrote:
> Hi Julien,

Hi Bertrand,

> 
>> On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Hi all,
>>
>> Last year, Arm released a whitepaper about a new category of speculation.
>> (see [1] and [2]). In short, a processor may be able to speculate past
>> some of the unconditional control flow instructions (e.g eret, smc, br).
>>
>> In some of the cases, the registers will contain values controlled by
>> the guest. While there is no known gadget afterwards, we still want to
>> prevent any leakage in the future.
>>
>> The mitigation is planned in two parts:
>>    1) Arm provided patches for both GCC and LLVM to add speculation barrier
>>    and remove problematic code sequence.
>>    2) Inspection of assembly code and call to higher level (e.g smc in our case).
>>
>> I still haven't looked at 1) and how to mitigate properly Arm32 (see
>> patch #1) and SMC call. So this issue is not fully addressed.
>>
>> Note that the ERET instruction was already addressed as part of XSA-312.
> 
> On my tests, this serie is breaking the arm64 build:
> | aarch64-poky-linux-ld --sysroot=/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/recipe-sysroot         -EL  --fix-cortex-a53-843419 --fix-cortex-a53-843419 -r -o built_in.o memcpy.o memcmp.o memmove.o memset.o memchr.o clear_page.o bitops.o find_next_bit.o strchr.o strcmp.o strlen.o strncmp.o strnlen.o strrchr.o

I can't see any build failure with the following GCC:

42sh> aarch64-linux-gnu-gcc
aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

AFAICT, there is also no compilation issue reported by gitlab:

https://gitlab.com/xen-project/patchew/xen/-/pipelines/269989894

What's the version of your compiler? Do you have steps to reproduce your 
setup?

> | arm64/head.S: Assembler messages:
> | arm64/head.S:305: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Boot CPU booting -\r\n")'

This is strange, the code should use RODATA_STR() but here it is in 
lower case. Can you check in your tree whether there some instance of 
the lower case version?

If not, this may just be GAS printing in lower cases.

> | arm64/head.S:331: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Ready -\r\n")'
> | arm64/head.S:365: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- CPU ")'
> | make[6]: Leaving directory '/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/local-xen/xen/xen/arch/arm/arm64/lib'
> | arm64/head.S:367: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, " booting -\r\n")'
> | arm64/head.S:398: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Ready -\r\n")'
> | arm64/head.S:412: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Current EL ")'
> | arm64/head.S:415: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, " -\r\n")'
> | arm64/head.S:424: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Xen must be entered in NS EL2 mode -\r\n")'
> | arm64/head.S:425: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Please update the bootloader -\r\n")'
> | arm64/head.S:441: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Zero BSS -\r\n")'
> | arm64/head.S:459: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Initialize CPU -\r\n")'
> | arm64/head.S:654: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Unable to build boot page tables - virt and phys addresses clash. -\r\n")'
> | arm64/head.S:666: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Turning on paging -\r\n")'
> | arm64/head.S:800: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Boot failed -\r\n")'
> | arm64/head.S:848: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- UART enabled -\r\n")'
> | {standard input}: Error: local label `"98" (instance number 1 of a fb label)' is not defined
> | /home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/local-xen/xen/xen/Rules.mk:204: recipe for target 'arm64/head.o' failed
> 
> This was done adding your 2 patches on top of current staging.
> 
> Cheers
> Bertrand
> 
>>
>> Cheers,
>>
>> [1] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability
>> [2] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability/downloads/straight-line-speculation
>>
>> Julien Grall (2):
>>   xen/arm: Include asm/asm-offsets.h and asm/macros.h on every assembly
>>     files
>>   xen/arm64: Place a speculation barrier following an ret instruction
>>
>> xen/arch/arm/Makefile                |  2 +-
>> xen/arch/arm/arm32/entry.S           |  2 +-
>> xen/arch/arm/arm32/head.S            |  1 -
>> xen/arch/arm/arm32/lib/lib1funcs.S   |  1 +
>> xen/arch/arm/arm32/proc-v7.S         |  1 -
>> xen/arch/arm/arm64/debug-cadence.inc |  1 -
>> xen/arch/arm/arm64/debug-pl011.inc   |  2 --
>> xen/arch/arm/arm64/entry.S           |  2 --
>> xen/arch/arm/arm64/head.S            |  2 --
>> xen/arch/arm/arm64/smc.S             |  3 ---
>> xen/include/asm-arm/arm64/macros.h   |  6 ++++++
>> xen/include/asm-arm/config.h         |  6 ++++++
>> xen/include/asm-arm/macros.h         | 18 +++++++++---------
>> 13 files changed, 24 insertions(+), 23 deletions(-)
>>
>> -- 
>> 2.17.1
>>
> 

-- 
Julien Grall

Re: [PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation
Posted by Bertrand Marquis 3 years, 1 month ago
Hi Julien,

> On 16 Mar 2021, at 15:27, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 15/03/2021 13:32, Bertrand Marquis wrote:
>> Hi Julien,
> 
> Hi Bertrand,
> 
>>> On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote:
>>> 
>>> From: Julien Grall <jgrall@amazon.com>
>>> 
>>> Hi all,
>>> 
>>> Last year, Arm released a whitepaper about a new category of speculation.
>>> (see [1] and [2]). In short, a processor may be able to speculate past
>>> some of the unconditional control flow instructions (e.g eret, smc, br).
>>> 
>>> In some of the cases, the registers will contain values controlled by
>>> the guest. While there is no known gadget afterwards, we still want to
>>> prevent any leakage in the future.
>>> 
>>> The mitigation is planned in two parts:
>>>   1) Arm provided patches for both GCC and LLVM to add speculation barrier
>>>   and remove problematic code sequence.
>>>   2) Inspection of assembly code and call to higher level (e.g smc in our case).
>>> 
>>> I still haven't looked at 1) and how to mitigate properly Arm32 (see
>>> patch #1) and SMC call. So this issue is not fully addressed.
>>> 
>>> Note that the ERET instruction was already addressed as part of XSA-312.
>> On my tests, this serie is breaking the arm64 build:
>> | aarch64-poky-linux-ld --sysroot=/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/recipe-sysroot         -EL  --fix-cortex-a53-843419 --fix-cortex-a53-843419 -r -o built_in.o memcpy.o memcmp.o memmove.o memset.o memchr.o clear_page.o bitops.o find_next_bit.o strchr.o strcmp.o strlen.o strncmp.o strnlen.o strrchr.o
> 
> I can't see any build failure with the following GCC:
> 
> 42sh> aarch64-linux-gnu-gcc
> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
> Copyright (C) 2017 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> AFAICT, there is also no compilation issue reported by gitlab:
> 
> https://gitlab.com/xen-project/patchew/xen/-/pipelines/269989894
> 
> What's the version of your compiler? Do you have steps to reproduce your setup?

You need to have earlyprintk enabled
I am using gcc 7.5.0:
aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0

one configuration triggering the issue is using the default .config with the following items added:
CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS=y
CONFIG_DEBUG_LOCK_PROFILE=y
CONFIG_PERF_COUNTERS=y
CONFIG_PERF_ARRAYS=y
CONFIG_DEVICE_TREE_DEBUG=y
CONFIG_DEBUG_TRACE=y
CONFIG_EARLY_PRINTK_JUNO=y
CONFIG_EARLY_UART_PL011=y
CONFIG_EARLY_PRINTK=y
CONFIG_EARLY_UART_BASE_ADDRESS=0x7ff80000
CONFIG_EARLY_UART_PL011_BAUD_RATE=115200
CONFIG_EARLY_UART_INIT=y
CONFIG_EARLY_PRINTK_INC="debug-pl011.inc”

> 
>> | arm64/head.S: Assembler messages:
>> | arm64/head.S:305: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Boot CPU booting -\r\n")'
> 
> This is strange, the code should use RODATA_STR() but here it is in lower case. Can you check in your tree whether there some instance of the lower case version?

I have no instance of rodata_str in lower case.

> 
> If not, this may just be GAS printing in lower cases.

it probably is then.

If you need help on this i can try to dig on that a bit later this week (thursday or friday).

Cheers
Bertrand

> 
>> | arm64/head.S:331: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Ready -\r\n")'
>> | arm64/head.S:365: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- CPU ")'
>> | make[6]: Leaving directory '/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/local-xen/xen/xen/arch/arm/arm64/lib'
>> | arm64/head.S:367: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, " booting -\r\n")'
>> | arm64/head.S:398: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Ready -\r\n")'
>> | arm64/head.S:412: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Current EL ")'
>> | arm64/head.S:415: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, " -\r\n")'
>> | arm64/head.S:424: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Xen must be entered in NS EL2 mode -\r\n")'
>> | arm64/head.S:425: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Please update the bootloader -\r\n")'
>> | arm64/head.S:441: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Zero BSS -\r\n")'
>> | arm64/head.S:459: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Initialize CPU -\r\n")'
>> | arm64/head.S:654: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Unable to build boot page tables - virt and phys addresses clash. -\r\n")'
>> | arm64/head.S:666: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Turning on paging -\r\n")'
>> | arm64/head.S:800: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- Boot failed -\r\n")'
>> | arm64/head.S:848: Error: unknown mnemonic `rodata_str' -- `rodata_str(98, "- UART enabled -\r\n")'
>> | {standard input}: Error: local label `"98" (instance number 1 of a fb label)' is not defined
>> | /home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/local-xen/xen/xen/Rules.mk:204: recipe for target 'arm64/head.o' failed
>> This was done adding your 2 patches on top of current staging.
>> Cheers
>> Bertrand
>>> 
>>> Cheers,
>>> 
>>> [1] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability
>>> [2] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability/downloads/straight-line-speculation
>>> 
>>> Julien Grall (2):
>>>  xen/arm: Include asm/asm-offsets.h and asm/macros.h on every assembly
>>>    files
>>>  xen/arm64: Place a speculation barrier following an ret instruction
>>> 
>>> xen/arch/arm/Makefile                |  2 +-
>>> xen/arch/arm/arm32/entry.S           |  2 +-
>>> xen/arch/arm/arm32/head.S            |  1 -
>>> xen/arch/arm/arm32/lib/lib1funcs.S   |  1 +
>>> xen/arch/arm/arm32/proc-v7.S         |  1 -
>>> xen/arch/arm/arm64/debug-cadence.inc |  1 -
>>> xen/arch/arm/arm64/debug-pl011.inc   |  2 --
>>> xen/arch/arm/arm64/entry.S           |  2 --
>>> xen/arch/arm/arm64/head.S            |  2 --
>>> xen/arch/arm/arm64/smc.S             |  3 ---
>>> xen/include/asm-arm/arm64/macros.h   |  6 ++++++
>>> xen/include/asm-arm/config.h         |  6 ++++++
>>> xen/include/asm-arm/macros.h         | 18 +++++++++---------
>>> 13 files changed, 24 insertions(+), 23 deletions(-)
>>> 
>>> -- 
>>> 2.17.1
>>> 
> 
> -- 
> Julien Grall

Re: [PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation
Posted by Julien Grall 3 years, 1 month ago

On 16/03/2021 17:16, Bertrand Marquis wrote:
> Hi Julien,

Hi Bertrand,

> 
>> On 16 Mar 2021, at 15:27, Julien Grall <julien@xen.org> wrote:
>>
>>
>>
>> On 15/03/2021 13:32, Bertrand Marquis wrote:
>>> Hi Julien,
>>
>> Hi Bertrand,
>>
>>>> On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote:
>>>>
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> Hi all,
>>>>
>>>> Last year, Arm released a whitepaper about a new category of speculation.
>>>> (see [1] and [2]). In short, a processor may be able to speculate past
>>>> some of the unconditional control flow instructions (e.g eret, smc, br).
>>>>
>>>> In some of the cases, the registers will contain values controlled by
>>>> the guest. While there is no known gadget afterwards, we still want to
>>>> prevent any leakage in the future.
>>>>
>>>> The mitigation is planned in two parts:
>>>>    1) Arm provided patches for both GCC and LLVM to add speculation barrier
>>>>    and remove problematic code sequence.
>>>>    2) Inspection of assembly code and call to higher level (e.g smc in our case).
>>>>
>>>> I still haven't looked at 1) and how to mitigate properly Arm32 (see
>>>> patch #1) and SMC call. So this issue is not fully addressed.
>>>>
>>>> Note that the ERET instruction was already addressed as part of XSA-312.
>>> On my tests, this serie is breaking the arm64 build:
>>> | aarch64-poky-linux-ld --sysroot=/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/recipe-sysroot         -EL  --fix-cortex-a53-843419 --fix-cortex-a53-843419 -r -o built_in.o memcpy.o memcmp.o memmove.o memset.o memchr.o clear_page.o bitops.o find_next_bit.o strchr.o strcmp.o strlen.o strncmp.o strnlen.o strrchr.o
>>
>> I can't see any build failure with the following GCC:
>>
>> 42sh> aarch64-linux-gnu-gcc
>> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
>> Copyright (C) 2017 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
>> AFAICT, there is also no compilation issue reported by gitlab:
>>
>> https://gitlab.com/xen-project/patchew/xen/-/pipelines/269989894
>>
>> What's the version of your compiler? Do you have steps to reproduce your setup?
> 
> You need to have earlyprintk enabled
> I am using gcc 7.5.0:
> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
> 
> one configuration triggering the issue is using the default .config with the following items added:
> CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS=y
> CONFIG_DEBUG_LOCK_PROFILE=y
> CONFIG_PERF_COUNTERS=y
> CONFIG_PERF_ARRAYS=y
> CONFIG_DEVICE_TREE_DEBUG=y
> CONFIG_DEBUG_TRACE=y
> CONFIG_EARLY_PRINTK_JUNO=y
> CONFIG_EARLY_UART_PL011=y
> CONFIG_EARLY_PRINTK=y
> CONFIG_EARLY_UART_BASE_ADDRESS=0x7ff80000
> CONFIG_EARLY_UART_PL011_BAUD_RATE=115200
> CONFIG_EARLY_UART_INIT=y
> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc”

Thanks for providing the .config. I managed to reproduce it. So I 
removed "asm_defns.h" everywhere but forgot to include it in the 
"config.h" :/.

This small change fixed the error:

diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 51273b9db1fc..c7b77912013e 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -192,7 +192,7 @@ extern unsigned long frametable_virt_end;
  #define watchdog_enable()  ((void)0)

  #if defined(__ASSEMBLY__) && !defined(__LINKER__)
-#include <asm/asm-offsets.h>
+#include <asm/asm_defns.h>
  #include <asm/macros.h>
  #endif

Would you still be happy to review the series before I send a v3?

Cheers,

-- 
Julien Grall

Re: [PATCH for-next v2 0/2] xen/arm: Mitigate straight-line speculation
Posted by Bertrand Marquis 3 years, 1 month ago
Hi Julien,

> On 17 Mar 2021, at 11:20, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 16/03/2021 17:16, Bertrand Marquis wrote:
>> Hi Julien,
> 
> Hi Bertrand,
> 
>>> On 16 Mar 2021, at 15:27, Julien Grall <julien@xen.org> wrote:
>>> 
>>> 
>>> 
>>> On 15/03/2021 13:32, Bertrand Marquis wrote:
>>>> Hi Julien,
>>> 
>>> Hi Bertrand,
>>> 
>>>>> On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote:
>>>>> 
>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>> 
>>>>> Hi all,
>>>>> 
>>>>> Last year, Arm released a whitepaper about a new category of speculation.
>>>>> (see [1] and [2]). In short, a processor may be able to speculate past
>>>>> some of the unconditional control flow instructions (e.g eret, smc, br).
>>>>> 
>>>>> In some of the cases, the registers will contain values controlled by
>>>>> the guest. While there is no known gadget afterwards, we still want to
>>>>> prevent any leakage in the future.
>>>>> 
>>>>> The mitigation is planned in two parts:
>>>>>   1) Arm provided patches for both GCC and LLVM to add speculation barrier
>>>>>   and remove problematic code sequence.
>>>>>   2) Inspection of assembly code and call to higher level (e.g smc in our case).
>>>>> 
>>>>> I still haven't looked at 1) and how to mitigate properly Arm32 (see
>>>>> patch #1) and SMC call. So this issue is not fully addressed.
>>>>> 
>>>>> Note that the ERET instruction was already addressed as part of XSA-312.
>>>> On my tests, this serie is breaking the arm64 build:
>>>> | aarch64-poky-linux-ld --sysroot=/home/bermar01/Development/xen-dev/build/profile-fvp-base.prj/tmp/work/fvp_base-poky-linux/xen/4.15+git1-r0/recipe-sysroot         -EL  --fix-cortex-a53-843419 --fix-cortex-a53-843419 -r -o built_in.o memcpy.o memcmp.o memmove.o memset.o memchr.o clear_page.o bitops.o find_next_bit.o strchr.o strcmp.o strlen.o strncmp.o strnlen.o strrchr.o
>>> 
>>> I can't see any build failure with the following GCC:
>>> 
>>> 42sh> aarch64-linux-gnu-gcc
>>> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
>>> Copyright (C) 2017 Free Software Foundation, Inc.
>>> This is free software; see the source for copying conditions.  There is NO
>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>> 
>>> AFAICT, there is also no compilation issue reported by gitlab:
>>> 
>>> https://gitlab.com/xen-project/patchew/xen/-/pipelines/269989894
>>> 
>>> What's the version of your compiler? Do you have steps to reproduce your setup?
>> You need to have earlyprintk enabled
>> I am using gcc 7.5.0:
>> aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
>> one configuration triggering the issue is using the default .config with the following items added:
>> CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS=y
>> CONFIG_DEBUG_LOCK_PROFILE=y
>> CONFIG_PERF_COUNTERS=y
>> CONFIG_PERF_ARRAYS=y
>> CONFIG_DEVICE_TREE_DEBUG=y
>> CONFIG_DEBUG_TRACE=y
>> CONFIG_EARLY_PRINTK_JUNO=y
>> CONFIG_EARLY_UART_PL011=y
>> CONFIG_EARLY_PRINTK=y
>> CONFIG_EARLY_UART_BASE_ADDRESS=0x7ff80000
>> CONFIG_EARLY_UART_PL011_BAUD_RATE=115200
>> CONFIG_EARLY_UART_INIT=y
>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc”
> 
> Thanks for providing the .config. I managed to reproduce it. So I removed "asm_defns.h" everywhere but forgot to include it in the "config.h" :/.
> 
> This small change fixed the error:
> 
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 51273b9db1fc..c7b77912013e 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -192,7 +192,7 @@ extern unsigned long frametable_virt_end;
> #define watchdog_enable()  ((void)0)
> 
> #if defined(__ASSEMBLY__) && !defined(__LINKER__)
> -#include <asm/asm-offsets.h>
> +#include <asm/asm_defns.h>
> #include <asm/macros.h>
> #endif
> 
> Would you still be happy to review the series before I send a v3?

Sure,

I will restart my tests with this change now and review the v2 once passed.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall