1
The following changes since commit f6b06fcceef465de0cf2514c9f76fe0192896781:
1
The following changes since commit 661c2e1ab29cd9c4d268ae3f44712e8d421c0e56:
2
2
3
Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190121-pull-request' into staging (2019-01-23 17:57:47 +0000)
3
scripts/checkpatch: Fix a typo (2025-03-04 09:30:26 +0800)
4
4
5
are available in the Git repository at:
5
are available in the Git repository at:
6
6
7
git://github.com/stefanha/qemu.git tags/block-pull-request
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 8595685986152334b1ec28c78cb0e5e855d56b54:
9
for you to fetch changes up to 2ad638a3d160923ef3dbf87c73944e6e44bdc724:
10
10
11
qemu-coroutine-sleep: drop CoSleepCB (2019-01-24 10:05:16 +0000)
11
block/qed: fix use-after-free by nullifying timer pointer after free (2025-03-06 10:19:54 +0800)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
Changelog: No user-visible changes.
16
QED need_check_timer use-after-free fix
17
17
18
----------------------------------------------------------------
18
----------------------------------------------------------------
19
19
20
Stefan Hajnoczi (2):
20
Denis Rastyogin (1):
21
throttle-groups: fix restart coroutine iothread race
21
block/qed: fix use-after-free by nullifying timer pointer after free
22
iotests: add 238 for throttling tgm unregister iothread segfault
23
22
24
Vladimir Sementsov-Ogievskiy (1):
23
block/qed.c | 1 +
25
qemu-coroutine-sleep: drop CoSleepCB
24
1 file changed, 1 insertion(+)
26
27
include/block/throttle-groups.h | 5 ++++
28
block/throttle-groups.c | 9 +++++++
29
util/qemu-coroutine-sleep.c | 27 +++++++------------
30
tests/qemu-iotests/238 | 47 +++++++++++++++++++++++++++++++++
31
tests/qemu-iotests/238.out | 6 +++++
32
tests/qemu-iotests/group | 1 +
33
6 files changed, 78 insertions(+), 17 deletions(-)
34
create mode 100755 tests/qemu-iotests/238
35
create mode 100644 tests/qemu-iotests/238.out
36
25
37
--
26
--
38
2.20.1
27
2.48.1
39
40
diff view generated by jsdifflib
Deleted patch
1
The following QMP command leads to a crash when iothreads are used:
2
1
3
{ 'execute': 'device_del', 'arguments': {'id': 'data'} }
4
5
The backtrace involves the queue restart coroutine where
6
tgm->throttle_state is a NULL pointer because
7
throttle_group_unregister_tgm() has already been called:
8
9
(gdb) bt full
10
#0 0x00005585a7a3b378 in qemu_mutex_lock_impl (mutex=0xffffffffffffffd0, file=0x5585a7bb3d54 "block/throttle-groups.c", line=412) at util/qemu-thread-posix.c:64
11
err = <optimized out>
12
__PRETTY_FUNCTION__ = "qemu_mutex_lock_impl"
13
__func__ = "qemu_mutex_lock_impl"
14
#1 0x00005585a79be074 in throttle_group_restart_queue_entry (opaque=0x5585a9de4eb0) at block/throttle-groups.c:412
15
_f = <optimized out>
16
data = 0x5585a9de4eb0
17
tgm = 0x5585a9079440
18
ts = 0x0
19
tg = 0xffffffffffffff98
20
is_write = false
21
empty_queue = 255
22
23
This coroutine should not execute in the iothread after the throttle
24
group member has been unregistered!
25
26
The root cause is that the device_del code path schedules the restart
27
coroutine in the iothread while holding the AioContext lock. Therefore
28
the iothread cannot execute the coroutine until after device_del
29
releases the lock - by this time it's too late.
30
31
This patch adds a reference count to ThrottleGroupMember so we can
32
synchronously wait for restart coroutines to complete. Once they are
33
done it is safe to unregister the ThrottleGroupMember.
34
35
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
36
Reviewed-by: Alberto Garcia <berto@igalia.com>
37
Message-id: 20190114133257.30299-2-stefanha@redhat.com
38
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
39
---
40
include/block/throttle-groups.h | 5 +++++
41
block/throttle-groups.c | 9 +++++++++
42
2 files changed, 14 insertions(+)
43
44
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
45
index XXXXXXX..XXXXXXX 100644
46
--- a/include/block/throttle-groups.h
47
+++ b/include/block/throttle-groups.h
48
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleGroupMember {
49
*/
50
unsigned int io_limits_disabled;
51
52
+ /* Number of pending throttle_group_restart_queue_entry() coroutines.
53
+ * Accessed with atomic operations.
54
+ */
55
+ unsigned int restart_pending;
56
+
57
/* The following fields are protected by the ThrottleGroup lock.
58
* See the ThrottleGroup documentation for details.
59
* throttle_state tells us if I/O limits are configured. */
60
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
61
index XXXXXXX..XXXXXXX 100644
62
--- a/block/throttle-groups.c
63
+++ b/block/throttle-groups.c
64
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
65
}
66
67
g_free(data);
68
+
69
+ atomic_dec(&tgm->restart_pending);
70
+ aio_wait_kick();
71
}
72
73
static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write)
74
@@ -XXX,XX +XXX,XX @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
75
* be no timer pending on this tgm at this point */
76
assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
77
78
+ atomic_inc(&tgm->restart_pending);
79
+
80
co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
81
aio_co_enter(tgm->aio_context, co);
82
}
83
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
84
85
tgm->throttle_state = ts;
86
tgm->aio_context = ctx;
87
+ atomic_set(&tgm->restart_pending, 0);
88
89
qemu_mutex_lock(&tg->lock);
90
/* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
91
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
92
return;
93
}
94
95
+ /* Wait for throttle_group_restart_queue_entry() coroutines to finish */
96
+ AIO_WAIT_WHILE(tgm->aio_context, atomic_read(&tgm->restart_pending) > 0);
97
+
98
qemu_mutex_lock(&tg->lock);
99
for (i = 0; i < 2; i++) {
100
assert(tgm->pending_reqs[i] == 0);
101
--
102
2.20.1
103
104
diff view generated by jsdifflib
Deleted patch
1
Hot-unplug a scsi-hd using an iothread. The previous patch fixes a
2
segfault in this scenario.
3
1
4
This patch adds a regression test.
5
6
Suggested-by: Alberto Garcia <berto@igalia.com>
7
Suggested-by: Kevin Wolf <kwolf@redhat.com>
8
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9
Reviewed-by: Alberto Garcia <berto@igalia.com>
10
Message-id: 20190114133257.30299-3-stefanha@redhat.com
11
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
12
---
13
tests/qemu-iotests/238 | 47 ++++++++++++++++++++++++++++++++++++++
14
tests/qemu-iotests/238.out | 6 +++++
15
tests/qemu-iotests/group | 1 +
16
3 files changed, 54 insertions(+)
17
create mode 100755 tests/qemu-iotests/238
18
create mode 100644 tests/qemu-iotests/238.out
19
20
diff --git a/tests/qemu-iotests/238 b/tests/qemu-iotests/238
21
new file mode 100755
22
index XXXXXXX..XXXXXXX
23
--- /dev/null
24
+++ b/tests/qemu-iotests/238
25
@@ -XXX,XX +XXX,XX @@
26
+#!/usr/bin/env python
27
+#
28
+# Regression test for throttle group member unregister segfault with iothread
29
+#
30
+# Copyright (c) 2019 Red Hat, Inc.
31
+#
32
+# This program is free software; you can redistribute it and/or modify
33
+# it under the terms of the GNU General Public License as published by
34
+# the Free Software Foundation; either version 2 of the License, or
35
+# (at your option) any later version.
36
+#
37
+# This program is distributed in the hope that it will be useful,
38
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
39
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
40
+# GNU General Public License for more details.
41
+#
42
+# You should have received a copy of the GNU General Public License
43
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
44
+#
45
+
46
+import sys
47
+import os
48
+import iotests
49
+from iotests import log
50
+
51
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
52
+
53
+from qemu import QEMUMachine
54
+
55
+if iotests.qemu_default_machine == 's390-ccw-virtio':
56
+ virtio_scsi_device = 'virtio-scsi-ccw'
57
+else:
58
+ virtio_scsi_device = 'virtio-scsi-pci'
59
+
60
+vm = QEMUMachine(iotests.qemu_prog)
61
+vm.add_args('-machine', 'accel=kvm')
62
+vm.launch()
63
+
64
+log(vm.qmp('blockdev-add', node_name='hd0', driver='null-co'))
65
+log(vm.qmp('object-add', qom_type='iothread', id='iothread0'))
66
+log(vm.qmp('device_add', id='scsi0', driver=virtio_scsi_device, iothread='iothread0'))
67
+log(vm.qmp('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0'))
68
+log(vm.qmp('block_set_io_throttle', id='scsi-hd0', bps=0, bps_rd=0, bps_wr=0,
69
+ iops=1000, iops_rd=0, iops_wr=0, conv_keys=False))
70
+log(vm.qmp('device_del', id='scsi-hd0'))
71
+
72
+vm.shutdown()
73
diff --git a/tests/qemu-iotests/238.out b/tests/qemu-iotests/238.out
74
new file mode 100644
75
index XXXXXXX..XXXXXXX
76
--- /dev/null
77
+++ b/tests/qemu-iotests/238.out
78
@@ -XXX,XX +XXX,XX @@
79
+{"return": {}}
80
+{"return": {}}
81
+{"return": {}}
82
+{"return": {}}
83
+{"return": {}}
84
+{"return": {}}
85
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
86
index XXXXXXX..XXXXXXX 100644
87
--- a/tests/qemu-iotests/group
88
+++ b/tests/qemu-iotests/group
89
@@ -XXX,XX +XXX,XX @@
90
234 auto quick migration
91
235 auto quick
92
236 auto quick
93
+238 auto quick
94
--
95
2.20.1
96
97
diff view generated by jsdifflib
1
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
1
From: Denis Rastyogin <gerben@altlinux.org>
2
2
3
Drop CoSleepCB structure. It's actually unused.
3
This error was discovered by fuzzing qemu-img.
4
4
5
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
5
In the QED block driver, the need_check_timer timer is freed in
6
Message-id: 20190122143113.20331-1-vsementsov@virtuozzo.com
6
bdrv_qed_detach_aio_context, but the pointer to the timer is not
7
set to NULL. This can lead to a use-after-free scenario
8
in bdrv_qed_drain_begin().
9
10
The need_check_timer pointer is set to NULL after freeing the timer.
11
Which helps catch this condition when checking in bdrv_qed_drain_begin().
12
13
Closes: https://gitlab.com/qemu-project/qemu/-/issues/2852
14
Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
15
Message-ID: <20250304083927.37681-1-gerben@altlinux.org>
7
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
16
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
8
---
17
---
9
util/qemu-coroutine-sleep.c | 27 ++++++++++-----------------
18
block/qed.c | 1 +
10
1 file changed, 10 insertions(+), 17 deletions(-)
19
1 file changed, 1 insertion(+)
11
20
12
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
21
diff --git a/block/qed.c b/block/qed.c
13
index XXXXXXX..XXXXXXX 100644
22
index XXXXXXX..XXXXXXX 100644
14
--- a/util/qemu-coroutine-sleep.c
23
--- a/block/qed.c
15
+++ b/util/qemu-coroutine-sleep.c
24
+++ b/block/qed.c
16
@@ -XXX,XX +XXX,XX @@
25
@@ -XXX,XX +XXX,XX @@ static void bdrv_qed_detach_aio_context(BlockDriverState *bs)
17
#include "qemu/timer.h"
26
18
#include "block/aio.h"
27
qed_cancel_need_check_timer(s);
19
28
timer_free(s->need_check_timer);
20
-typedef struct CoSleepCB {
29
+ s->need_check_timer = NULL;
21
- QEMUTimer *ts;
22
- Coroutine *co;
23
-} CoSleepCB;
24
-
25
static void co_sleep_cb(void *opaque)
26
{
27
- CoSleepCB *sleep_cb = opaque;
28
+ Coroutine *co = opaque;
29
30
/* Write of schedule protected by barrier write in aio_co_schedule */
31
- atomic_set(&sleep_cb->co->scheduled, NULL);
32
- aio_co_wake(sleep_cb->co);
33
+ atomic_set(&co->scheduled, NULL);
34
+ aio_co_wake(co);
35
}
30
}
36
31
37
void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
32
static void bdrv_qed_attach_aio_context(BlockDriverState *bs,
38
{
39
AioContext *ctx = qemu_get_current_aio_context();
40
- CoSleepCB sleep_cb = {
41
- .co = qemu_coroutine_self(),
42
- };
43
+ QEMUTimer *ts;
44
+ Coroutine *co = qemu_coroutine_self();
45
46
- const char *scheduled = atomic_cmpxchg(&sleep_cb.co->scheduled, NULL,
47
- __func__);
48
+ const char *scheduled = atomic_cmpxchg(&co->scheduled, NULL, __func__);
49
if (scheduled) {
50
fprintf(stderr,
51
"%s: Co-routine was already scheduled in '%s'\n",
52
__func__, scheduled);
53
abort();
54
}
55
- sleep_cb.ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &sleep_cb);
56
- timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns);
57
+ ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, co);
58
+ timer_mod(ts, qemu_clock_get_ns(type) + ns);
59
qemu_coroutine_yield();
60
- timer_del(sleep_cb.ts);
61
- timer_free(sleep_cb.ts);
62
+ timer_del(ts);
63
+ timer_free(ts);
64
}
65
--
33
--
66
2.20.1
34
2.48.1
67
68
diff view generated by jsdifflib