[PATCH for-9.2 1/2] kvm: Make 'mmap_size' be 'int' in kvm_init_vcpu(), do_kvm_destroy_vcpu()

Peter Maydell posted 2 patches 3 months, 1 week ago
[PATCH for-9.2 1/2] kvm: Make 'mmap_size' be 'int' in kvm_init_vcpu(), do_kvm_destroy_vcpu()
Posted by Peter Maydell 3 months, 1 week ago
In kvm_init_vcpu()and do_kvm_destroy_vcpu(), the return value from
  kvm_ioctl(..., KVM_GET_VCPU_MMAP_SIZE, ...)
is an 'int', but we put it into a 'long' logal variable mmap_size.
Coverity then complains that there might be a truncation when we copy
that value into the 'int ret' which we use for returning a value in
an error-exit codepath. This can't ever actually overflow because
the value was in an 'int' to start with, but it makes more sense
to use 'int' for mmap_size so we don't do the widen-then-narrow
sequence in the first place.

Resolves: Coverity CID 1547515
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Borderline whether this was worth changing, but I came down on
the side of "yes".
---
 accel/kvm/kvm-all.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index beb1988d12c..6c4cb263ba3 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -414,7 +414,7 @@ int kvm_create_and_park_vcpu(CPUState *cpu)
 static int do_kvm_destroy_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
-    long mmap_size;
+    int mmap_size;
     int ret = 0;
 
     trace_kvm_destroy_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
@@ -459,7 +459,7 @@ void kvm_destroy_vcpu(CPUState *cpu)
 int kvm_init_vcpu(CPUState *cpu, Error **errp)
 {
     KVMState *s = kvm_state;
-    long mmap_size;
+    int mmap_size;
     int ret;
 
     trace_kvm_init_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
-- 
2.34.1
Re: [PATCH for-9.2 1/2] kvm: Make 'mmap_size' be 'int' in kvm_init_vcpu(), do_kvm_destroy_vcpu()
Posted by Philippe Mathieu-Daudé 3 months, 1 week ago
On 15/8/24 15:12, Peter Maydell wrote:
> In kvm_init_vcpu()and do_kvm_destroy_vcpu(), the return value from
>    kvm_ioctl(..., KVM_GET_VCPU_MMAP_SIZE, ...)
> is an 'int', but we put it into a 'long' logal variable mmap_size.

Typo "local".

> Coverity then complains that there might be a truncation when we copy
> that value into the 'int ret' which we use for returning a value in
> an error-exit codepath. This can't ever actually overflow because
> the value was in an 'int' to start with, but it makes more sense
> to use 'int' for mmap_size so we don't do the widen-then-narrow
> sequence in the first place.
> 
> Resolves: Coverity CID 1547515
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Borderline whether this was worth changing, but I came down on
> the side of "yes".
> ---
>   accel/kvm/kvm-all.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>