From 99788909d9ec36e3210cf85976fe5b18da690ddd Mon Sep 17 00:00:00 2001

From: "Richard W.M. Jones" <rjones@redhat.com>

Date: Wed, 4 Aug 2021 20:24:59 +0100

Subject: [PATCH] cache, cow: Fix data corruption in zero and trim on unaligned

 tail

Commit eb6009b092 ("cache, cow: Reduce use of bounce-buffer") first

introduced in nbdkit 1.14 added an optimization of the

read-modify-write mechanism used for unaligned heads and tails when

zeroing in the cache layer.

Unfortunately the part applied to the tail contained a mistake: It

zeroes the end of the buffer rather than the beginning.  This causes

data corruption when you use the zero or trim function with an offset

and count which is not aligned to the block size.

Although the bug has been around for years, a recent change made it

more likely to happen.  Commit c1905b0a28 ("cache, cow: Use a 64K

block size by default") increased the default block size from 4K to

64K.  Most filesystems use a 4K block size so operations like fstrim

will make 4K-aligned requests, and with a 4K block size also in the

cache or cow filter the unaligned case would never have been hit

before.

We can demonstrate the bug simply by filling a buffer with data

(100000 bytes in the example), and then trimming that data, which

ought to zero it out.

Before this commit there is data visible after the trim:

$ nbdkit --filter=cow data "0x21 * 100000" --run 'nbdsh -u $uri -c "h.trim(100000, 0)" ; nbdcopy $uri - | hexdump -C'

00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|

*

00018000  21 21 21 21 21 21 21 21  21 21 21 21 21 21 21 21  |!!!!!!!!!!!!!!!!|

*

000186a0

After this commit the trim completely clears the data:

$ nbdkit --filter=cow data "0x21 * 100000" --run 'nbdsh -u $uri -c "h.trim(100000, 0)" ; nbdcopy $uri - | hexdump -C'

00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|

*

000186a0

Thanks: Ming Xie for finding the bug

Fixes: commit eb6009b092ae642ed25f133d487dd40ef7bf70f8

(cherry picked from commit a0ae7b2158598ce48ac31706319007f716d01c87)

(cherry picked from commit c0b15574647672cb5c48178333acdd07424692ef)

---

 filters/cache/cache.c | 2 +-

 filters/cow/cow.c     | 2 +-

 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/filters/cache/cache.c b/filters/cache/cache.c

index 91dcc43d..0616cc7b 100644

--- a/filters/cache/cache.c

+++ b/filters/cache/cache.c

@@ -493,7 +493,7 @@ cache_zero (struct nbdkit_next_ops *next_ops, void *nxdata,

     ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);

     r = blk_read (next_ops, nxdata, blknum, block, err);

     if (r != -1) {

-      memset (&block[count], 0, blksize - count);

+      memset (block, 0, count);

       r = blk_write (next_ops, nxdata, blknum, block, flags, err);

     }

     if (r == -1)

diff --git a/filters/cow/cow.c b/filters/cow/cow.c

index 51ca64a4..1cfcc4e7 100644

--- a/filters/cow/cow.c

+++ b/filters/cow/cow.c

@@ -419,7 +419,7 @@ cow_zero (struct nbdkit_next_ops *next_ops, void *nxdata,

     ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);

     r = blk_read (next_ops, nxdata, blknum, block, err);

     if (r != -1) {

-      memset (&block[count], 0, BLKSIZE - count);

+      memset (block, 0, count);

       r = blk_write (blknum, block, err);

     }

     if (r == -1)

-- 

2.31.1