Commit 75b0a993da for qemu.org

commit 75b0a993dacbd5f8579e8f8c228f9bae80541e42
Author: Denis V. Lunev <den@openvz.org>
Date:   Thu Aug 20 13:52:27 2026 +0200

    block/nbd: never index requests[] with an unchecked cookie

    Cookies are converted into indices of s->requests[] in several places
    and the result is used right away, without any check:

        int i = COOKIE_TO_INDEX(cookie);
        ...
        return nbd_co_receive_offset_data_payload(s, s->requests[i].offset,

    COOKIE_TO_INDEX() subtracts one, so a zero cookie becomes an index of
    -1 and the access lands in front of the array. This is undefined and,
    depending on the type of the index and on what the compiler has put
    there, it can as well pass silently: at the site above i is signed, so
    even a plain i < MAX_NBD_REQUESTS check would happily let -1 through.

    Route every conversion through a helper which hands out the slot only
    for a cookie in range and owned by a request in flight, so that the
    check can not be forgotten again. The helper reports through an Error,
    as the one cookie which is not ours to trust, the one taken from the
    wire in nbd_receive_replies(), is a protocol error rather than an
    internal inconsistency and has to stay a channel error. Every other
    cookie is one we have issued and still own, so a bad value there is a
    bug in this file, which is what &error_abort spells out.

    The cookie of the reply in flight goes through the helper as well. It
    is not an unchecked value either: it has passed the check on the wire
    path, or it has been cleared by the previous commit.

    Cc: Eric Blake <eblake@redhat.com>
    Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
    Signed-off-by: Denis V. Lunev <den@openvz.org>
    Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
    Message-ID: <20260820115228.587427-3-den@openvz.org>
    Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

diff --git a/block/nbd.c b/block/nbd.c
index d9b776283f..d0a7097034 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -136,6 +136,19 @@ static void nbd_clear_bdrvstate(BlockDriverState *bs)
     s->x_dirty_bitmap = NULL;
 }

+static NBDClientRequest *nbd_request_by_cookie(BDRVNBDState *s, uint64_t cookie,
+                                               Error **errp)
+{
+    uint64_t ind = COOKIE_TO_INDEX(cookie);
+
+    if (ind >= MAX_NBD_REQUESTS || !s->requests[ind].coroutine) {
+        error_setg(errp, "unexpected cookie value");
+        return NULL;
+    }
+
+    return &s->requests[ind];
+}
+
 /* Called with s->receive_mutex taken.  */
 static bool coroutine_fn nbd_recv_coroutine_wake_one(NBDClientRequest *req)
 {
@@ -422,7 +435,8 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie,
                                             Error **errp)
 {
     int ret;
-    uint64_t ind = COOKIE_TO_INDEX(cookie), ind2;
+    NBDClientRequest *req = nbd_request_by_cookie(s, cookie, &error_abort);
+    NBDClientRequest *owner;
     QEMU_LOCK_GUARD(&s->receive_mutex);

     while (true) {
@@ -437,10 +451,10 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie,
              * woken by whoever set s->reply.cookie (or never wait in this
              * yield). So, we should not wake it here.
              */
-            ind2 = COOKIE_TO_INDEX(s->reply.cookie);
-            assert(!s->requests[ind2].receiving);
+            owner = nbd_request_by_cookie(s, s->reply.cookie, &error_abort);
+            assert(!owner->receiving);

-            s->requests[ind].receiving = true;
+            req->receiving = true;
             qemu_co_mutex_unlock(&s->receive_mutex);

             qemu_coroutine_yield();
@@ -454,7 +468,7 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie,
              */

             qemu_co_mutex_lock(&s->receive_mutex);
-            assert(!s->requests[ind].receiving);
+            assert(!req->receiving);
             continue;
         }

@@ -474,17 +488,16 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie,
             error_setg(errp, "unexpected structured reply");
             goto err;
         }
-        ind2 = COOKIE_TO_INDEX(s->reply.cookie);
-        if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].coroutine) {
+        owner = nbd_request_by_cookie(s, s->reply.cookie, errp);
+        if (!owner) {
             ret = -EINVAL;
-            error_setg(errp, "unexpected cookie value");
             goto err;
         }
         if (s->reply.cookie == cookie) {
             /* We are done */
             return 0;
         }
-        nbd_recv_coroutine_wake_one(&s->requests[ind2]);
+        nbd_recv_coroutine_wake_one(owner);
     }

 err:
@@ -861,7 +874,6 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 {
     ERRP_GUARD();
     int ret;
-    int i = COOKIE_TO_INDEX(cookie);
     void *local_payload = NULL;
     NBDStructuredReplyChunk *chunk;

@@ -919,8 +931,9 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
             return -EINVAL;
         }

-        return nbd_co_receive_offset_data_payload(s, s->requests[i].offset,
-                                                  qiov, errp);
+        return nbd_co_receive_offset_data_payload(
+                s, nbd_request_by_cookie(s, cookie, &error_abort)->offset,
+                qiov, errp);
     }

     if (nbd_reply_type_is_error(chunk->type)) {
@@ -1067,7 +1080,7 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,

 break_loop:
     qemu_mutex_lock(&s->requests_lock);
-    s->requests[COOKIE_TO_INDEX(cookie)].coroutine = NULL;
+    nbd_request_by_cookie(s, cookie, &error_abort)->coroutine = NULL;
     s->in_flight--;
     qemu_co_queue_next(&s->free_sema);
     qemu_mutex_unlock(&s->requests_lock);