Libguestfs - Oct 2019 - [nbdkit PATCH 0/5] Another round of retry fixes

I still don't have .prepare/.finalize working cleanly across reopen,
but did find a nasty bug where a botched assertion means we failed to
notice reads beyond EOF in both the xz and retry filter.

Refactoring backend.c will make .finalize work easier.

Eric Blake (5):
  xz: Avoid reading beyond EOF
  retry: Check size before transactions
  tests: Test retry when get_size values change
  server: Fix backend range check
  server: Refactor to drop connection_get_handle()

 server/internal.h                    |  66 +++++++------
 server/backend.c                     |  53 ++++++----
 server/connections.c                 |   2 +-
 server/filters.c                     |  70 +++++--------
 server/plugins.c                     | 142 ++++++++++-----------------
 server/protocol-handshake-newstyle.c |   4 +-
 filters/retry/retry.c                |  23 +++++
 filters/xz/xzfile.c                  |   4 +
 tests/Makefile.am                    |   2 +
 tests/test-retry-size.sh             | 101 +++++++++++++++++++
 10 files changed, 275 insertions(+), 192 deletions(-)
 create mode 100755 tests/test-retry-size.sh

-- 
2.21.0

Check that the file is long enough before reading the header, rather
than violating assumptions of the backend that all requests have
already passed bounds checks.  Missed when converting from a plugin to
a filter.

Fixes: c879d310
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 filters/xz/xzfile.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/filters/xz/xzfile.c b/filters/xz/xzfile.c
index ee4af713..1ad010ef 100644
--- a/filters/xz/xzfile.c
+++ b/filters/xz/xzfile.c
@@ -117,6 +117,10 @@ check_header_magic (struct nbdkit_next_ops *next_ops, void
*nxdata)
   char buf[XZ_HEADER_MAGIC_LEN];
   int err;

+  if (next_ops->get_size (nxdata) < XZ_HEADER_MAGIC_LEN) {
+    nbdkit_error ("xz: file too short");
+    return false;
+  }
   if (next_ops->pread (nxdata, buf, XZ_HEADER_MAGIC_LEN, 0, 0, &err) ==
-1) {
     nbdkit_error ("xz: could not read header magic: error %d", err);
     return false;
-- 
2.21.0

Although it is unusual for a plugin to shrink size on reopen, it is
not impossible.  Failure to check our bounds could result in violating
assumptions in the plugin that all requests are in-bounds.  Note that
if the plugin gains a larger size, we merely never access the new tail
of the file (when the NBD protocol ever gains dynamic resize support,
we'll get to revisit how the retry filter fits in).

Fixes: f0f0ec49
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 filters/retry/retry.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/filters/retry/retry.c b/filters/retry/retry.c
index 840d7383..cf8f5246 100644
--- a/filters/retry/retry.c
+++ b/filters/retry/retry.c
@@ -148,6 +148,17 @@ struct retry_data {
   int delay;                    /* Seconds to wait before retrying. */
 };

+static bool
+valid_range (struct nbdkit_next_ops *next_ops, void *nxdata,
+             uint32_t count, uint64_t offset, bool is_write, int *err)
+{
+  if ((int64_t) offset + count > next_ops->get_size (nxdata)) {
+    *err = is_write ? ENOSPC : EIO;
+    return false;
+  }
+  return true;
+}
+
 static bool
 do_retry (struct retry_handle *h,
           struct retry_data *data,
@@ -204,6 +215,8 @@ retry_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
   int r;

  again:
+  if (! valid_range (next_ops, nxdata, count, offset, false, err))
+    return -1;
   r = next_ops->pread (nxdata, buf, count, offset, flags, err);
   if (r == -1 && do_retry (h, &data, next_ops, nxdata, err)) goto
again;

@@ -226,6 +239,8 @@ retry_pwrite (struct nbdkit_next_ops *next_ops, void
*nxdata,
     *err = EROFS;
     return -1;
   }
+  if (! valid_range (next_ops, nxdata, count, offset, true, err))
+    return -1;
   if (next_ops->can_write (nxdata) != 1) {
     *err = EROFS;
     r = -1;
@@ -258,6 +273,8 @@ retry_trim (struct nbdkit_next_ops *next_ops, void *nxdata,
     *err = EROFS;
     return -1;
   }
+  if (! valid_range (next_ops, nxdata, count, offset, true, err))
+    return -1;
   if (next_ops->can_trim (nxdata) != 1) {
     *err = EROFS;
     r = -1;
@@ -312,6 +329,8 @@ retry_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
     *err = EROFS;
     return -1;
   }
+  if (! valid_range (next_ops, nxdata, count, offset, true, err))
+    return -1;
   if (flags & NBDKIT_FLAG_FAST_ZERO &&
            next_ops->can_fast_zero (nxdata) != 1) {
     *err = EOPNOTSUPP;
@@ -347,6 +366,8 @@ retry_extents (struct nbdkit_next_ops *next_ops, void
*nxdata,
   size_t i;

  again:
+  if (! valid_range (next_ops, nxdata, count, offset, false, err))
+    return -1;
   if (next_ops->can_extents (nxdata) != 1) {
     *err = EIO;
     r = -1;
@@ -390,6 +411,8 @@ retry_cache (struct nbdkit_next_ops *next_ops, void *nxdata,
   int r;

  again:
+  if (! valid_range (next_ops, nxdata, count, offset, false, err))
+    return -1;
   if (next_ops->can_cache (nxdata) <= NBDKIT_CACHE_NONE) {
     *err = EIO;
     r = -1;
-- 
2.21.0

Someday, we want to enhance the NBD protocol to support resizes.  But
until then, it's worth testing that we don't crash if a reopen happens
to result in a smaller image.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile.am        |   2 +
 tests/test-retry-size.sh | 101 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100755 tests/test-retry-size.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9d95d567..558ea86f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -121,6 +121,7 @@ EXTRA_DIST = \
 	test-readahead-copy.sh \
 	test-retry.sh \
 	test-retry-extents.sh \
+	test-retry-size.sh \
 	test-retry-readonly.sh \
 	test-retry-reopen-fail.sh \
 	test-retry-zero-flags.sh \
@@ -1062,6 +1063,7 @@ TESTS += \
 	test-retry.sh \
 	test-retry-readonly.sh \
 	test-retry-extents.sh \
+	test-retry-size.sh \
 	test-retry-reopen-fail.sh \
 	test-retry-zero-flags.sh \
 	$(NULL)
diff --git a/tests/test-retry-size.sh b/tests/test-retry-size.sh
new file mode 100755
index 00000000..eed7aef1
--- /dev/null
+++ b/tests/test-retry-size.sh
@@ -0,0 +1,101 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2019 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+source ./functions.sh
+set -e
+set -x
+
+requires qemu-io --version
+
+files="retry-size-open-count"
+rm -f $files
+cleanup_fn rm -f $files
+
+touch retry-size-open-count
+start_t=$SECONDS
+
+# Create a custom plugin which will test retrying.
+st=0
+nbdkit -v -U - \
+       sh - \
+       --filter=retry retry-delay=1 \
+       --run 'qemu-io -f raw -r $nbd \
+    -c "r 512 512" -c "r 0 512"' <<'EOF'
|| st=$?
+#!/usr/bin/env bash
+case "$1" in
+    open)
+        # Count how many times the connection is (re-)opened.
+        read i < retry-size-open-count
+        echo $((i+1)) > retry-size-open-count
+        ;;
+    get_size)
+        # Start big, then clamp down to reflect unreadable sector
+        read i < retry-size-open-count
+        if [ $i = 1 ]; then
+            echo 1024
+        else
+            echo 512
+        fi
+        ;;
+    pread)
+        # Fail for anything beyond 512
+        if [ $(( $3 + $4 )) -gt 512 ]; then
+            echo "EINVAL too far into file" >&2
+            exit 1
+        fi
+        dd if=/dev/zero count=$3 iflag=count_bytes
+        ;;
+    *) exit 2 ;;
+esac
+EOF
+
+# In this test we should see the following:
+# open reports size 1024
+# first pread FAILS
+# retry and wait 1 seconds
+# open reports size 512
+# first pread FAILS immediately for being out of bounds
+# second pread succeeds
+
+# The minimum time for the test should be 1 = 1 second.
+end_t=$SECONDS
+if [ $((end_t - start_t)) -lt 1 ]; then
+    echo "$0: test ran too quickly"
+    exit 1
+fi
+
+# Check the handle was opened 2 times (first open + one reopen).
+read open_count < retry-size-open-count
+if [ $open_count -ne 2 ]; then
+    echo "$0: open-count ($open_count) != 2"
+    exit 1
+fi
-- 
2.21.0

h->exportsize is unsigned, so the assertion as written was a no-op.
It was intended to catch cases where exportsize was left at -1 from
prior to .open (which, until recently, could happen in both the xz and
retry filters).

Fixes: dbdec62725
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/backend.c b/server/backend.c
index 5cbbe2c1..f02f4745 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -229,7 +229,7 @@ backend_valid_range (struct backend *b, struct connection
*conn,
 {
   struct b_conn_handle *h = &conn->handles[b->i];

-  assert (h->exportsize >= 0); /* Guaranteed by negotiation phase */
+  assert (h->exportsize <= INT64_MAX); /* Guaranteed by negotiation phase
*/
   return count > 0 && offset <= h->exportsize &&
     offset + count <= h->exportsize;
 }
-- 
2.21.0

Add backend_finalize() as one remaining function in backend.c, at
which point all backend functions that use a handle now also have a
wrapper.  As such, it's easier to change the signature to have the
backend hand the handle to filters.c or plugins.c, and drop the
connection_get_handle() function entirely.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/internal.h                    |  66 +++++++------
 server/backend.c                     |  51 ++++++----
 server/connections.c                 |   2 +-
 server/filters.c                     |  70 +++++--------
 server/plugins.c                     | 142 ++++++++++-----------------
 server/protocol-handshake-newstyle.c |   4 +-
 6 files changed, 144 insertions(+), 191 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index 97e417f9..d4b57419 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -214,8 +214,6 @@ struct connection {
 };

 extern int handle_single_connection (int sockin, int sockout);
-extern void *connection_get_handle (struct connection *conn, size_t i)
-  __attribute__((__nonnull__ (1)));
 extern int connection_get_status (struct connection *conn)
   __attribute__((__nonnull__ (1)));
 extern int connection_set_status (struct connection *conn, int value)
@@ -306,37 +304,43 @@ struct backend {
   void (*config_complete) (struct backend *);
   const char *(*magic_config_key) (struct backend *);
   int (*open) (struct backend *, struct connection *conn, int readonly);
-  int (*prepare) (struct backend *, struct connection *conn, int readonly);
-  int (*finalize) (struct backend *, struct connection *conn);
-  void (*close) (struct backend *, struct connection *conn);
+  int (*prepare) (struct backend *, struct connection *conn, void *handle,
+                  int readonly);
+  int (*finalize) (struct backend *, struct connection *conn, void *handle);
+  void (*close) (struct backend *, struct connection *conn, void *handle);

-  int64_t (*get_size) (struct backend *, struct connection *conn);
-  int (*can_write) (struct backend *, struct connection *conn);
-  int (*can_flush) (struct backend *, struct connection *conn);
-  int (*is_rotational) (struct backend *, struct connection *conn);
-  int (*can_trim) (struct backend *, struct connection *conn);
-  int (*can_zero) (struct backend *, struct connection *conn);
-  int (*can_fast_zero) (struct backend *, struct connection *conn);
-  int (*can_extents) (struct backend *, struct connection *conn);
-  int (*can_fua) (struct backend *, struct connection *conn);
-  int (*can_multi_conn) (struct backend *, struct connection *conn);
-  int (*can_cache) (struct backend *, struct connection *conn);
+  int64_t (*get_size) (struct backend *, struct connection *conn, void
*handle);
+  int (*can_write) (struct backend *, struct connection *conn, void *handle);
+  int (*can_flush) (struct backend *, struct connection *conn, void *handle);
+  int (*is_rotational) (struct backend *, struct connection *conn,
+                        void *handle);
+  int (*can_trim) (struct backend *, struct connection *conn, void *handle);
+  int (*can_zero) (struct backend *, struct connection *conn, void *handle);
+  int (*can_fast_zero) (struct backend *, struct connection *conn,
+                        void *handle);
+  int (*can_extents) (struct backend *, struct connection *conn, void *handle);
+  int (*can_fua) (struct backend *, struct connection *conn, void *handle);
+  int (*can_multi_conn) (struct backend *, struct connection *conn,
+                         void *handle);
+  int (*can_cache) (struct backend *, struct connection *conn, void *handle);

-  int (*pread) (struct backend *, struct connection *conn, void *buf,
-                uint32_t count, uint64_t offset, uint32_t flags, int *err);
-  int (*pwrite) (struct backend *, struct connection *conn, const void *buf,
-                 uint32_t count, uint64_t offset, uint32_t flags, int *err);
-  int (*flush) (struct backend *, struct connection *conn, uint32_t flags,
-                int *err);
-  int (*trim) (struct backend *, struct connection *conn, uint32_t count,
-               uint64_t offset, uint32_t flags, int *err);
-  int (*zero) (struct backend *, struct connection *conn, uint32_t count,
-               uint64_t offset, uint32_t flags, int *err);
-  int (*extents) (struct backend *, struct connection *conn, uint32_t count,
-                  uint64_t offset, uint32_t flags,
+  int (*pread) (struct backend *, struct connection *conn, void *handle,
+                void *buf, uint32_t count, uint64_t offset,
+                uint32_t flags, int *err);
+  int (*pwrite) (struct backend *, struct connection *conn, void *handle,
+                 const void *buf, uint32_t count, uint64_t offset,
+                 uint32_t flags, int *err);
+  int (*flush) (struct backend *, struct connection *conn, void *handle,
+                uint32_t flags, int *err);
+  int (*trim) (struct backend *, struct connection *conn, void *handle,
+               uint32_t count, uint64_t offset, uint32_t flags, int *err);
+  int (*zero) (struct backend *, struct connection *conn, void *handle,
+               uint32_t count, uint64_t offset, uint32_t flags, int *err);
+  int (*extents) (struct backend *, struct connection *conn, void *handle,
+                  uint32_t count, uint64_t offset, uint32_t flags,
                   struct nbdkit_extents *extents, int *err);
-  int (*cache) (struct backend *, struct connection *conn, uint32_t count,
-                uint64_t offset, uint32_t flags, int *err);
+  int (*cache) (struct backend *, struct connection *conn, void *handle,
+                uint32_t count, uint64_t offset, uint32_t flags, int *err);
 };

 extern void backend_init (struct backend *b, struct backend *next, size_t
index,
@@ -353,6 +357,8 @@ extern int backend_open (struct backend *b, struct
connection *conn,
   __attribute__((__nonnull__ (1, 2)));
 extern int backend_prepare (struct backend *b, struct connection *conn)
   __attribute__((__nonnull__ (1, 2)));
+extern int backend_finalize (struct backend *b, struct connection *conn)
+  __attribute__((__nonnull__ (1, 2)));
 extern void backend_close (struct backend *b, struct connection *conn)
   __attribute__((__nonnull__ (1, 2)));
 extern void backend_set_handle (struct backend *b, struct connection *conn,
diff --git a/server/backend.c b/server/backend.c
index f02f4745..ace61c29 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -201,7 +201,17 @@ backend_prepare (struct backend *b, struct connection
*conn)

   debug ("%s: prepare readonly=%d", b->name, h->can_write ==
0);

-  return b->prepare (b, conn, h->can_write == 0);
+  return b->prepare (b, conn, h->handle, h->can_write == 0);
+}
+
+int
+backend_finalize (struct backend *b, struct connection *conn)
+{
+  struct b_conn_handle *h = &conn->handles[b->i];
+
+  debug ("%s: finalize", b->name);
+
+  return b->finalize (b, conn, h->handle);
 }

 void
@@ -211,7 +221,7 @@ backend_close (struct backend *b, struct connection *conn)

   debug ("%s: close", b->name);

-  b->close (b, conn);
+  b->close (b, conn, h->handle);
   reset_b_conn_handle (h);
 }

@@ -256,7 +266,7 @@ backend_get_size (struct backend *b, struct connection
*conn)
   debug ("%s: get_size", b->name);

   if (h->exportsize == -1)
-    h->exportsize = b->get_size (b, conn);
+    h->exportsize = b->get_size (b, conn, h->handle);
   return h->exportsize;
 }

@@ -268,7 +278,7 @@ backend_can_write (struct backend *b, struct connection
*conn)
   debug ("%s: can_write", b->name);

   if (h->can_write == -1)
-    h->can_write = b->can_write (b, conn);
+    h->can_write = b->can_write (b, conn, h->handle);
   return h->can_write;
 }

@@ -280,7 +290,7 @@ backend_can_flush (struct backend *b, struct connection
*conn)
   debug ("%s: can_flush", b->name);

   if (h->can_flush == -1)
-    h->can_flush = b->can_flush (b, conn);
+    h->can_flush = b->can_flush (b, conn, h->handle);
   return h->can_flush;
 }

@@ -292,7 +302,7 @@ backend_is_rotational (struct backend *b, struct connection
*conn)
   debug ("%s: is_rotational", b->name);

   if (h->is_rotational == -1)
-    h->is_rotational = b->is_rotational (b, conn);
+    h->is_rotational = b->is_rotational (b, conn, h->handle);
   return h->is_rotational;
 }

@@ -310,7 +320,7 @@ backend_can_trim (struct backend *b, struct connection
*conn)
       h->can_trim = 0;
       return r;
     }
-    h->can_trim = b->can_trim (b, conn);
+    h->can_trim = b->can_trim (b, conn, h->handle);
   }
   return h->can_trim;
 }
@@ -329,7 +339,7 @@ backend_can_zero (struct backend *b, struct connection
*conn)
       h->can_zero = NBDKIT_ZERO_NONE;
       return r; /* Relies on 0 == NBDKIT_ZERO_NONE */
     }
-    h->can_zero = b->can_zero (b, conn);
+    h->can_zero = b->can_zero (b, conn, h->handle);
   }
   return h->can_zero;
 }
@@ -348,7 +358,7 @@ backend_can_fast_zero (struct backend *b, struct connection
*conn)
       h->can_fast_zero = 0;
       return r; /* Relies on 0 == NBDKIT_ZERO_NONE */
     }
-    h->can_fast_zero = b->can_fast_zero (b, conn);
+    h->can_fast_zero = b->can_fast_zero (b, conn, h->handle);
   }
   return h->can_fast_zero;
 }
@@ -361,7 +371,7 @@ backend_can_extents (struct backend *b, struct connection
*conn)
   debug ("%s: can_extents", b->name);

   if (h->can_extents == -1)
-    h->can_extents = b->can_extents (b, conn);
+    h->can_extents = b->can_extents (b, conn, h->handle);
   return h->can_extents;
 }

@@ -379,7 +389,7 @@ backend_can_fua (struct backend *b, struct connection *conn)
       h->can_fua = NBDKIT_FUA_NONE;
       return r; /* Relies on 0 == NBDKIT_FUA_NONE */
     }
-    h->can_fua = b->can_fua (b, conn);
+    h->can_fua = b->can_fua (b, conn, h->handle);
   }
   return h->can_fua;
 }
@@ -392,7 +402,7 @@ backend_can_multi_conn (struct backend *b, struct connection
*conn)
   debug ("%s: can_multi_conn", b->name);

   if (h->can_multi_conn == -1)
-    h->can_multi_conn = b->can_multi_conn (b, conn);
+    h->can_multi_conn = b->can_multi_conn (b, conn, h->handle);
   return h->can_multi_conn;
 }

@@ -404,7 +414,7 @@ backend_can_cache (struct backend *b, struct connection
*conn)
   debug ("%s: can_cache", b->name);

   if (h->can_cache == -1)
-    h->can_cache = b->can_cache (b, conn);
+    h->can_cache = b->can_cache (b, conn, h->handle);
   return h->can_cache;
 }

@@ -413,6 +423,7 @@ backend_pread (struct backend *b, struct connection *conn,
                void *buf, uint32_t count, uint64_t offset,
                uint32_t flags, int *err)
 {
+  struct b_conn_handle *h = &conn->handles[b->i];
   int r;

   assert (backend_valid_range (b, conn, offset, count));
@@ -420,7 +431,7 @@ backend_pread (struct backend *b, struct connection *conn,
   debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64,
          b->name, count, offset);

-  r = b->pread (b, conn, buf, count, offset, flags, err);
+  r = b->pread (b, conn, h->handle, buf, count, offset, flags, err);
   if (r == -1)
     assert (*err);
   return r;
@@ -443,7 +454,7 @@ backend_pwrite (struct backend *b, struct connection *conn,
   debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64
" fua=%d",
          b->name, count, offset, fua);

-  r = b->pwrite (b, conn, buf, count, offset, flags, err);
+  r = b->pwrite (b, conn, h->handle, buf, count, offset, flags, err);
   if (r == -1)
     assert (*err);
   return r;
@@ -460,7 +471,7 @@ backend_flush (struct backend *b, struct connection *conn,
   assert (flags == 0);
   debug ("%s: flush", b->name);

-  r = b->flush (b, conn, flags, err);
+  r = b->flush (b, conn, h->handle, flags, err);
   if (r == -1)
     assert (*err);
   return r;
@@ -484,7 +495,7 @@ backend_trim (struct backend *b, struct connection *conn,
   debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64
" fua=%d",
          b->name, count, offset, fua);

-  r = b->trim (b, conn, count, offset, flags, err);
+  r = b->trim (b, conn, h->handle, count, offset, flags, err);
   if (r == -1)
     assert (*err);
   return r;
@@ -513,7 +524,7 @@ backend_zero (struct backend *b, struct connection *conn,
          " may_trim=%d fua=%d fast=%d",
          b->name, count, offset, !!(flags & NBDKIT_FLAG_MAY_TRIM), fua,
fast);

-  r = b->zero (b, conn, count, offset, flags, err);
+  r = b->zero (b, conn, h->handle, count, offset, flags, err);
   if (r == -1) {
     assert (*err);
     if (!fast)
@@ -545,7 +556,7 @@ backend_extents (struct backend *b, struct connection *conn,
       *err = errno;
     return r;
   }
-  r = b->extents (b, conn, count, offset, flags, extents, err);
+  r = b->extents (b, conn, h->handle, count, offset, flags, extents,
err);
   if (r == -1)
     assert (*err);
   return r;
@@ -577,7 +588,7 @@ backend_cache (struct backend *b, struct connection *conn,
     }
     return 0;
   }
-  r = b->cache (b, conn, count, offset, flags, err);
+  r = b->cache (b, conn, h->handle, count, offset, flags, err);
   if (r == -1)
     assert (*err);
   return r;
diff --git a/server/connections.c b/server/connections.c
index df5e09af..c6fa232f 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -217,7 +217,7 @@ _handle_single_connection (int sockin, int sockout)
   /* Finalize (for filters), called just before close. */
   lock_request (conn);
   if (backend)
-    r = backend->finalize (backend, conn);
+    r = backend_finalize (backend, conn);
   else
     r = 0;
   unlock_request (conn);
diff --git a/server/filters.c b/server/filters.c
index 37e8b51e..0635f0bb 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -223,10 +223,9 @@ filter_open (struct backend *b, struct connection *conn,
int readonly)
 }

 static void
-filter_close (struct backend *b, struct connection *conn)
+filter_close (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);

   /* outer-to-inner order, opposite .open */
   if (handle && f->filter.close)
@@ -406,10 +405,10 @@ static struct nbdkit_next_ops next_ops = {
 };

 static int
-filter_prepare (struct backend *b, struct connection *conn, int readonly)
+filter_prepare (struct backend *b, struct connection *conn, void *handle,
+                int readonly)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   /* Call these in order starting from the filter closest to the
@@ -426,14 +425,11 @@ filter_prepare (struct backend *b, struct connection
*conn, int readonly)
 }

 static int
-filter_finalize (struct backend *b, struct connection *conn)
+filter_finalize (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

-  debug ("%s: finalize", b->name);
-
   /* Call these in reverse order to .prepare above, starting from the
    * filter furthest away from the plugin, and matching .close order.
    */
@@ -441,14 +437,13 @@ filter_finalize (struct backend *b, struct connection
*conn)
       f->filter.finalize (&next_ops, &nxdata, handle) == -1)
     return -1;

-  return b->next->finalize (b->next, conn);
+  return backend_finalize (b->next, conn);
 }

 static int64_t
-filter_get_size (struct backend *b, struct connection *conn)
+filter_get_size (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.get_size)
@@ -458,10 +453,9 @@ filter_get_size (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_write (struct backend *b, struct connection *conn)
+filter_can_write (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_write)
@@ -471,10 +465,9 @@ filter_can_write (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_flush (struct backend *b, struct connection *conn)
+filter_can_flush (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_flush)
@@ -484,10 +477,9 @@ filter_can_flush (struct backend *b, struct connection
*conn)
 }

 static int
-filter_is_rotational (struct backend *b, struct connection *conn)
+filter_is_rotational (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.is_rotational)
@@ -497,10 +489,9 @@ filter_is_rotational (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_trim (struct backend *b, struct connection *conn)
+filter_can_trim (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_trim)
@@ -510,10 +501,9 @@ filter_can_trim (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_zero (struct backend *b, struct connection *conn)
+filter_can_zero (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_zero)
@@ -523,10 +513,9 @@ filter_can_zero (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_fast_zero (struct backend *b, struct connection *conn)
+filter_can_fast_zero (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_fast_zero)
@@ -536,10 +525,9 @@ filter_can_fast_zero (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_extents (struct backend *b, struct connection *conn)
+filter_can_extents (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_extents)
@@ -549,10 +537,9 @@ filter_can_extents (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_fua (struct backend *b, struct connection *conn)
+filter_can_fua (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_fua)
@@ -562,10 +549,9 @@ filter_can_fua (struct backend *b, struct connection *conn)
 }

 static int
-filter_can_multi_conn (struct backend *b, struct connection *conn)
+filter_can_multi_conn (struct backend *b, struct connection *conn, void
*handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_multi_conn)
@@ -575,10 +561,9 @@ filter_can_multi_conn (struct backend *b, struct connection
*conn)
 }

 static int
-filter_can_cache (struct backend *b, struct connection *conn)
+filter_can_cache (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.can_cache)
@@ -588,12 +573,11 @@ filter_can_cache (struct backend *b, struct connection
*conn)
 }

 static int
-filter_pread (struct backend *b, struct connection *conn,
+filter_pread (struct backend *b, struct connection *conn, void *handle,
               void *buf, uint32_t count, uint64_t offset,
               uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.pread)
@@ -604,12 +588,11 @@ filter_pread (struct backend *b, struct connection *conn,
 }

 static int
-filter_pwrite (struct backend *b, struct connection *conn,
+filter_pwrite (struct backend *b, struct connection *conn, void *handle,
                const void *buf, uint32_t count, uint64_t offset,
                uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.pwrite)
@@ -620,11 +603,10 @@ filter_pwrite (struct backend *b, struct connection *conn,
 }

 static int
-filter_flush (struct backend *b, struct connection *conn, uint32_t flags,
-              int *err)
+filter_flush (struct backend *b, struct connection *conn, void *handle,
+              uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.flush)
@@ -634,12 +616,11 @@ filter_flush (struct backend *b, struct connection *conn,
uint32_t flags,
 }

 static int
-filter_trim (struct backend *b, struct connection *conn,
+filter_trim (struct backend *b, struct connection *conn, void *handle,
              uint32_t count, uint64_t offset,
              uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.trim)
@@ -650,11 +631,10 @@ filter_trim (struct backend *b, struct connection *conn,
 }

 static int
-filter_zero (struct backend *b, struct connection *conn,
+filter_zero (struct backend *b, struct connection *conn, void *handle,
              uint32_t count, uint64_t offset, uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.zero)
@@ -665,12 +645,11 @@ filter_zero (struct backend *b, struct connection *conn,
 }

 static int
-filter_extents (struct backend *b, struct connection *conn,
+filter_extents (struct backend *b, struct connection *conn, void *handle,
                 uint32_t count, uint64_t offset, uint32_t flags,
                 struct nbdkit_extents *extents, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   if (f->filter.extents)
@@ -683,12 +662,11 @@ filter_extents (struct backend *b, struct connection
*conn,
 }

 static int
-filter_cache (struct backend *b, struct connection *conn,
+filter_cache (struct backend *b, struct connection *conn, void *handle,
               uint32_t count, uint64_t offset,
               uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  void *handle = connection_get_handle (conn, b->i);
   struct b_conn nxdata = { .b = b->next, .conn = conn };


diff --git a/server/plugins.c b/server/plugins.c
index 120e3cc2..f309ac22 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -239,7 +239,6 @@ plugin_open (struct backend *b, struct connection *conn, int
readonly)
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   void *handle;

-  assert (connection_get_handle (conn, 0) == NULL);
   assert (p->plugin.open != NULL);

   handle = p->plugin.open (readonly);
@@ -255,104 +254,93 @@ plugin_open (struct backend *b, struct connection *conn,
int readonly)
  * .close.
  */
 static int
-plugin_prepare (struct backend *b, struct connection *conn, int readonly)
+plugin_prepare (struct backend *b, struct connection *conn, void *handle,
+                int readonly)
 {
   return 0;
 }

 static int
-plugin_finalize (struct backend *b, struct connection *conn)
+plugin_finalize (struct backend *b, struct connection *conn, void *handle)
 {
   return 0;
 }

 static void
-plugin_close (struct backend *b, struct connection *conn)
+plugin_close (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
-  void *handle = connection_get_handle (conn, 0);

   if (handle && p->plugin.close)
     p->plugin.close (handle);
 }

 static int64_t
-plugin_get_size (struct backend *b, struct connection *conn)
+plugin_get_size (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
   assert (p->plugin.get_size != NULL);

-  return p->plugin.get_size (connection_get_handle (conn, 0));
+  return p->plugin.get_size (handle);
 }

 static int
-plugin_can_write (struct backend *b, struct connection *conn)
+plugin_can_write (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_write)
-    return p->plugin.can_write (connection_get_handle (conn, 0));
+    return p->plugin.can_write (handle);
   else
     return p->plugin.pwrite || p->plugin._pwrite_old;
 }

 static int
-plugin_can_flush (struct backend *b, struct connection *conn)
+plugin_can_flush (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_flush)
-    return p->plugin.can_flush (connection_get_handle (conn, 0));
+    return p->plugin.can_flush (handle);
   else
     return p->plugin.flush || p->plugin._flush_old;
 }

 static int
-plugin_is_rotational (struct backend *b, struct connection *conn)
+plugin_is_rotational (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.is_rotational)
-    return p->plugin.is_rotational (connection_get_handle (conn, 0));
+    return p->plugin.is_rotational (handle);
   else
     return 0; /* assume false */
 }

 static int
-plugin_can_trim (struct backend *b, struct connection *conn)
+plugin_can_trim (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_trim)
-    return p->plugin.can_trim (connection_get_handle (conn, 0));
+    return p->plugin.can_trim (handle);
   else
     return p->plugin.trim || p->plugin._trim_old;
 }

 static int
-plugin_can_zero (struct backend *b, struct connection *conn)
+plugin_can_zero (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
-
   /* Note the special case here: the plugin's .can_zero returns a bool
    * which controls only whether we call .zero; while the backend
    * expects .can_zero to return a tri-state on level of support.
    */
   if (p->plugin.can_zero) {
-    r = p->plugin.can_zero (connection_get_handle (conn, 0));
+    r = p->plugin.can_zero (handle);
     if (r == -1)
       return -1;
     return r ? NBDKIT_ZERO_NATIVE : NBDKIT_ZERO_EMULATE;
@@ -363,15 +351,13 @@ plugin_can_zero (struct backend *b, struct connection
*conn)
 }

 static int
-plugin_can_fast_zero (struct backend *b, struct connection *conn)
+plugin_can_fast_zero (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_fast_zero)
-    return p->plugin.can_fast_zero (connection_get_handle (conn, 0));
+    return p->plugin.can_fast_zero (handle);
   /* Advertise support for fast zeroes if no .zero or .can_zero is
    * false: in those cases, we fail fast instead of using .pwrite.
    * This also works when v1 plugin has only ._zero_old.
@@ -385,30 +371,26 @@ plugin_can_fast_zero (struct backend *b, struct connection
*conn)
 }

 static int
-plugin_can_extents (struct backend *b, struct connection *conn)
+plugin_can_extents (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_extents)
-    return p->plugin.can_extents (connection_get_handle (conn, 0));
+    return p->plugin.can_extents (handle);
   else
     return p->plugin.extents != NULL;
 }

 static int
-plugin_can_fua (struct backend *b, struct connection *conn)
+plugin_can_fua (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
-
   /* The plugin must use API version 2 and have .can_fua return
      NBDKIT_FUA_NATIVE before we will pass the FUA flag on. */
   if (p->plugin.can_fua) {
-    r = p->plugin.can_fua (connection_get_handle (conn, 0));
+    r = p->plugin.can_fua (handle);
     if (r > NBDKIT_FUA_EMULATE && p->plugin._api_version == 1)
       r = NBDKIT_FUA_EMULATE;
     return r;
@@ -420,27 +402,23 @@ plugin_can_fua (struct backend *b, struct connection
*conn)
 }

 static int
-plugin_can_multi_conn (struct backend *b, struct connection *conn)
+plugin_can_multi_conn (struct backend *b, struct connection *conn, void
*handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_multi_conn)
-    return p->plugin.can_multi_conn (connection_get_handle (conn, 0));
+    return p->plugin.can_multi_conn (handle);
   else
     return 0; /* assume false */
 }

 static int
-plugin_can_cache (struct backend *b, struct connection *conn)
+plugin_can_cache (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.can_cache)
-    return p->plugin.can_cache (connection_get_handle (conn, 0));
+    return p->plugin.can_cache (handle);
   if (p->plugin.cache)
     return NBDKIT_CACHE_NATIVE;
   return NBDKIT_CACHE_NONE;
@@ -468,40 +446,35 @@ get_error (struct backend_plugin *p)
 }

 static int
-plugin_pread (struct backend *b, struct connection *conn,
+plugin_pread (struct backend *b, struct connection *conn, void *handle,
               void *buf, uint32_t count, uint64_t offset, uint32_t flags,
               int *err)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
   assert (p->plugin.pread || p->plugin._pread_old);

   if (p->plugin.pread)
-    r = p->plugin.pread (connection_get_handle (conn, 0), buf, count,
offset,
-                         0);
+    r = p->plugin.pread (handle, buf, count, offset, 0);
   else
-    r = p->plugin._pread_old (connection_get_handle (conn, 0), buf, count,
-                              offset);
+    r = p->plugin._pread_old (handle, buf, count, offset);
   if (r == -1)
     *err = get_error (p);
   return r;
 }

 static int
-plugin_flush (struct backend *b, struct connection *conn, uint32_t flags,
-              int *err)
+plugin_flush (struct backend *b, struct connection *conn, void *handle,
+              uint32_t flags, int *err)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
-
   if (p->plugin.flush)
-    r = p->plugin.flush (connection_get_handle (conn, 0), 0);
+    r = p->plugin.flush (handle, 0);
   else if (p->plugin._flush_old)
-    r = p->plugin._flush_old (connection_get_handle (conn, 0));
+    r = p->plugin._flush_old (handle);
   else {
     *err = EINVAL;
     return -1;
@@ -512,7 +485,7 @@ plugin_flush (struct backend *b, struct connection *conn,
uint32_t flags,
 }

 static int
-plugin_pwrite (struct backend *b, struct connection *conn,
+plugin_pwrite (struct backend *b, struct connection *conn, void *handle,
                const void *buf, uint32_t count, uint64_t offset, uint32_t
flags,
                int *err)
 {
@@ -521,31 +494,27 @@ plugin_pwrite (struct backend *b, struct connection *conn,
   bool fua = flags & NBDKIT_FLAG_FUA;
   bool need_flush = false;

-  assert (connection_get_handle (conn, 0));
-
   if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
     flags &= ~NBDKIT_FLAG_FUA;
     need_flush = true;
   }
   if (p->plugin.pwrite)
-    r = p->plugin.pwrite (connection_get_handle (conn, 0), buf, count,
offset,
-                          flags);
+    r = p->plugin.pwrite (handle, buf, count, offset, flags);
   else if (p->plugin._pwrite_old)
-    r = p->plugin._pwrite_old (connection_get_handle (conn, 0),
-                               buf, count, offset);
+    r = p->plugin._pwrite_old (handle, buf, count, offset);
   else {
     *err = EROFS;
     return -1;
   }
   if (r != -1 && need_flush)
-    r = plugin_flush (b, conn, 0, err);
+    r = plugin_flush (b, conn, handle, 0, err);
   if (r == -1 && !*err)
     *err = get_error (p);
   return r;
 }

 static int
-plugin_trim (struct backend *b, struct connection *conn,
+plugin_trim (struct backend *b, struct connection *conn, void *handle,
              uint32_t count, uint64_t offset, uint32_t flags, int *err)
 {
   int r;
@@ -553,29 +522,27 @@ plugin_trim (struct backend *b, struct connection *conn,
   bool fua = flags & NBDKIT_FLAG_FUA;
   bool need_flush = false;

-  assert (connection_get_handle (conn, 0));
-
   if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
     flags &= ~NBDKIT_FLAG_FUA;
     need_flush = true;
   }
   if (p->plugin.trim)
-    r = p->plugin.trim (connection_get_handle (conn, 0), count, offset,
flags);
+    r = p->plugin.trim (handle, count, offset, flags);
   else if (p->plugin._trim_old)
-    r = p->plugin._trim_old (connection_get_handle (conn, 0), count,
offset);
+    r = p->plugin._trim_old (handle, count, offset);
   else {
     *err = EINVAL;
     return -1;
   }
   if (r != -1 && need_flush)
-    r = plugin_flush (b, conn, 0, err);
+    r = plugin_flush (b, conn, handle, 0, err);
   if (r == -1 && !*err)
     *err = get_error (p);
   return r;
 }

 static int
-plugin_zero (struct backend *b, struct connection *conn,
+plugin_zero (struct backend *b, struct connection *conn, void *handle,
              uint32_t count, uint64_t offset, uint32_t flags, int *err)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
@@ -586,8 +553,6 @@ plugin_zero (struct backend *b, struct connection *conn,
   bool emulate = false;
   bool need_flush = false;

-  assert (connection_get_handle (conn, 0));
-
   if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
     flags &= ~NBDKIT_FLAG_FUA;
     need_flush = true;
@@ -598,15 +563,13 @@ plugin_zero (struct backend *b, struct connection *conn,
   if (backend_can_zero (b, conn) == NBDKIT_ZERO_NATIVE) {
     errno = 0;
     if (p->plugin.zero)
-      r = p->plugin.zero (connection_get_handle (conn, 0), count, offset,
-                          flags);
+      r = p->plugin.zero (handle, count, offset, flags);
     else if (p->plugin._zero_old) {
       if (fast_zero) {
         *err = EOPNOTSUPP;
         return -1;
       }
-      r = p->plugin._zero_old (connection_get_handle (conn, 0), count,
offset,
-                               may_trim);
+      r = p->plugin._zero_old (handle, count, offset, may_trim);
     }
     else
       emulate = true;
@@ -634,7 +597,7 @@ plugin_zero (struct backend *b, struct connection *conn,
     static /* const */ char buf[MAX_REQUEST_SIZE];
     uint32_t limit = MIN (count, sizeof buf);

-    r = plugin_pwrite (b, conn, buf, limit, offset, flags, err);
+    r = plugin_pwrite (b, conn, handle, buf, limit, offset, flags, err);
     if (r == -1)
       break;
     count -= limit;
@@ -642,27 +605,24 @@ plugin_zero (struct backend *b, struct connection *conn,

  done:
   if (r != -1 && need_flush)
-    r = plugin_flush (b, conn, 0, err);
+    r = plugin_flush (b, conn, handle, 0, err);
   if (r == -1 && !*err)
     *err = get_error (p);
   return r;
 }

 static int
-plugin_extents (struct backend *b, struct connection *conn,
+plugin_extents (struct backend *b, struct connection *conn, void *handle,
                 uint32_t count, uint64_t offset, uint32_t flags,
                 struct nbdkit_extents *extents, int *err)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
-
   /* This should be true because plugin_can_extents checks it. */
   assert (p->plugin.extents);

-  r = p->plugin.extents (connection_get_handle (conn, 0), count, offset,
-                         flags, extents);
+  r = p->plugin.extents (handle, count, offset, flags, extents);
   if (r >= 0 && nbdkit_extents_count (extents) < 1) {
     nbdkit_error ("extents: plugin must return at least one extent");
     nbdkit_set_error (EINVAL);
@@ -674,21 +634,19 @@ plugin_extents (struct backend *b, struct connection
*conn,
 }

 static int
-plugin_cache (struct backend *b, struct connection *conn,
+plugin_cache (struct backend *b, struct connection *conn, void *handle,
               uint32_t count, uint64_t offset, uint32_t flags,
               int *err)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   int r;

-  assert (connection_get_handle (conn, 0));
-
   /* A plugin may advertise caching but not provide .cache; in that
    * case, caching is explicitly a no-op. */
   if (!p->plugin.cache)
     return 0;

-  r = p->plugin.cache (connection_get_handle (conn, 0), count, offset,
flags);
+  r = p->plugin.cache (handle, count, offset, flags);
   if (r == -1)
     *err = get_error (p);
   return r;
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 3ee9fb25..7179186f 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -481,7 +481,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
          * disconnecting.
          */
         if (finish_newstyle_options (conn, &exportsize) == -1) {
-          if (backend->finalize (backend, conn) == -1)
+          if (backend_finalize (backend, conn) == -1)
             return -1;
           backend_close (backend, conn);
           if (send_newstyle_option_reply (conn, option,
@@ -522,7 +522,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
         return -1;

       if (option == NBD_OPT_INFO) {
-        if (backend->finalize (backend, conn) == -1)
+        if (backend_finalize (backend, conn) == -1)
           return -1;
         backend_close (backend, conn);
       }
-- 
2.21.0

On Thu, Oct 03, 2019 at 09:54:37PM -0500, Eric Blake
wrote:
> Although it is unusual for a plugin to shrink size on reopen, it is
> not impossible.  Failure to check our bounds could result in violating
> assumptions in the plugin that all requests are in-bounds.  Note that
> if the plugin gains a larger size, we merely never access the new tail
> of the file (when the NBD protocol ever gains dynamic resize support,
> we'll get to revisit how the retry filter fits in).
> 
> Fixes: f0f0ec49
> Signed-off-by: Eric Blake <eblake@redhat.com>
I'm inclined to say we should document that the retry filter only
works with reasonable plugins and document those assumptions, but as
you've written the code now let's go with this.

Rich.
>  filters/retry/retry.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/filters/retry/retry.c b/filters/retry/retry.c
> index 840d7383..cf8f5246 100644
> --- a/filters/retry/retry.c
> +++ b/filters/retry/retry.c
> @@ -148,6 +148,17 @@ struct retry_data {
>    int delay;                    /* Seconds to wait before retrying. */
>  };
> 
> +static bool
> +valid_range (struct nbdkit_next_ops *next_ops, void *nxdata,
> +             uint32_t count, uint64_t offset, bool is_write, int *err)
> +{
> +  if ((int64_t) offset + count > next_ops->get_size (nxdata)) {
> +    *err = is_write ? ENOSPC : EIO;
> +    return false;
> +  }
> +  return true;
> +}
> +
>  static bool
>  do_retry (struct retry_handle *h,
>            struct retry_data *data,
> @@ -204,6 +215,8 @@ retry_pread (struct nbdkit_next_ops *next_ops, void
*nxdata,
>    int r;
> 
>   again:
> +  if (! valid_range (next_ops, nxdata, count, offset, false, err))
> +    return -1;
>    r = next_ops->pread (nxdata, buf, count, offset, flags, err);
>    if (r == -1 && do_retry (h, &data, next_ops, nxdata, err))
goto again;
> 
> @@ -226,6 +239,8 @@ retry_pwrite (struct nbdkit_next_ops *next_ops, void
*nxdata,
>      *err = EROFS;
>      return -1;
>    }
> +  if (! valid_range (next_ops, nxdata, count, offset, true, err))
> +    return -1;
>    if (next_ops->can_write (nxdata) != 1) {
>      *err = EROFS;
>      r = -1;
> @@ -258,6 +273,8 @@ retry_trim (struct nbdkit_next_ops *next_ops, void
*nxdata,
>      *err = EROFS;
>      return -1;
>    }
> +  if (! valid_range (next_ops, nxdata, count, offset, true, err))
> +    return -1;
>    if (next_ops->can_trim (nxdata) != 1) {
>      *err = EROFS;
>      r = -1;
> @@ -312,6 +329,8 @@ retry_zero (struct nbdkit_next_ops *next_ops, void
*nxdata,
>      *err = EROFS;
>      return -1;
>    }
> +  if (! valid_range (next_ops, nxdata, count, offset, true, err))
> +    return -1;
>    if (flags & NBDKIT_FLAG_FAST_ZERO &&
>             next_ops->can_fast_zero (nxdata) != 1) {
>      *err = EOPNOTSUPP;
> @@ -347,6 +366,8 @@ retry_extents (struct nbdkit_next_ops *next_ops, void
*nxdata,
>    size_t i;
> 
>   again:
> +  if (! valid_range (next_ops, nxdata, count, offset, false, err))
> +    return -1;
>    if (next_ops->can_extents (nxdata) != 1) {
>      *err = EIO;
>      r = -1;
> @@ -390,6 +411,8 @@ retry_cache (struct nbdkit_next_ops *next_ops, void
*nxdata,
>    int r;
> 
>   again:
> +  if (! valid_range (next_ops, nxdata, count, offset, false, err))
> +    return -1;
>    if (next_ops->can_cache (nxdata) <= NBDKIT_CACHE_NONE) {
>      *err = EIO;
>      r = -1;
> -- 
> 2.21.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html