OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

block/nbd: ensure ->connection_thread is always valid

Browse Source 
Simplify lifetime management of BDRVNBDState->connect_thread by
delaying the possible cleanup of it until the BDRVNBDState itself goes
away.

This also reverts
 0267101af6 "block/nbd: fix possible use after free of s->connect_thread"
as now s->connect_thread can't be cleared until the very end.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
 [vsementsov: rebase, revert 0267101af6 changes]
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 [eblake: tweak comment]
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-8-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Roman Kagan 2021-06-10 13:07:37 +03:00 committed by Eric Blake
parent 6cc702beac
commit e8b35bf5dc
1 changed files with 21 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
block/nbd.c
View File

@ -144,17 +144,31 @@ typedef struct BDRVNBDState {
NBDConnectThread *connect_thread;
} BDRVNBDState;
static void nbd_free_connect_thread(NBDConnectThread *thr);
static int nbd_establish_connection(BlockDriverState *bs, SocketAddress *saddr,
Error **errp);
static int nbd_co_establish_connection(BlockDriverState *bs, Error **errp);
static void nbd_co_establish_connection_cancel(BlockDriverState *bs,
bool detach);
static void nbd_co_establish_connection_cancel(BlockDriverState *bs);
static int nbd_client_handshake(BlockDriverState *bs, Error **errp);
static void nbd_yank(void *opaque);
static void nbd_clear_bdrvstate(BlockDriverState *bs)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
NBDConnectThread *thr = s->connect_thread;
bool thr_running;
qemu_mutex_lock(&thr->mutex);
thr_running = thr->state == CONNECT_THREAD_RUNNING;
if (thr_running) {
thr->state = CONNECT_THREAD_RUNNING_DETACHED;
}
qemu_mutex_unlock(&thr->mutex);
/* the runaway thread will clean up itself */
if (!thr_running) {
nbd_free_connect_thread(thr);
}
yank_unregister_instance(BLOCKDEV_YANK_INSTANCE(bs->node_name));
@ -295,7 +309,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
s->drained = true;
qemu_co_sleep_wake(&s->reconnect_sleep);
nbd_co_establish_connection_cancel(bs, false);
nbd_co_establish_connection_cancel(bs);
reconnect_delay_timer_del(s);
@ -333,7 +347,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
s->state = NBD_CLIENT_QUIT;
if (s->connection_co) {
qemu_co_sleep_wake(&s->reconnect_sleep);
nbd_co_establish_connection_cancel(bs, true);
nbd_co_establish_connection_cancel(bs);
}
if (qemu_in_coroutine()) {
s->teardown_co = qemu_coroutine_self();
@ -446,11 +460,6 @@ nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
BDRVNBDState *s = bs->opaque;
NBDConnectThread *thr = s->connect_thread;
if (!thr) {
/* detached */
return -1;
}
qemu_mutex_lock(&thr->mutex);
switch (thr->state) {
@ -494,12 +503,6 @@ nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
s->wait_connect = true;
qemu_coroutine_yield();
if (!s->connect_thread) {
/* detached */
return -1;
}
assert(thr == s->connect_thread);
qemu_mutex_lock(&thr->mutex);
switch (thr->state) {
@ -547,18 +550,12 @@ nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
* nbd_co_establish_connection_cancel
* Cancel nbd_co_establish_connection asynchronously: it will finish soon, to
* allow drained section to begin.
*
* If detach is true, also cleanup the state (or if thread is running, move it
* to CONNECT_THREAD_RUNNING_DETACHED state). s->connect_thread becomes NULL if
* detach is true.
*/
static void nbd_co_establish_connection_cancel(BlockDriverState *bs,
bool detach)
static void nbd_co_establish_connection_cancel(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
NBDConnectThread *thr = s->connect_thread;
bool wake = false;
bool do_free = false;
qemu_mutex_lock(&thr->mutex);
@ -569,21 +566,10 @@ static void nbd_co_establish_connection_cancel(BlockDriverState *bs,
s->wait_connect = false;
wake = true;
}
if (detach) {
thr->state = CONNECT_THREAD_RUNNING_DETACHED;
s->connect_thread = NULL;
}
} else if (detach) {
do_free = true;
}
qemu_mutex_unlock(&thr->mutex);
if (do_free) {
nbd_free_connect_thread(thr);
s->connect_thread = NULL;
}
if (wake) {
aio_co_wake(s->connection_co);
}
@ -2310,6 +2296,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
nbd_init_connect_thread(s);
/*
* establish TCP connection, return error if it fails
* TODO: Configurable retry-until-timeout behaviour.
@ -2326,8 +2314,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
/* successfully connected */
s->state = NBD_CLIENT_CONNECTED;
nbd_init_connect_thread(s);
s->connection_co = qemu_coroutine_create(nbd_connection_entry, s);
bdrv_inc_in_flight(bs);
aio_co_schedule(bdrv_get_aio_context(bs), s->connection_co);