qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/7] block/nbd: avoid touching freed connect_thread


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH 1/7] block/nbd: avoid touching freed connect_thread
Date: Mon, 15 Mar 2021 18:40:12 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

15.03.2021 09:06, Roman Kagan wrote:
When the NBD connection is being torn down, the connection thread gets
canceled and "detached", meaning it is about to get freed.

If this happens while the connection coroutine yielded waiting for the
connection thread to complete, when it resumes it may access the
invalidated connection thread data.

To prevent this, revalidate the ->connect_thread pointer in
nbd_co_establish_connection_cancel before using after the the yield.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>

Seems possible. Do you have a reproducer? Would be great to make an iotest.

---
  block/nbd.c | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index c26dc5a54f..447d176b76 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -486,6 +486,15 @@ nbd_co_establish_connection(BlockDriverState *bs, Error 
**errp)
      s->wait_connect = true;
      qemu_coroutine_yield();
+ /*
+     * If nbd_co_establish_connection_cancel had a chance to run it may have
+     * invalidated ->connect_thread.
+     */
+    thr = s->connect_thread;
+    if (!thr) {
+        return -ECONNABORTED;
+    }
+
      qemu_mutex_lock(&thr->mutex);
switch (thr->state) {


Patch looks a bit like s->connect_thread may become something other than NULL 
during the yield.. But it can't actually. Maybe it will be possible with further 
patches?

Anyway, make sense to me:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]