[Libguestfs] [nbdkit PATCH 2/2] filters: Stronger version match requirements

[Eric Blake]

Rather than guessing on API/ABI compatibility based on when we
remember to bump a compile-time constant (which should be done at
least once per stable release, although we forgot in the last two, per
6934d4c1), we can go even tighter and require that the version be
precisely the version string used at compile time (catching
incompatibility even in between non-stable releases).  But as this is
yet another ABI change, and we want to avoid seg-faults if an old
filter .so is accidentally loaded, we first sanity check whether it
looks like we have a pointer (which will not live in the first page of
memory - new ABI) or a small integer (old ABI); we generally get lucky
that this works for both 32- and 64-bit platforms, whether little- or
big-endian (on 64-bit platforms, there are various corner-case
pointers where it won't work; for example, an address of 0x100000004
would fail the test on either a little- or a big- endian machine; but
in general, that doesn't happen):

$ ./nbdkit --filter /usr/lib64/nbdkit/filters/nbdkit-nozero-filter.so null
nbdkit: /usr/lib64/nbdkit/filters/nbdkit-nozero-filter.so: filter is incompatible with this version of nbdkit, and appears to stem from nbdkit 1.14 or earlier

Similarly, we are fairly likely
that any new filter will fail to load with an old nbdkit, for the same
reasons (and again with the same corner-case addresses where it
doesn't actually fail).  A nice error message in 99.99% of the cases
with segv on the corner cases is better than nothing, especially since
all such version mismatch cases are user configuration error:

$ nbdkit --filter ./filters/nozero/.libs/nbdkit-nozero-filter.so null
nbdkit: ./filters/nozero/.libs/nbdkit-nozero-filter.so: filter is incompatible with this version of nbdkit (_api_version = 430821533)

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 server/filters.c        | 32 ++++++++++++++++++++++++++------
 include/nbdkit-filter.h | 10 ++++------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/server/filters.c b/server/filters.c
index 3bea476b..bb6394fb 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -839,6 +839,10 @@ filter_register (struct backend *next, size_t index, const char *filename,
   struct backend_filter *f;
   const struct nbdkit_filter *filter;
   size_t i, len;
+  union overlay {
+    const char *str;
+    unsigned int api;
+  } u;

   f = calloc (1, sizeof *f);
   if (f == NULL) {
@@ -866,15 +870,31 @@ filter_register (struct backend *next, size_t index, const char *filename,
     exit (EXIT_FAILURE);
   }

-  /* We do not provide API or ABI guarantees for filters, other than
-   * the ABI position of _api_version that will let us diagnose
-   * mismatch when the API changes.
+  /* We do not provide API or ABI guarantees for filters; instead, we
+   * use _nbdkit_version to diagnose mismatch in compilation
+   * vs. runtime setup.  However, nbdkit 1.2 through 1.14 used an int
+   * instead of const char * (containing values ranging from 1 through
+   * 5 over time), and if we blindly dereference that ABI as a pointer
+   * without at least a sanity check, we get a SEGV instead of a nice
+   * failure message.  Hence, we have to run a sanity-check: if the
+   * first four bytes of resemble a small positive integer (but not 0,
+   * as that may be a typical upper half of a big-endian 64-bit
+   * pointer), assume it is the old filter ABI; and only dereference
+   * if it looks like we have the new API.
    */
-  if (filter->_api_version != NBDKIT_FILTER_API_VERSION) {
+  u.str = filter->_nbdkit_version;
+  if (u.api && u.api < 8) {
+    fprintf (stderr,
+             "%s: %s: filter is incompatible with this version of nbdkit, "
+             "and appears to stem from nbdkit 1.14 or earlier\n",
+             program_name, f->filename);
+    exit (EXIT_FAILURE);
+  }
+  if (strcmp (filter->_nbdkit_version, NBDKIT_VERSION_STRING) != 0) {
     fprintf (stderr,
              "%s: %s: filter is incompatible with this version of nbdkit "
-             "(_api_version = %d)\n",
-             program_name, f->filename, filter->_api_version);
+             "(_nbdkit_version = %s)\n",
+             program_name, f->filename, filter->_nbdkit_version);
     exit (EXIT_FAILURE);
   }

diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index 4f8f19ab..29d92755 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -43,8 +43,6 @@
 extern "C" {
 #endif

-#define NBDKIT_FILTER_API_VERSION 5 /* Corresponding to v1.14 */
-
 struct nbdkit_extent {
   uint64_t offset;
   uint64_t length;
@@ -98,11 +96,11 @@ struct nbdkit_filter {
    * one version of the header with a runtime compiled against a
    * different version.
    */
-  int _api_version;
+  const char *_nbdkit_version;

   /* Because there is no ABI guarantee, new fields may be added where
-   * logically appropriate, as long as we correctly bump
-   * NBDKIT_FILTER_API_VERSION once per stable release.
+   * logically appropriate, as long as _nbdkit_version can be used to
+   * flag version mismatch.
    */
   const char *name;
   const char *longname;
@@ -178,7 +176,7 @@ struct nbdkit_filter {
   struct nbdkit_filter *                                                \
   filter_init (void)                                                    \
   {                                                                     \
-    (filter)._api_version = NBDKIT_FILTER_API_VERSION;                  \
+    (filter)._nbdkit_version = NBDKIT_VERSION_STRING;                   \
     return &(filter);                                                   \
   }

-- 
2.21.0