Re: [PATCH] x86: guard against straight-line speculation past RET

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.09.2020 15:50, Andrew Cooper wrote:
> On 07/09/2020 10:25, Jan Beulich wrote:
>> On 04.09.2020 20:18, Andrew Cooper wrote:
>>> Third, there is a huge quantity of complexity for a form of the
>>> instruction we don't use.
>> The complexity isn't with handling the possible immediate operand,
>> but with the need to override the "ret" insn, and then to transiently
>> cancel this override.
> 
> What is the purpose of transiently cancelling the override?

To have access to the underlying insn again.

> It's not possible to pull this trick twice, so its not as if you're
> falling back to a different macro rather than the plain instruction.

Not sure I understand what you're saying here. The helper macro
purging and then re-instating the insn surrogate macro can be
done any number of times.

>>> Clang doesn't actually expand the macro for ret instructions, so a Clang
>>> build of Xen only ends up getting protected in the assembly files.
>>>
>>> The following experiment demonstrates the issue:
>>>
>>> $ cat ret.c
>>> asm (".macro ret\n\t"
>>>      ".error \"foo\"\n\t"
>>>      ".endm\n\t");
>>> void foo(void) {}
>>>
>>> $ gcc -O3 -c ret.c -o ret.o && objdump -d ret.o
>>> /tmp/ccf8hkyN.s: Assembler messages:
>>> /tmp/ccf8hkyN.s:16: Error: foo
>>>
>>> $ clang-10 -O3 -c ret.c -o ret.o && objdump -d ret.o
>>>
>>> ret.o:     file format elf64-x86-64
>>>
>>>
>>> Disassembly of section .text:
>>>
>>> 0000000000000000 <foo>:
>>>    0:    c3                       retq
>>>
>>>
>>> Worse, -no-integrated-as doesn't immediately help, even though it
>>> invokes $(AS).
>>>
>>> I tracked that down to the difference between ret and retq, which
>>> highlights an assumption about GCC which may not remain true in the future.
>>>
>>> Adding a second macro covering retq fixes the scenario in combination
>>> with -no-integrated-as.
>> Ah, yes, I should of course have thought of retq. Albeit as per
>> above - generated code looks fine here when using clang 5.
>>
>>> So overall I think we can make a safe binary with a clang build. 
>>> However, it is at the expense of the integrated assembler, which I
>>> believe is now mandatory for LTO, and control-flow integrity, neither of
>>> which we want to lose in the long term.
>> Why at this expense? Are you saying that even when going the .byte
>> route and even with very new clang one has to force
>> -mno-integrated-as?
> 
> Yes, which was the whole point of providing the full transcript above.
> 
> You can't wrap an instruction with a macro with the integrated assembler.

Ugly. I can't currently think of a way to address this then, for clang,
other than by them offering a suitable command line option. Question is:
Do we consider it worthwhile then to still address for gcc? Or shall I
drop the patch?

Jan