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(a)redhat.com&gt; --- 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