* [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 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: 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 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).