Re: [PATCH 0/3] KVM: arm64: BUG FIX: Correctly handle zero register transfers
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
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
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