From d7a0e8fc0484ef18c25e7396baea07dfd6520a30 Mon Sep 17 00:00:00 2001

From: John Snow <jsnow@redhat.com>

Date: Fri, 22 Mar 2019 03:22:20 +0100

Subject: [PATCH 053/163] nbd/server: fix bitmap export

RH-Author: John Snow <jsnow@redhat.com>

Message-id: <20190322032241.8111-8-jsnow@redhat.com>

Patchwork-id: 85094

O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 07/28] nbd/server: fix bitmap export

Bugzilla: 1691563

RH-Acked-by: Max Reitz <mreitz@redhat.com>

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>

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

bitmap_to_extents function is broken: it switches dirty variable after

every iteration, however it can process only part of dirty (or zero)

area during one iteration in case when this area is too large for one

extent.

Fortunately, the bug doesn't produce wrong extent flags: it just inserts

a zero-length extent between sequential extents representing large dirty

(or zero) area. However, zero-length extents are forbidden by the NBD

protocol. So, a careful client should consider such a reply as a server

fault, while a less-careful will likely ignore zero-length extents.

The bug can only be triggered by a client that requests block status

for nearly 4G at once (a request of 4G and larger is impossible per

the protocol, and requests smaller than 4G less the bitmap granularity

cause the loop to quit iterating rather than revisit the tail of the

large area); it also cannot trigger if the client used the

NBD_CMD_FLAG_REQ_ONE flag.  Since qemu 3.0 as client (using the

x-dirty-bitmap extension) always passes the flag, it is immune; and

we are not aware of other open-source clients that know how to request

qemu:dirty-bitmap:FOO contexts.  Clients that want to avoid the bug

could cap block status requests to a smaller length, such as 2G or 3G.

Fix this by more careful handling of dirty variable.

Bug was introduced in 3d068aff16

 "nbd/server: implement dirty bitmap export", with the whole function.

and is present in v3.0.0 release.

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

Message-Id: <20180914165116.23182-1-vsementsov@virtuozzo.com>

CC: qemu-stable@nongnu.org

Reviewed-by: Eric Blake <eblake@redhat.com>

[eblake: improved commit message]

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 6545916d528de7a6b784f4d10e7b236b30bfaced)

Signed-off-by: John Snow <jsnow@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 nbd/server.c | 5 ++++-

 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/nbd/server.c b/nbd/server.c

index 0ab0dbd..a8ddc4a 100644

--- a/nbd/server.c

+++ b/nbd/server.c

@@ -1951,6 +1951,8 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,

     assert(begin < overall_end && nb_extents);

     while (begin < overall_end && i < nb_extents) {

+        bool next_dirty = !dirty;

+

         if (dirty) {

             end = bdrv_dirty_bitmap_next_zero(bitmap, begin, UINT64_MAX);

         } else {

@@ -1962,6 +1964,7 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,

             end = MIN(bdrv_dirty_bitmap_size(bitmap),

                       begin + UINT32_MAX + 1 -

                       bdrv_dirty_bitmap_granularity(bitmap));

+            next_dirty = dirty;

         }

         if (dont_fragment && end > overall_end) {

             end = overall_end;

@@ -1971,7 +1974,7 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,

         extents[i].flags = cpu_to_be32(dirty ? NBD_STATE_DIRTY : 0);

         i++;

         begin = end;

-        dirty = !dirty;

+        dirty = next_dirty;

     }

     bdrv_dirty_iter_free(it);

-- 

1.8.3.1