qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno
@ 2025-11-25  9:01 Markus Armbruster
  2025-11-25  9:03 ` Markus Armbruster
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Markus Armbruster @ 2025-11-25  9:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, kvm, eesposit

Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
instead of afterwards, so it works whether accel_ioctl_end() preserves
@errno or not.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 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 f9254ae654..28006d73c5 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3373,10 +3373,10 @@ int kvm_vm_ioctl(KVMState *s, unsigned long type, ...)
     trace_kvm_vm_ioctl(type, arg);
     accel_ioctl_begin();
     ret = ioctl(s->vmfd, type, arg);
-    accel_ioctl_end();
     if (ret == -1) {
         ret = -errno;
     }
+    accel_ioctl_end();
     return ret;
 }
 
@@ -3413,10 +3413,10 @@ int kvm_device_ioctl(int fd, unsigned long type, ...)
     trace_kvm_device_ioctl(fd, type, arg);
     accel_ioctl_begin();
     ret = ioctl(fd, type, arg);
-    accel_ioctl_end();
     if (ret == -1) {
         ret = -errno;
     }
+    accel_ioctl_end();
     return ret;
 }
 
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno
  2025-11-25  9:01 [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno Markus Armbruster
@ 2025-11-25  9:03 ` Markus Armbruster
  2025-11-25 10:33   ` Philippe Mathieu-Daudé
  2025-11-25 10:37 ` Paolo Bonzini
  2025-11-29  7:06 ` Markus Armbruster
  2 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2025-11-25  9:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, kvm, eesposit

Markus Armbruster <armbru@redhat.com> writes:

> Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
> instead of afterwards, so it works whether accel_ioctl_end() preserves
> @errno or not.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

I did not check whether the assumption holds or not.  If it doesn't,
then this needs

  Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno
  2025-11-25  9:03 ` Markus Armbruster
@ 2025-11-25 10:33   ` Philippe Mathieu-Daudé
  2025-11-25 11:40     ` Markus Armbruster
  0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-11-25 10:33 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: pbonzini, kvm, eesposit

On 25/11/25 10:03, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
>> instead of afterwards, so it works whether accel_ioctl_end() preserves
>> @errno or not.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> I did not check whether the assumption holds or not.

Indeed, on Linux the futex syscall is called via qemu_event_set.

>  If it doesn't,
> then this needs
> 
>    Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

LGTM.

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno
  2025-11-25  9:01 [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno Markus Armbruster
  2025-11-25  9:03 ` Markus Armbruster
@ 2025-11-25 10:37 ` Paolo Bonzini
  2025-11-29  7:06 ` Markus Armbruster
  2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2025-11-25 10:37 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, pbonzini, kvm, eesposit

Queued, thanks.

Paolo



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno
  2025-11-25 10:33   ` Philippe Mathieu-Daudé
@ 2025-11-25 11:40     ` Markus Armbruster
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2025-11-25 11:40 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, pbonzini, kvm, eesposit

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

> On 25/11/25 10:03, Markus Armbruster wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>>> Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
>>> instead of afterwards, so it works whether accel_ioctl_end() preserves
>>> @errno or not.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> 
>> I did not check whether the assumption holds or not.
>
> Indeed, on Linux the futex syscall is called via qemu_event_set.

So ...

>>  If it doesn't,
>> then this needs
>> 
>>    Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

... we definitely want this tag, and the commit message should be
clarified as well.  Here's my try:

    kvm: Fix kvm_vm_ioctl() and kvm_device_ioctl() return value

    These functions wrap ioctl().  When ioctl() fails, it sets @errno.
    The wrappers then return that @errno negated.

    Except they call accel_ioctl_end() between calling ioctl() and reading
    @errno.  accel_ioctl_end() can clobber @errno, e.g. when a futex()
    system call fails.  Seems unlikely, but it's a bug all the same.

    Fix by retrieving @errno before calling accel_ioctl_end().

    Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

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

Thanks!



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno
  2025-11-25  9:01 [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno Markus Armbruster
  2025-11-25  9:03 ` Markus Armbruster
  2025-11-25 10:37 ` Paolo Bonzini
@ 2025-11-29  7:06 ` Markus Armbruster
  2 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2025-11-29  7:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, kvm, eesposit

Superseded by

Subject: [PATCH v2] kvm: Fix kvm_vm_ioctl() and kvm_device_ioctl() return value
Date: Fri, 28 Nov 2025 16:20:50 +0100
Message-ID: <20251128152050.3417834-1-armbru@redhat.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-11-29  7:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-25  9:01 [PATCH] kvm: Don't assume accel_ioctl_end() preserves @errno Markus Armbruster
2025-11-25  9:03 ` Markus Armbruster
2025-11-25 10:33   ` Philippe Mathieu-Daudé
2025-11-25 11:40     ` Markus Armbruster
2025-11-25 10:37 ` Paolo Bonzini
2025-11-29  7:06 ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).