11aadaf893
- python/aqmp: use absolute import statement - sysemu: Cleanup qemu_run_machine_init_done_notifiers() - vhost-backend: avoid overflow on memslots_limit - hw/i386/vmmouse: Require 'i8042' property to be set - hw/scsi/megasas: Fails command if SGL buffer overflows - target/i386/kvm: Replace use of __u32 type - hw/avr: Realize AVRCPU qdev object using qdev_realize() - qemu-keymap: Add license in generated files - configure: Symlink binaries using .exe suffix with MinGW - ui: remove break after g_assert_not_reached() - io/channel-websock: Replace strlen(const_str) by sizeof(const_str) - 1 - target/ppc: Add HASHKEYR and HASHPKEYR SPRs - tests: Fix error strings - Hexagon (target/hexagon) remove unused encodings - target/i386: introduce insn_get_addr - target/i386: REPZ and REPNZ are mutually exclusive - target/i386: correctly mask SSE4a bit indices in register operands - bios-tables-test: Make oem-fields tests be consistent - tests/vm: update NetBSD to 9.3 - monitor/hmp-cmds: Avoid displaying bogus size in 'info pci' When BAR aren't mapped, we get: - virtio-mem: don't warn about THP sizes on a kernel without THP Support - Subject: [PATCH] kvm: Use 'unsigned long' for request argument in functions wrapping ioctl() Signed-off-by: Jiabo Feng <fengjiabo1@huawei.com> (cherry picked from commit 2430c96ac522f910c11eb98c2d9f74c2c3336a00)
190 lines
7.3 KiB
Diff
190 lines
7.3 KiB
Diff
From d3dcd2635b1830648a1aa95d5653aba6fd20bde6 Mon Sep 17 00:00:00 2001
|
|
From: dinglimin <dinglimin@cmss.chinamobile.com>
|
|
Date: Mon, 16 Sep 2024 16:36:27 +0800
|
|
Subject: [PATCH] Subject: [PATCH] kvm: Use 'unsigned long' for request
|
|
argument in functions wrapping ioctl()
|
|
|
|
Change the data type of the ioctl _request_ argument from 'int' to
|
|
'unsigned long' for the various accel/kvm functions which are
|
|
essentially wrappers around the ioctl() syscall.
|
|
|
|
The correct type for ioctl()'s 'request' argument is confused:
|
|
* POSIX defines the request argument as 'int'
|
|
* glibc uses 'unsigned long' in the prototype in sys/ioctl.h
|
|
* the glibc info documentation uses 'int'
|
|
* the Linux manpage uses 'unsigned long'
|
|
* the Linux implementation of the syscall uses 'unsigned int'
|
|
|
|
If we wrap ioctl() with another function which uses 'int' as the
|
|
type for the request argument, then requests with the 0x8000_0000
|
|
bit set will be sign-extended when the 'int' is cast to
|
|
'unsigned long' for the call to ioctl().
|
|
|
|
On x86_64 one such example is the KVM_IRQ_LINE_STATUS request.
|
|
Bit requests with the _IOC_READ direction bit set, will have the high
|
|
bit set.
|
|
|
|
Fortunately the Linux Kernel truncates the upper 32bit of the request
|
|
on 64bit machines (because it uses 'unsigned int', and see also Linus
|
|
Torvalds' comments in
|
|
https://sourceware.org/bugzilla/show_bug.cgi?id=14362 )
|
|
so this doesn't cause active problems for us. However it is more
|
|
consistent to follow the glibc ioctl() prototype when we define
|
|
functions that are essentially wrappers around ioctl().
|
|
|
|
This resolves a Coverity issue where it points out that in
|
|
kvm_get_xsave() we assign a value (KVM_GET_XSAVE or KVM_GET_XSAVE2)
|
|
to an 'int' variable which can't hold it without overflow.
|
|
|
|
Resolves: Coverity CID 1547759
|
|
Signed-off-by: Johannes Stoelp <johannes.stoelp@gmail.com>
|
|
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
|
|
Reviewed-by: Eric Blake <eblake@redhat.com>
|
|
Message-id: 20240815122747.3053871-1-peter.maydell@linaro.org
|
|
[PMM: Rebased patch, adjusted commit message, included note about
|
|
Coverity fix, updated the type of the local var in kvm_get_xsave,
|
|
updated the comment in the KVMState struct definition]
|
|
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
|
|
Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
|
|
---
|
|
accel/kvm/kvm-all.c | 8 ++++----
|
|
accel/kvm/trace-events | 8 ++++----
|
|
include/sysemu/kvm.h | 8 ++++----
|
|
include/sysemu/kvm_int.h | 17 +++++++++++++----
|
|
target/i386/kvm/kvm.c | 3 ++-
|
|
5 files changed, 27 insertions(+), 17 deletions(-)
|
|
|
|
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
|
|
index 3a09307a26..2eb0666bd7 100644
|
|
--- a/accel/kvm/kvm-all.c
|
|
+++ b/accel/kvm/kvm-all.c
|
|
@@ -2985,7 +2985,7 @@ int kvm_cpu_exec(CPUState *cpu)
|
|
return ret;
|
|
}
|
|
|
|
-int kvm_ioctl(KVMState *s, int type, ...)
|
|
+int kvm_ioctl(KVMState *s, unsigned long type, ...)
|
|
{
|
|
int ret;
|
|
void *arg;
|
|
@@ -3003,7 +3003,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
|
|
return ret;
|
|
}
|
|
|
|
-int kvm_vm_ioctl(KVMState *s, int type, ...)
|
|
+int kvm_vm_ioctl(KVMState *s, unsigned long type, ...)
|
|
{
|
|
int ret;
|
|
void *arg;
|
|
@@ -3021,7 +3021,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
|
|
return ret;
|
|
}
|
|
|
|
-int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
|
|
+int kvm_vcpu_ioctl(CPUState *cpu, unsigned long type, ...)
|
|
{
|
|
int ret;
|
|
void *arg;
|
|
@@ -3039,7 +3039,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
|
|
return ret;
|
|
}
|
|
|
|
-int kvm_device_ioctl(int fd, int type, ...)
|
|
+int kvm_device_ioctl(int fd, unsigned long type, ...)
|
|
{
|
|
int ret;
|
|
void *arg;
|
|
diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events
|
|
index 399aaeb0ec..a1905fe985 100644
|
|
--- a/accel/kvm/trace-events
|
|
+++ b/accel/kvm/trace-events
|
|
@@ -1,11 +1,11 @@
|
|
# See docs/devel/tracing.rst for syntax documentation.
|
|
|
|
# kvm-all.c
|
|
-kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
|
|
-kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
|
|
-kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type 0x%x, arg %p"
|
|
+kvm_ioctl(unsigned long type, void *arg) "type 0x%lx, arg %p"
|
|
+kvm_vm_ioctl(unsigned long type, void *arg) "type 0x%lx, arg %p"
|
|
+kvm_vcpu_ioctl(int cpu_index, unsigned long type, void *arg) "cpu_index %d, type 0x%lx, arg %p"
|
|
kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d"
|
|
-kvm_device_ioctl(int fd, int type, void *arg) "dev fd %d, type 0x%x, arg %p"
|
|
+kvm_device_ioctl(int fd, unsigned long type, void *arg) "dev fd %d, type 0x%lx, arg %p"
|
|
kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve ONEREG %" PRIu64 " from KVM: %s"
|
|
kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set ONEREG %" PRIu64 " to KVM: %s"
|
|
kvm_init_vcpu(int cpu_index, unsigned long arch_cpu_id) "index: %d id: %lu"
|
|
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
|
|
index 5860228034..5c06cd3d91 100644
|
|
--- a/include/sysemu/kvm.h
|
|
+++ b/include/sysemu/kvm.h
|
|
@@ -258,11 +258,11 @@ int kvm_on_sigbus(int code, void *addr);
|
|
|
|
/* internal API */
|
|
|
|
-int kvm_ioctl(KVMState *s, int type, ...);
|
|
+int kvm_ioctl(KVMState *s, unsigned long type, ...);
|
|
|
|
-int kvm_vm_ioctl(KVMState *s, int type, ...);
|
|
+int kvm_vm_ioctl(KVMState *s, unsigned long type, ...);
|
|
|
|
-int kvm_vcpu_ioctl(CPUState *cpu, int type, ...);
|
|
+int kvm_vcpu_ioctl(CPUState *cpu, unsigned long type, ...);
|
|
|
|
/**
|
|
* kvm_device_ioctl - call an ioctl on a kvm device
|
|
@@ -271,7 +271,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...);
|
|
*
|
|
* Returns: -errno on error, nonnegative on success
|
|
*/
|
|
-int kvm_device_ioctl(int fd, int type, ...);
|
|
+int kvm_device_ioctl(int fd, unsigned long type, ...);
|
|
|
|
/**
|
|
* kvm_vm_check_attr - check for existence of a specific vm attribute
|
|
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
|
|
index 3b4adcdc10..b332d79ce5 100644
|
|
--- a/include/sysemu/kvm_int.h
|
|
+++ b/include/sysemu/kvm_int.h
|
|
@@ -83,10 +83,19 @@ struct KVMState
|
|
OnOffAuto kernel_irqchip_split;
|
|
bool sync_mmu;
|
|
uint64_t manual_dirty_log_protect;
|
|
- /* The man page (and posix) say ioctl numbers are signed int, but
|
|
- * they're not. Linux, glibc and *BSD all treat ioctl numbers as
|
|
- * unsigned, and treating them as signed here can break things */
|
|
- unsigned irq_set_ioctl;
|
|
+ /*
|
|
+ * Older POSIX says that ioctl numbers are signed int, but in
|
|
+ * practice they are not. (Newer POSIX doesn't specify ioctl
|
|
+ * at all.) Linux, glibc and *BSD all treat ioctl numbers as
|
|
+ * unsigned, and real-world ioctl values like KVM_GET_XSAVE have
|
|
+ * bit 31 set, which means that passing them via an 'int' will
|
|
+ * result in sign-extension when they get converted back to the
|
|
+ * 'unsigned long' which the ioctl() prototype uses. Luckily Linux
|
|
+ * always treats the argument as an unsigned 32-bit int, so any
|
|
+ * possible sign-extension is deliberately ignored, but for
|
|
+ * consistency we keep to the same type that glibc is using.
|
|
+ */
|
|
+ unsigned long irq_set_ioctl;
|
|
unsigned int sigmask_len;
|
|
GHashTable *gsimap;
|
|
#ifdef KVM_CAP_IRQ_ROUTING
|
|
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
|
|
index 9f3ddd5da2..15755197f4 100644
|
|
--- a/target/i386/kvm/kvm.c
|
|
+++ b/target/i386/kvm/kvm.c
|
|
@@ -3360,7 +3360,8 @@ static int kvm_get_xsave(X86CPU *cpu)
|
|
{
|
|
CPUX86State *env = &cpu->env;
|
|
void *xsave = env->xsave_buf;
|
|
- int type, ret;
|
|
+ unsigned long type;
|
|
+ int ret;
|
|
|
|
if (!has_xsave) {
|
|
return kvm_get_fpu(cpu);
|
|
--
|
|
2.41.0.windows.1
|
|
|