* [PATCH v2] target/i386: kvm: do not access uninitialized variable on older kernels
@ 2022-03-20 19:39 Paolo Bonzini
2022-03-21 8:21 ` Peter Krempa
2022-03-22 13:49 ` Richard Henderson
0 siblings, 2 replies; 3+ messages in thread
From: Paolo Bonzini @ 2022-03-20 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pkrempa, vr_qemu
KVM support for AMX includes a new system attribute, KVM_X86_XCOMP_GUEST_SUPP.
Commit 19db68ca68 ("x86: Grant AMX permission for guest", 2022-03-15) however
did not fully consider the behavior on older kernels. First, it warns
too aggressively. Second, it invokes the KVM_GET_DEVICE_ATTR ioctl
unconditionally and then uses the "bitmask" variable, which remains
uninitialized if the ioctl fails. Third, kvm_ioctl returns -errno rather
than -1 on errors.
While at it, explain why the ioctl is needed and KVM_GET_SUPPORTED_CPUID
is not enough.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
v1->v2: for error condition for kvm_ioctl [Volker]
target/i386/kvm/kvm.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index ef2c68a6f4..06901c2a43 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -411,6 +411,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
}
} else if (function == 0xd && index == 0 &&
(reg == R_EAX || reg == R_EDX)) {
+ /*
+ * The value returned by KVM_GET_SUPPORTED_CPUID does not include
+ * features that still have to be enabled with the arch_prctl
+ * system call. QEMU needs the full value, which is retrieved
+ * with KVM_GET_DEVICE_ATTR.
+ */
struct kvm_device_attr attr = {
.group = 0,
.attr = KVM_X86_XCOMP_GUEST_SUPP,
@@ -419,13 +425,16 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
bool sys_attr = kvm_check_extension(s, KVM_CAP_SYS_ATTRIBUTES);
if (!sys_attr) {
- warn_report("cannot get sys attribute capabilities %d", sys_attr);
+ return ret;
}
int rc = kvm_ioctl(s, KVM_GET_DEVICE_ATTR, &attr);
- if (rc == -1 && (errno == ENXIO || errno == EINVAL)) {
- warn_report("KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) "
- "error: %d", rc);
+ if (rc < 0) {
+ if (rc != -ENXIO) {
+ warn_report("KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) "
+ "error: %d", rc);
+ }
+ return ret;
}
ret = (reg == R_EAX) ? bitmask : bitmask >> 32;
} else if (function == 0x80000001 && reg == R_ECX) {
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] target/i386: kvm: do not access uninitialized variable on older kernels
2022-03-20 19:39 [PATCH v2] target/i386: kvm: do not access uninitialized variable on older kernels Paolo Bonzini
@ 2022-03-21 8:21 ` Peter Krempa
2022-03-22 13:49 ` Richard Henderson
1 sibling, 0 replies; 3+ messages in thread
From: Peter Krempa @ 2022-03-21 8:21 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: vr_qemu, qemu-devel
On Sun, Mar 20, 2022 at 20:39:14 +0100, Paolo Bonzini wrote:
> KVM support for AMX includes a new system attribute, KVM_X86_XCOMP_GUEST_SUPP.
> Commit 19db68ca68 ("x86: Grant AMX permission for guest", 2022-03-15) however
> did not fully consider the behavior on older kernels. First, it warns
> too aggressively. Second, it invokes the KVM_GET_DEVICE_ATTR ioctl
> unconditionally and then uses the "bitmask" variable, which remains
> uninitialized if the ioctl fails. Third, kvm_ioctl returns -errno rather
> than -1 on errors.
>
> While at it, explain why the ioctl is needed and KVM_GET_SUPPORTED_CPUID
> is not enough.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> v1->v2: for error condition for kvm_ioctl [Volker]
> target/i386/kvm/kvm.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
Based on the commit message it seems that this is only for the problem
that Michal reported, right?
Because it doesn't still fix the assertion failure on my AMD box:
$ ./build/qemu-system-x86_64 -accel kvm
qemu-system-x86_64: ../target/i386/kvm/kvm-cpu.c:105: kvm_cpu_xsave_init: Assertion `esa->size == eax' failed.
Aborted (core dumped)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] target/i386: kvm: do not access uninitialized variable on older kernels
2022-03-20 19:39 [PATCH v2] target/i386: kvm: do not access uninitialized variable on older kernels Paolo Bonzini
2022-03-21 8:21 ` Peter Krempa
@ 2022-03-22 13:49 ` Richard Henderson
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2022-03-22 13:49 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: pkrempa, vr_qemu
On 3/20/22 12:39, Paolo Bonzini wrote:
> + if (rc < 0) {
> + if (rc != -ENXIO) {
> + warn_report("KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) "
> + "error: %d", rc);
Better as "error: %s", strerror(-rc) ?
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-22 13:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-20 19:39 [PATCH v2] target/i386: kvm: do not access uninitialized variable on older kernels Paolo Bonzini
2022-03-21 8:21 ` Peter Krempa
2022-03-22 13:49 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.