All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Lukas Straub <lukasstraub2@web.de>,
	"open list:Network Block Dev..." <qemu-block@nongnu.org>,
	Hanna Reitz <hreitz@redhat.com>,
	Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>,
	Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [PULL 04/13] nbd: safeguard against waking up invalid coroutine
Date: Tue, 26 Apr 2022 15:15:05 -0500	[thread overview]
Message-ID: <20220426201514.170410-5-eblake@redhat.com> (raw)
In-Reply-To: <20220426201514.170410-1-eblake@redhat.com>

From: Paolo Bonzini <pbonzini@redhat.com>

The .reply_possible field of s->requests is never set to false.  This is
not a problem as it is only a safeguard to detect protocol errors,
but it's sloppy.  In fact, the field is actually not necessary at all,
because .coroutine is set to NULL in NBD_FOREACH_REPLY_CHUNK after
receiving the last chunk.  Thus, replace .reply_possible with .coroutine
and move the check before deciding the fate of this request.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-2-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/nbd.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 567872ac5338..1f9716094924 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -58,7 +58,6 @@ typedef struct {
     Coroutine *coroutine;
     uint64_t offset;        /* original offset of the request */
     bool receiving;         /* sleeping in the yield in nbd_receive_replies */
-    bool reply_possible;    /* reply header not yet received */
 } NBDClientRequest;

 typedef enum NBDClientState {
@@ -454,15 +453,15 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
             nbd_channel_error(s, -EINVAL);
             return -EINVAL;
         }
-        if (s->reply.handle == handle) {
-            /* We are done */
-            return 0;
-        }
         ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
-        if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].reply_possible) {
+        if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].coroutine) {
             nbd_channel_error(s, -EINVAL);
             return -EINVAL;
         }
+        if (s->reply.handle == handle) {
+            /* We are done */
+            return 0;
+        }
         nbd_recv_coroutine_wake_one(&s->requests[ind2]);
     }
 }
@@ -505,7 +504,6 @@ static int nbd_co_send_request(BlockDriverState *bs,
     s->requests[i].coroutine = qemu_coroutine_self();
     s->requests[i].offset = request->from;
     s->requests[i].receiving = false;
-    s->requests[i].reply_possible = true;

     request->handle = INDEX_TO_HANDLE(s, i);

-- 
2.35.1



  parent reply	other threads:[~2022-04-26 20:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-26 20:15 [PULL 00/13] NBD patches through 2022-04-26 Eric Blake
2022-04-26 20:15 ` [PULL 01/13] qapi: rename BlockDirtyBitmapMergeSource to BlockDirtyBitmapOrStr Eric Blake
2022-04-26 20:15 ` [PULL 02/13] qapi: nbd-export: allow select bitmaps by node/name pair Eric Blake
2022-04-26 20:15 ` [PULL 03/13] iotests/223: check new possibility of exporting bitmaps by node/name Eric Blake
2022-04-26 20:15 ` Eric Blake [this message]
2022-04-26 20:15 ` [PULL 05/13] nbd: mark more coroutine_fns Eric Blake
2022-04-26 20:15 ` [PULL 06/13] nbd: remove peppering of nbd_client_connected Eric Blake
2022-05-12 16:16   ` Peter Maydell
2022-05-13 20:42     ` Eric Blake
2022-04-26 20:15 ` [PULL 07/13] nbd: keep send_mutex/free_sema handling outside nbd_co_do_establish_connection Eric Blake
2022-04-26 20:15 ` [PULL 08/13] nbd: use a QemuMutex to synchronize yanking, reconnection and coroutines Eric Blake
2022-04-26 20:15 ` [PULL 09/13] nbd: code motion and function renaming Eric Blake
2022-04-26 20:15 ` [PULL 10/13] nbd: move s->state under requests_lock Eric Blake
2022-04-26 20:15 ` [PULL 11/13] nbd: take receive_mutex when reading requests[].receiving Eric Blake
2022-04-26 20:15 ` [PULL 12/13] nbd: document what is protected by the CoMutexes Eric Blake
2022-04-26 20:15 ` [PULL 13/13] qemu-nbd: Pass max connections to blockdev layer Eric Blake
2022-04-27  0:36 ` [PULL 00/13] NBD patches through 2022-04-26 Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220426201514.170410-5-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lukasstraub2@web.de \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=v.sementsov-og@mail.ru \
    --cc=vsementsov@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.