[PATCH 1/5] arm64: KVM: Do not use stack-protector to compile EL2 code

[Marc Zyngier]

On 02/05/17 15:50, Marc Zyngier wrote:
> On 02/05/17 15:40, Catalin Marinas wrote:
>> On Tue, May 02, 2017 at 02:30:37PM +0100, Marc Zyngier wrote:
>>> We like living dangerously. Nothing explicitely forbids stack-protector
>>> to be used in the EL2 code, while distributions routinely compile their
>>> kernel with it. We're just lucky that no code actually triggers the
>>> instrumentation.
>>>
>>> Let's not try our luck for much longer, and disable stack-protector
>>> for code living at EL2.
>>>
>>> Cc: stable at vger.kernel.org
>>> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
>>> ---
>>>  arch/arm64/kvm/hyp/Makefile | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
>>> index aaf42ae8d8c3..14c4e3b14bcb 100644
>>> --- a/arch/arm64/kvm/hyp/Makefile
>>> +++ b/arch/arm64/kvm/hyp/Makefile
>>> @@ -2,6 +2,8 @@
>>>  # Makefile for Kernel-based Virtual Machine module, HYP part
>>>  #
>>>  
>>> +ccflags-y += -fno-stack-protector
>>> +
>>
>> While you are at it, should we have a -fpic here as well? The hyp code
>> runs at a different location than the rest of the kernel.
> 
> We definitely should. I've just tried this, and this doesn't seem to
> work very well. At least this seems to break our jump label
> implementation. I need to page in that part of the code base and see
> what happens.

So here's the issue:

  CC      arch/arm64/kvm/hyp/../../../../virt/kvm/arm/hyp/vgic-v3-sr.o
In file included from ./include/linux/jump_label.h:120:0,
                 from ./include/linux/dynamic_debug.h:5,
                 from ./include/linux/printk.h:329,
                 from ./include/linux/kernel.h:13,
                 from ./include/asm-generic/bug.h:15,
                 from ./arch/arm64/include/asm/bug.h:66,
                 from ./include/linux/bug.h:4,
                 from ./include/linux/mmdebug.h:4,
                 from ./include/linux/mm.h:8,
                 from ./arch/arm64/include/asm/cacheflush.h:22,
                 from ./arch/arm64/include/asm/arch_gicv3.h:27,
                 from ./include/linux/irqchip/arm-gic-v3.h:453,
                 from arch/arm64/kvm/hyp/../../../../virt/kvm/arm/hyp/vgic-v3-sr.c:19:
./arch/arm64/include/asm/jump_label.h: In function '__vgic_v3_save_state':
./arch/arm64/include/asm/jump_label.h:31:2: warning: asm operand 0 probably doesn't match constraints
  asm goto("1: nop\n\t"
  ^~~
./arch/arm64/include/asm/jump_label.h:31:2: error: impossible constraint in 'asm'
./arch/arm64/include/asm/jump_label.h: In function '__vgic_v3_restore_state':
./arch/arm64/include/asm/jump_label.h:31:2: warning: asm operand 0 probably doesn't match constraints
  asm goto("1: nop\n\t"
  ^~~
scripts/Makefile.build:294: recipe for target 'arch/arm64/kvm/hyp/../../../../virt/kvm/arm/hyp/vgic-v3-sr.o' failed
make[1]: *** [arch/arm64/kvm/hyp/../../../../virt/kvm/arm/hyp/vgic-v3-sr.o] Error 1
Makefile:1664: recipe for target 'arch/arm64/kvm/hyp/' failed

The corresponding code does this:

static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
{
	asm goto("1: nop\n\t"
		 ".pushsection __jump_table,  \"aw\"\n\t"
		 ".align 3\n\t"
		 ".quad 1b, %l[l_yes], %c0\n\t"
		 ".popsection\n\t"
		 :  :  "i"(&((char *)key)[branch]) :  : l_yes);

	return false;
l_yes:
	return true;
}

and the problem lies in the evaluation of "key", which probably
cannot be guaranteed a constant at that point. There is also the
issue that even if it was known, the branch cannot be easily 
patched in from the rest of the kernel (how is the l_yes address
represented?).

It looks to me like we either need to rewrite the whole of our
static key infrastructure to cope with PIC, or switch over to
the hyp_alternate_select() hack, which I'd rather avoid spreading
further.

In the end, I wonder if that's even worth it...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...