[Libguestfs] [nbdkit PATCH 2/2] cache, cow: Round size down

Eric Blake
2019-05-16T02:27:40Z

The blocksize filter already rounds sizes down, to avoid having to
special-case a partial action at the end of an unaligned file.  But
our current behavior for cache and cow is not similarly careful, and
can end up causing errors by passing a too-large count to the
plugin. The bulk of our plugins are somewhat safe, in that they either
fail the pread (for example, the file plugin gets a short read due to
EOF) or populate the tail of the buffer with other data (which the
client will never see, because we at least clamp user requests), so
that the cache is only populated with untainted data. But it's hard to
argue whether there might be another plugin out there that might abort
because we violated our documented constraints, or which might leave
the tail of the buffer unpopulated on a short read while still
reporting success. Thus, I cannot rule out that our cache file might
contain uninitialized data from the heap, or even leak it back out to
the file system through the plugin or the NBD client.

The quickest fix is to just always round down, and then do a better
job of documenting how the user can round up as needed.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 filters/blocksize/nbdkit-blocksize-filter.pod | 14 ++++++++++++--
 filters/cache/nbdkit-cache-filter.pod         |  9 ++++++++-
 filters/cow/nbdkit-cow-filter.pod             |  8 ++++++++
 filters/truncate/nbdkit-truncate-filter.pod   |  9 +++++++++
 filters/cache/cache.c                         |  8 ++++++--
 filters/cow/cow.c                             |  6 +++++-
 6 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/filters/blocksize/nbdkit-blocksize-filter.pod
b/filters/blocksize/nbdkit-blocksize-filter.pod
index 3b79e28..c684f7d 100644
--- a/filters/blocksize/nbdkit-blocksize-filter.pod
+++ b/filters/blocksize/nbdkit-blocksize-filter.pod
@@ -38,7 +38,9 @@ read requests to alignment boundaries, performs
read-modify-write
 cycles for any unaligned head or tail of a write or zero request, and
 silently ignores any unaligned head or tail of a trim request.  The
 filter also truncates the plugin size down to an aligned value (as it
-cannot safely operate on the unaligned tail).
+cannot safely operate on the unaligned tail); if you need access to
+the last few bytes, you can combine the use of
+L<nbdkit-truncate-filter(1)> to round the size up instead.

 This parameter understands the suffix 'k' for 1024.

@@ -82,12 +84,20 @@ nbd plugin:

  nbdkit --filter=blocksize nbd maxdata=32M socket=/path/to/socket

+Serve a file as if it were a block device that insists on 4k
+alignment, while still allowing access to any unaligned bytes at the
+end of the file:
+
+ nbdkit --filter=blocksize --filter=truncate file /path/to/file \
+ mindata=4k round-up=4k
+
 =head1 SEE ALSO

 L<nbdkit(1)>,
 L<nbdkit-nbd-plugin(1)>,
 L<nbdkit-vddk-plugin(1)>,
-L<nbdkit-filter(3)>.
+L<nbdkit-filter(3)>,
+L<nbdkit-truncate-filter(1)>.

 =head1 AUTHORS

diff --git a/filters/cache/nbdkit-cache-filter.pod
b/filters/cache/nbdkit-cache-filter.pod
index 431a720..5993831 100644
--- a/filters/cache/nbdkit-cache-filter.pod
+++ b/filters/cache/nbdkit-cache-filter.pod
@@ -25,6 +25,12 @@ does not have effective caching, or (with
C<cache=unsafe>) to defeat
 flush requests from the client (which is unsafe and can cause data
 loss, as the name suggests).

+Note that the use of this filter rounds the image size down to a
+multiple of the caching granularity (the larger of 4096 or the
+C<f_bsize> field of L<fstatvfs(3)>), to ease the implementation. If
+you need to round the image size up instead to access the last few
+bytes, combine this filter with L<nbdkit-truncate-filter(1)>.
+
 =head1 PARAMETERS

 =over 4
@@ -136,6 +142,7 @@ environment variable before starting nbdkit.
 L<nbdkit(1)>,
 L<nbdkit-file-plugin(1)>,
 L<nbdkit-readahead-filter(1)>,
+L<nbdkit-truncate-filter(1)>,
 L<nbdkit-filter(3)>,
 L<qemu-img(1)>.

@@ -147,4 +154,4 @@ Richard W.M. Jones

 =head1 COPYRIGHT

-Copyright (C) 2018 Red Hat Inc.
+Copyright (C) 2018-2019 Red Hat Inc.
diff --git a/filters/cow/nbdkit-cow-filter.pod
b/filters/cow/nbdkit-cow-filter.pod
index f88d8ee..448f48c 100644
--- a/filters/cow/nbdkit-cow-filter.pod
+++ b/filters/cow/nbdkit-cow-filter.pod
@@ -31,6 +31,13 @@ that away.  It also allows us to create diffs, see below.
 The plugin is opened read-only (as if the I<-r> flag was passed), but
 you should B<not> pass the I<-r> flag to nbdkit.

+=item *
+
+Use of this filter rounds the image size down to a multiple of the
+caching granularity (4096), to ease the implementation. If you need to
+round the image size up instead to access the last few bytes, combine
+this filter with L<nbdkit-truncate-filter(1)>.
+
 =back

 Limitations of the filter include:
@@ -108,6 +115,7 @@ C<TMPDIR> environment variable before starting nbdkit.
 L<nbdkit(1)>,
 L<nbdkit-file-plugin(1)>,
 L<nbdkit-xz-plugin(1)>,
+L<nbdkit-truncate-filter(1)>,
 L<nbdkit-filter(3)>,
 L<qemu-img(1)>.

diff --git a/filters/truncate/nbdkit-truncate-filter.pod
b/filters/truncate/nbdkit-truncate-filter.pod
index 9eba6cd..92645c0 100644
--- a/filters/truncate/nbdkit-truncate-filter.pod
+++ b/filters/truncate/nbdkit-truncate-filter.pod
@@ -59,6 +59,12 @@ Round the size up to the next multiple of C<N> bytes.
If the size of
 the underlying plugin is already a multiple of C<N> bytes, this has no
 effect.

+This option is useful when combined with other filters (such as
+L<nbdkit-blocksize-filter(1)>, L<nbdkit-cache-filter(1)>, or
+L<nbdkit-cow-filter(1)>) that normally round down due to a larger
+granularity, in order to access the last few bytes of a file that
+would otherwise be rendered inaccessible.
+
 This parameter is optional.

 =item B<round-down=>N
@@ -76,6 +82,9 @@ This parameter is optional.
 L<nbdkit(1)>,
 L<nbdkit-file-plugin(1)>,
 L<nbdkit-filter(3)>,
+L<nbdkit-blocksize-filter(1)>,
+L<nbdkit-cache-filter(1)>,
+L<nbdkit-cow-filter(1)>,
 L<nbdkit-offset-filter(1)>,
 L<nbdkit-partition-filter(1)>.

diff --git a/filters/cache/cache.c b/filters/cache/cache.c
index 360458f..e215cac 100644
--- a/filters/cache/cache.c
+++ b/filters/cache/cache.c
@@ -60,6 +60,7 @@
 #include "reclaim.h"
 #include "isaligned.h"
 #include "minmax.h"
+#include "rounding.h"

 #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL

@@ -188,10 +189,12 @@ cache_config_complete (nbdkit_next_config_complete *next,
void *nxdata)
   return next (nxdata);
 }

-/* Get the file size and ensure the cache is the correct size. */
+/* Get the file size; round it down to cache granularity before
+ * setting cache size.
+ */
 static int64_t
 cache_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
-              void *handle)
+                void *handle)
 {
   int64_t size;
   int r;
@@ -201,6 +204,7 @@ cache_get_size (struct nbdkit_next_ops *next_ops, void
*nxdata,
     return -1;

   nbdkit_debug ("cache: underlying file size: %" PRIi64, size);
+  size = ROUND_DOWN (size, blksize);

   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
   r = blk_set_size (size);
diff --git a/filters/cow/cow.c b/filters/cow/cow.c
index 1ce5893..aa1348b 100644
--- a/filters/cow/cow.c
+++ b/filters/cow/cow.c
@@ -49,6 +49,7 @@
 #include "blk.h"
 #include "isaligned.h"
 #include "minmax.h"
+#include "rounding.h"

 #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL

@@ -80,7 +81,9 @@ cow_open (nbdkit_next_open *next, void *nxdata, int readonly)
   return NBDKIT_HANDLE_NOT_NEEDED;
 }

-/* Get the file size and ensure the overlay is the correct size. */
+/* Get the file size; round it down to overlay granularity before
+ * setting overlay size.
+ */
 static int64_t
 cow_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
               void *handle)
@@ -93,6 +96,7 @@ cow_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
     return -1;

   nbdkit_debug ("cow: underlying file size: %" PRIi64, size);
+  size = ROUND_DOWN (size, BLKSIZE);

   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
   r = blk_set_size (size);
--
2.20.1