Re: [PATCH 0/3] KVM: arm64: BUG FIX: Correctly handle zero register transfers

2015-12-03 Thread Marc Zyngier
On 03/12/15 10:53, Pavel Fedin wrote:
>  Hello!
> 
>>> The problem has been discovered by performing an operation
>>>
>>>  *((volatile int *)reg) = 0;
>>>
>>> which compiles as "str xzr, [xx]", and resulted in strange values being
>>> written.
>>
>> Interesting find. Which compiler is that?
> 
> $ aarch64-linux-gnu-gcc --version
> aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease)
> Copyright (C) 2014 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.

OK. I was just wondering if that was a new thing or not.

[...]

>  Isn't it legitimate to write from ZR to MMIO register?
>  Another potential case is in our vgic-v3-switch.S:
> 
>   msr_s   ICH_HCR_EL2, xzr
> 
>  It's only because it is KVM code we have never discovered this problem yet. 
> Somebody could write such a thing in some other place,
> with some other register, which would be executed by KVM, and... boo...

I'm certainly not disputing that, this is a real bug that should be
fixed right now.

Looking forward to seeing your v2.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 0/3] KVM: arm64: BUG FIX: Correctly handle zero register transfers

2015-12-03 Thread Pavel Fedin
 Hello!

> > The problem has been discovered by performing an operation
> >
> >  *((volatile int *)reg) = 0;
> >
> > which compiles as "str xzr, [xx]", and resulted in strange values being
> > written.
> 
> Interesting find. Which compiler is that?

$ aarch64-linux-gnu-gcc --version
aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease)
Copyright (C) 2014 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.

 This is from my colleague who actually hit the bug by his driver. And i can 
reproduce the issue with different compiler version
using the following small testcase:
--- cut ---
p.fedin@fedinw7x64 /cygdrive/d/Projects/Test
$ cat test.c
volatile int *addr;

int test_val(int val)
{
*addr = val;
}

int test_zero(void)
{
*addr = 0;
}

p.fedin@fedinw7x64 /cygdrive/d/Projects/Test
$ aarch64-unknown-linux-gnu-gcc -O2 -c test.c

p.fedin@fedinw7x64 /cygdrive/d/Projects/Test
$ aarch64-unknown-linux-gnu-objdump -d test.o

test.o: file format elf64-littleaarch64


Disassembly of section .text:

 :
   0:   2a0003e2mov w2, w0
   4:   2a0103e0mov w0, w1
   8:   9001adrpx1, 8 
   c:   f9400021ldr x1, [x1]
  10:   b922str w2, [x1]
  14:   d65f03c0ret

0018 :
  18:   9001adrpx1, 8 
  1c:   f9400021ldr x1, [x1]
  20:   b93fstr wzr, [x1]
  24:   d65f03c0ret

p.fedin@fedinw7x64 /cygdrive/d/Projects/Test
$ aarch64-unknown-linux-gnu-gcc --version
aarch64-unknown-linux-gnu-gcc (GCC) 4.9.0
Copyright (C) 2014 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.
--- cut ---

 Isn't it legitimate to write from ZR to MMIO register?
 Another potential case is in our vgic-v3-switch.S:

msr_s   ICH_HCR_EL2, xzr

 It's only because it is KVM code we have never discovered this problem yet. 
Somebody could write such a thing in some other place,
with some other register, which would be executed by KVM, and... boo...

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] KVM: arm64: BUG FIX: Correctly handle zero register transfers

2015-12-03 Thread Marc Zyngier
On 03/12/15 09:58, Pavel Fedin wrote:
> ARM64 CPU has zero register which is read-only, with a value of 0.
> However, KVM currently incorrectly recognizes it being SP (because
> Rt == 31, and in struct user_pt_regs 'regs' array is followed by SP),
> resulting in invalid value being read, or even SP corruption on write.

No really. XZR and SP do share the same encoding.

> The problem has been discovered by performing an operation
> 
>  *((volatile int *)reg) = 0;
> 
> which compiles as "str xzr, [xx]", and resulted in strange values being
> written.

Interesting find. Which compiler is that?

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html