LIBNBD SECURITY: Denial of service vulnerability
by Eric Blake
We have discovered a denial of service vulnerability in libnbd.
Lifecycle
---------
Reported: 2021-03-01 Fixed: 2021-03-01 Published: 2021-03-12
This has been assigned CVE-2021-20286.
Credit
------
Reported and patched by Eric Blake <eblake(a)redhat.com>
Description
-----------
libnbd is a Network Block Device (NBD) client library.
A malicious server that disconnects at a certain point in the NBD
handshake involving NBD_OPT_GO can cause libnbd to hit an assertion
failure related to an unexpected state; this assertion failure can be
used as a denial of service attack against the libnbd client.
The NBD_OPT_INFO and NBD_OPT_GO handshake commands are a feature of the
newstyle NBD protocol allowing a client to respond gracefully to an
unavailable export without having to re-establish communication with the
server. Although it is unusual that a server would disconnect on
failure to either of these commands rather than letting the client try
again, the client should not die from an assertion failure based on the
server behavior.
Test if libnbd is vulnerable
----------------------------
(There is no simple test for this vulnerability)
Workarounds
-----------
The assertion failure is only triggered in clients that use
nbd_set_opt_mode() for manual control over the handshake sequence (for
example, using 'nbdsh --opt-mode'). It is recommended to apply the fix
or upgrade to a fixed version.
Fixes
-----
This affects versions of libnbd that contain nbd_set_opt_mode(), first
introduced in 1.3.12. A fix is available for 1.6, and the current
development branch.
* development branch (1.7)
https://gitlab.com/nbdkit/libnbd/-/commit/fb4440de9cc76e9c14bd3ddf3333e78...
or use libnbd >= 1.7.3 from
http://download.libguestfs.org/libnbd/1.7-development/
* stable branch 1.6
https://gitlab.com/nbdkit/libnbd/-/commit/2216190ecbbd853648df6a3280c17b3...
or use libnbd >= 1.6.2 from
http://download.libguestfs.org/libnbd/1.6-stable/
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
3 years, 9 months
guestfs tools have moved to a new repository
by Richard W.M. Jones
As part of splitting and rationalising the huge libguestfs git
repository, I have moved all the tools (like virt-df, virt-customize,
virt-builder and many more) out to a new source repository.
It is currently located here:
https://github.com/rwmjones/guestfs-tools/
but this is not the final location, as it will eventually be hosted
(with libguestfs and virt-v2v) on gitlab.com.
The tarballs are here:
https://download.libguestfs.org/guestfs-tools/
In Fedora we're planning to split the downstream packaging into two
separate spec files (the existing libguestfs.spec and a new
guestfs-tools.spec).
I expect that a lot of things are a bit broken at the moment. If
there's anything pressing then let me know, otherwise I'll be working
to fix and improve things over the next few weeks.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
3 years, 9 months
[PATCH nbdkit] multi-conn: Multiple updates to the documentation.
by Richard W.M. Jones
Improve the title to make it non-generic.
Start by introducing the feature and motivation for the filter.
The flag is NBD_FLAG_CAN_MULTI_CONN (not NBD_FLAG_MULTI_CONN), but I
only mention the full name once, abbreviating it to "multi-conn"
elsewhere.
Multiple other smaller changes to improve readability.
---
.../multi-conn/nbdkit-multi-conn-filter.pod | 143 +++++++++++-------
1 file changed, 87 insertions(+), 56 deletions(-)
diff --git a/filters/multi-conn/nbdkit-multi-conn-filter.pod b/filters/multi-conn/nbdkit-multi-conn-filter.pod
index 8979f186..87b31692 100644
--- a/filters/multi-conn/nbdkit-multi-conn-filter.pod
+++ b/filters/multi-conn/nbdkit-multi-conn-filter.pod
@@ -1,34 +1,47 @@
=head1 NAME
-nbdkit-multi-conn-filter - nbdkit multi-conn filter
+nbdkit-multi-conn-filter - enable, emulate or disable multi-conn
=head1 SYNOPSIS
- nbdkit --filter=multi-conn plugin [multi-conn-mode=MODE] \
- [multi-conn-track-dirty=LEVEL] [multi-conn-exportname=BOOL] [plugin-args...]
+ nbdkit --filter=multi-conn plugin
+ [multi-conn-mode=MODE] [multi-conn-track-dirty=LEVEL]
+ [multi-conn-exportname=BOOL]
+ [plugin-args...]
=head1 DESCRIPTION
-C<nbdkit-multi-conn-filter> is a filter that enables alterations to
-the server's advertisement of NBD_FLAG_MULTI_CONN. When a server
-permits multiple simultaneous clients, and sets this flag, a client
-may assume that flush operations ensure consistent data across all
-connections (a sequence of getting a write response, sending and
-waiting for a flush response, then sending a read request will behave
-the same whether all three commands shared a single connection or were
-split among three connections). If the flag is not advertised, a
-client must presume that separate connections may have utilized
-independent caches (even after waiting for a flush on one connection,
-a read on another connection may still see stale data from a cache).
+C<NBD_FLAG_CAN_MULTI_CONN> ("multi-conn") is an L<NBD protocol|
+https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>
+feature that permits multiple clients to connect to the same export
+simultaneously, guaranteeing that flush operations are consistent
+across connections. Specifically a sequence of getting a write
+response, sending and waiting for a flush response, then sending a
+read request will behave the same whether all three commands shared a
+single connection or were split among three connections. When an NBD
+client and server are able to negotiate this feature it can provide
+significant performance benefits. Conversely if the feature is not
+advertised, clients must presume that separate connections can cache
+writes independently (so even after waiting for a flush on one
+connection, a read on another connection may see stale data from a
+cache). The NBD standard advises clients not to multiplex commands
+across connections if the server does not support multi-conn.
-The main use of this filter is to emulate flush-consistent semantics
-across multiple connections when not already provided by a plugin,
-although it also has additional modes useful for evaluating
+L<nbdkit(1)> plugins must normally opt in to multi-conn, after
+carefully ensuring the implementation meets the consistency
+requirements. This filter can emulate flush-consistent semantics
+across multiple connections for plugins that do not advertise this
+feature.
+
+This filter also has additional modes useful for evaluating
performance and correctness of client and plugin multi-conn behaviors.
+
This filter assumes that multiple connections to a plugin will
-eventually share data, other than any caching effects; it is not
+eventually share data, other than any caching effects. It is not
suitable for use with a plugin that produces completely independent
-data per connection from the same export name.
+data per connection from the same export name. An example of a plugin
+that must I<not> be used with this filter is
+L<nbdkit-tmpdisk-plugin(1)>.
Additional control over the behavior of client flush commands is
possible by combining this filter with L<nbdkit-fua-filter(1)>. Note
@@ -42,13 +55,29 @@ layer of caching not needed with this filter.
=item B<multi-conn-mode=auto>
-This filter defaults to B<auto> mode. If the selected thread model is
-SERIALIZE_CONNECTIONS, then this filter behaves the same as B<disable>
-mode; if the plugin advertises multi-conn, then this filter behaves
-the same as B<plugin> mode; otherwise, this filter behaves the same as
-B<emulate> mode. As a result, this mode advertises
-NBD_FLAG_MULTI_CONN to the client exactly when the server supports
-multiple simultaneous connections.
+This is the default mode. The behaviour of B<auto> is as follows:
+
+=over 4
+
+=item *
+
+If the selected thread model is C<SERIALIZE_CONNECTIONS>, then this
+filter behaves the same as B<disable> mode.
+
+=item *
+
+If the plugin advertises multi-conn, then this filter behaves the same
+as B<plugin> mode.
+
+=item *
+
+Otherwise, this filter behaves the same as B<emulate> mode.
+
+=back
+
+In other words, this mode advertises multi-conn to the client exactly
+when the plugin supports or can be made to support multiple
+simultaneous connections.
=item B<multi-conn-mode=emulate>
@@ -56,12 +85,13 @@ When B<emulate> mode is chosen, then this filter tracks all parallel
connections. When a client issues a flush command over any one
connection (including an implied flush by a write command with the FUA
(force unit access) flag set), the filter then replicates that flush
-across each connection to the plugin (although the amount of plugin
-calls can be tuned by adjusting B<multi-conn-track-dirty>). This
-assumes that flushing each connection is enough to clear any
+across each connection to the plugin. The number of plugin calls made
+by the filter can be tuned by adjusting B<multi-conn-track-dirty>.
+
+This mode assumes that flushing each connection is enough to clear any
per-connection cached data, in order to give each connection a
consistent view of the image; therefore, this mode advertises
-NBD_FLAG_MULTI_CONN to the client.
+multi-conn to the client.
Note that in this mode, a client will be unable to connect if the
plugin lacks support for flush, as there would be no way to emulate
@@ -70,25 +100,25 @@ cross-connection flush consistency.
=item B<multi-conn-mode=disable>
When B<disable> mode is chosen, this filter disables advertisement of
-NBD_FLAG_MULTI_CONN to the client, even if the plugin supports it, and
-does not replicate flush commands across connections. This is useful
-for testing whether a client with multiple connections properly sends
+multi-conn to the client, even if the plugin supports it, and does not
+replicate flush commands across connections. This is useful for
+testing whether a client with multiple connections properly sends
multiple flushes in order to overcome per-connection caching.
=item B<multi-conn-mode=plugin>
When B<plugin> mode is chosen, the filter does not change whether
-NBD_FLAG_MULTI_CONN is advertised by the plugin, and does not
-replicate flush commands across connections; but still honors
+multi-conn is advertised by the plugin, and does not replicate flush
+commands across connections; but still honors
B<multi-conn-track-dirty> for minimizing the number of flush commands
passed on to the plugin.
=item B<multi-conn-mode=unsafe>
When B<unsafe> mode is chosen, this filter blindly advertises
-NBD_FLAG_MULTI_CONN to the client even if the plugin lacks support.
-This is dangerous, and risks data corruption if the client makes
-assumptions about flush consistency that were not actually met.
+multi-conn to the client even if the plugin lacks support. This is
+dangerous, and risks data corruption if the client makes assumptions
+about flush consistency that were not actually met.
=item B<multi-conn-track-dirty=fast>
@@ -98,25 +128,25 @@ commands since the last flush, regardless of connection); if all
connections are clean, a client flush command is ignored rather than
sent on to the plugin. In this mode, a flush action on one connection
marks all other connections as clean, regardless of whether the filter
-actually advertised NBD_FLAG_MULTI_CONN, which can result in less
-activity when a client sends multiple flushes rather than taking
-advantage of multi-conn semantics. This is safe with
-B<multi-conn-mode=emulate>, but potentially unsafe with
-B<multi-conn-mode=plugin> when the plugin did not advertise
-multi-conn, as it does not track whether a read may have cached stale
-data prior to a flush.
+actually advertised multi-conn, which can result in less activity when
+a client sends multiple flushes rather than taking advantage of
+multi-conn semantics. This is safe with B<multi-conn-mode=emulate>,
+but potentially unsafe with B<multi-conn-mode=plugin> when the plugin
+did not advertise multi-conn, as it does not track whether a read may
+have cached stale data prior to a flush.
=item B<multi-conn-track-dirty=connection>
-Dirty tracking is set to B<connection> by default, where the filter
-tracks whether a given connection is dirty (any write, zero, or trim
-commands since the last flush on the given connection, and any read
-since the last flush on any other connection); if the connection is
-clean, a flush command to that connection (whether directly from the
-client, or replicated by B<multi-conn-mode=emulate> is ignored rather
-than sent on to the plugin. This mode may result in more flush calls
-than B<multi-conn-track-dirty=fast>, but in turn is safe to use with
-B<multi-conn-mode=plugin>.
+This is the default setting for B<multi-conn-track-dirty>.
+
+The filter tracks whether a given connection is dirty (any write,
+zero, or trim commands since the last flush on the given connection,
+and any read since the last flush on any other connection); if the
+connection is clean, a flush command to that connection (whether
+directly from the client, or replicated by B<multi-conn-mode=emulate>
+is ignored rather than sent on to the plugin. This mode may result in
+more flush calls than B<multi-conn-track-dirty=fast>, but in turn is
+safe to use with B<multi-conn-mode=plugin>.
=item B<multi-conn-track-dirty=off>
@@ -124,7 +154,7 @@ When dirty tracking is set to B<off>, all flush commands from the
client are passed on to the plugin, regardless of whether the flush
would be needed for cross-connection consistency. Note that when
combined with B<multi-conn-mode=emulate>, a client which disregards
-NBD_FLAG_MULTI_CONN by flushing on each connection itself results in a
+multi-conn by flushing on each connection itself results in a
quadratic number of flush operations on the plugin.
=item B<multi-conn-exportname=false>
@@ -191,7 +221,8 @@ L<nbdkit-fua-filter(1)>,
L<nbdkit-nocache-filter(1)>,
L<nbdkit-noextents-filter(1)>,
L<nbdkit-noparallel-filter(1)>,
-L<nbdkit-nozero-filter(1)>.
+L<nbdkit-nozero-filter(1)>,
+L<https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>
=head1 AUTHORS
--
2.29.0.rc2
3 years, 9 months
[PATCH libnbd 0/3] Support extents in libev example
by Nir Soffer
The first patches improve the way we poll events and debugging. The last patch
adds support for extents. With this copying real image performs similar to
qemu-img convert or nbdcopy.
This should be enhance later to support bigger requests for zero requests.
Currently same request size (1m) is used for any kind of request.
Nir Soffer (3):
examples: copy-libev.c: Improve polling
examples: copy-libev.c: Improve debug logs
examples: copy-libev.c: Support extents
examples/copy-libev.c | 279 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 256 insertions(+), 23 deletions(-)
--
2.26.2
3 years, 9 months
[PATCH libnbd v2 0/2] Enhance libev example
by Nir Soffer
- Add FAIL macro to simplfy error handling.
- Support simple sparsification using full requests.
Changes since v1:
- Keep example standalone by not using izzero.h (Rich)
- Use nbd_zero only if destination supports zero (Eric)
- Fix whitespace in one DEBUG call
Nir Soffer (2):
examples: copy-libev: Add FAIL macro
examples: copy-libev.c: Simple sparsifying
examples/copy-libev.c | 175 +++++++++++++++++++++++++++---------------
1 file changed, 115 insertions(+), 60 deletions(-)
--
2.26.2
3 years, 9 months
[PATCH libnbd 0/2] Enhance libev example
by Nir Soffer
- Add FAIL macro to simplfy error handling. This can proabably be used by oth=
er
examples.
- Support simple sparsification using full requests.
Nir Soffer (2):
examples: copy-libev: Add FAIL macro
examples: copy-libev.c: Simple sparsifying
examples/Makefile.am | 1 +
examples/copy-libev.c | 147 +++++++++++++++++++++++++-----------------
2 files changed, 89 insertions(+), 59 deletions(-)
--=20
2.26.2
3 years, 9 months
[PATCH] ext2: Override plugin .can_trim/.can_zero/.can_flush
by Eric Blake
Since we didn't override .can_trim or .can_zero, if the plugin
supports that and we end up advertising it to the client, then the
client trying to take advantage of them would end up trimming or
zeroing at the wrong offsets in the plugin, probably corrupting the
image. (Thankfully not an issue for nbdkit -r when you're just
reading the file system, since nbdkit doesn't advertise trim or zero
on read-only connections).
If we don't override .can_flush, we are at the mercy of the plugin for
whether the client will know that it can stabilize things into the
ext2 filesystem (regardless of whether we can then further stabilize
that filesystem back to the underlying plugin storage).
Finally, the docs for nbdkit-filter say we should not call
next_ops->cache unless can_cache returned NBDKIT_CACHE_NATIVE, but we
were checking for equality to 1 (NBDKIT_CACHE_EMULATE).
Fixes: 86b8466d260 (filters: Add ext2 filter)
---
Pushed now, but still posting this to document the issue.
filters/ext2/ext2.c | 47 ++++++++++++++++++++++++++++++++++++++++-----
filters/ext2/io.c | 11 ++++++-----
2 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/filters/ext2/ext2.c b/filters/ext2/ext2.c
index d7efb00c..1151effd 100644
--- a/filters/ext2/ext2.c
+++ b/filters/ext2/ext2.c
@@ -319,6 +319,45 @@ ext2_can_multi_conn (struct nbdkit_next_ops *next_ops, void *nxdata,
return 0;
}
+static int
+ext2_can_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
+{
+ /* Regardless of the underlying plugin, we handle flush at the level
+ * of the filesystem. However, we also need to cache the underlying
+ * plugin ability, since ext2 wants to flush the filesystem into
+ * permanent storage when possible.
+ */
+ if (next_ops->can_flush (nxdata) == -1)
+ return -1;
+ return 1;
+}
+
+/* XXX It seems as if we should be able to support trim and zero, if
+ * we could work out how those are implemented in the ext2fs API which
+ * is very obscure.
+ */
+static int
+ext2_can_zero (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
+{
+ /* For now, tell nbdkit to call .pwrite instead of any optimization.
+ * However, we also want to cache the underlying plugin support.
+ */
+ if (next_ops->can_zero (nxdata) == -1)
+ return -1;
+ return NBDKIT_ZERO_EMULATE;
+}
+
+static int
+ext2_can_trim (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
+{
+ /* For now, tell nbdkit to never call .trim. However, we also want
+ * to cache the underlying plugin support.
+ */
+ if (next_ops->can_trim (nxdata) == -1)
+ return -1;
+ return 0;
+}
+
/* It might be possible to relax this, but it's complicated.
*
* It's desirable for ‘nbdkit -r’ to behave the same way as
@@ -464,11 +503,6 @@ ext2_flush (struct nbdkit_next_ops *next_ops, void *nxdata,
return 0;
}
-/* XXX It seems as if we should be able to support trim and zero, if
- * we could work out how those are implemented in the ext2fs API which
- * is very obscure.
- */
-
static struct nbdkit_filter filter = {
.name = "ext2",
.longname = "nbdkit ext2 filter",
@@ -485,6 +519,9 @@ static struct nbdkit_filter filter = {
.can_fua = ext2_can_fua,
.can_cache = ext2_can_cache,
.can_multi_conn = ext2_can_multi_conn,
+ .can_zero = ext2_can_zero,
+ .can_trim = ext2_can_trim,
+ .can_flush = ext2_can_flush,
.export_description = ext2_export_description,
.get_size = ext2_get_size,
.pread = ext2_pread,
diff --git a/filters/ext2/io.c b/filters/ext2/io.c
index bab069f4..32c50683 100644
--- a/filters/ext2/io.c
+++ b/filters/ext2/io.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2020 Red Hat Inc.
+ * Copyright (C) 2020-2021 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -310,7 +310,7 @@ io_cache_readahead (io_channel channel,
data = (struct io_private_data *)channel->private_data;
EXT2_CHECK_MAGIC (data, EXT2_ET_MAGIC_NBDKIT_IO_CHANNEL);
- if (data->next_ops->can_cache (data->nxdata) == 1) {
+ if (data->next_ops->can_cache (data->nxdata) == NBDKIT_CACHE_NATIVE) {
/* TODO is 32-bit overflow ever likely to be a problem? */
if (data->next_ops->cache (data->nxdata,
(ext2_loff_t)count * channel->block_size,
@@ -362,8 +362,9 @@ io_flush (io_channel channel)
data = (struct io_private_data *) channel->private_data;
EXT2_CHECK_MAGIC (data, EXT2_ET_MAGIC_NBDKIT_IO_CHANNEL);
- if (data->next_ops->flush (data->nxdata, 0, &errno) == -1)
- return errno;
+ if (data->next_ops->can_flush (data->nxdata) == 1)
+ if (data->next_ops->flush (data->nxdata, 0, &errno) == -1)
+ return errno;
return retval;
}
@@ -432,7 +433,7 @@ io_zeroout (io_channel channel, unsigned long long block,
data = (struct io_private_data *) channel->private_data;
EXT2_CHECK_MAGIC (data, EXT2_ET_MAGIC_NBDKIT_IO_CHANNEL);
- if (data->next_ops->can_zero (data->nxdata) == 1) {
+ if (data->next_ops->can_zero (data->nxdata) > NBDKIT_ZERO_NONE) {
/* TODO is 32-bit overflow ever likely to be a problem? */
if (data->next_ops->zero (data->nxdata,
(off_t)(count) * channel->block_size,
--
2.30.1
3 years, 9 months
[libnbd PATCH] info: Let exit status reflect any failures during NBD_OPT_INFO
by Eric Blake
It turns out that at least nbdkit's testsuite was relying on a
non-zero exit status from nbdinfo when purposefully attempting to get
info on an invalid export name. Printing as much information as
possible instead of going silent becaus of one error is good, but any
time we print to stderr, the exit status should reflect that.
Fixes: 5473e34fc1 (info: Don't kill --list early just because one opt_info fails)
Reported-by: Rich Jones <rjones(a)redhat.com>
---
This fixes the regression we were seeing in nbdkit.
info/nbdinfo.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 4b18ab27..3dfc463f 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -58,9 +58,9 @@ DEFINE_VECTOR_TYPE (uint32_vector, uint32_t)
static int collect_context (void *opaque, const char *name);
static int collect_export (void *opaque, const char *name,
const char *desc);
-static void list_one_export (struct nbd_handle *nbd, const char *desc,
+static bool list_one_export (struct nbd_handle *nbd, const char *desc,
bool first, bool last);
-static void list_all_exports (struct nbd_handle *nbd1, const char *uri);
+static bool list_all_exports (struct nbd_handle *nbd1, const char *uri);
static void print_json_string (const char *);
static char *get_content (struct nbd_handle *, int64_t size);
static int extent_callback (void *user_data, const char *metacontext,
@@ -124,6 +124,7 @@ main (int argc, char *argv[])
int tls_negotiated;
char *output = NULL;
size_t output_len = 0;
+ bool list_okay = true;
progname = argv[0];
@@ -336,9 +337,9 @@ main (int argc, char *argv[])
}
if (!list_all)
- list_one_export (nbd, NULL, true, true);
+ list_okay = list_one_export (nbd, NULL, true, true);
else
- list_all_exports (nbd, argv[optind]);
+ list_okay = list_all_exports (nbd, argv[optind]);
if (json_output)
fprintf (fp, "}\n");
@@ -365,7 +366,7 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- exit (EXIT_SUCCESS);
+ exit (list_okay ? EXIT_SUCCESS : EXIT_FAILURE);
}
static int
@@ -398,7 +399,7 @@ collect_export (void *opaque, const char *name, const char *desc)
return 0;
}
-static void
+static bool
list_one_export (struct nbd_handle *nbd, const char *desc,
bool first, bool last)
{
@@ -424,7 +425,7 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
nbd_opt_go (nbd) == -1) {
fprintf (stderr, "%s: %s: %s\n", progname, nbd_get_export_name (nbd),
nbd_get_error ());
- return;
+ return false;
}
size = nbd_get_size (nbd);
if (size == -1) {
@@ -599,12 +600,14 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
free (content);
free (export_name);
free (export_desc);
+ return true;
}
-static void
+static bool
list_all_exports (struct nbd_handle *nbd1, const char *uri)
{
size_t i;
+ bool list_okay = true;
if (export_list.size == 0 && json_output)
fprintf (fp, "\"exports\": []\n");
@@ -639,14 +642,16 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri)
}
/* List the metadata of this export. */
- list_one_export (nbd2, export_list.ptr[i].desc, i == 0,
- i + 1 == export_list.size);
+ if (!list_one_export (nbd2, export_list.ptr[i].desc, i == 0,
+ i + 1 == export_list.size))
+ list_okay = false;
if (probe_content) {
nbd_shutdown (nbd2, 0);
nbd_close (nbd2);
}
}
+ return list_okay;
}
static void
--
2.30.1
3 years, 9 months
[PATCH] file: Better support for read-only files
by Eric Blake
Previously, attempts to use nbdkit without -r in order to visit a
read-only image failed to open the image (in fact, a single read-only
file in an otherwise usable directory makes "nbdkit file dir=. --run
'nbdinfo --list'" fail to print anything in libnbd 1.6). But a saner
approach is to try a fallback to a readonly fd if a read-write fd
fails.
The windows code compiles, but I wasn't able to test it as thoroughly
as the Unix code.
---
plugins/file/file.c | 30 ++++++++++++++++--
plugins/file/winfile.c | 21 +++++++++++--
tests/Makefile.am | 12 +++++--
tests/test-file-readonly.sh | 63 +++++++++++++++++++++++++++++++++++++
4 files changed, 118 insertions(+), 8 deletions(-)
create mode 100755 tests/test-file-readonly.sh
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 1651079a..a9ecceb6 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2021 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -304,6 +304,7 @@ struct handle {
int fd;
bool is_block_device;
int sector_size;
+ bool can_write;
bool can_punch_hole;
bool can_zero_range;
bool can_fallocate;
@@ -343,12 +344,25 @@ file_open (int readonly)
}
flags = O_CLOEXEC|O_NOCTTY;
- if (readonly)
+ if (readonly) {
flags |= O_RDONLY;
- else
+ h->can_write = false;
+ }
+ else {
flags |= O_RDWR;
+ h->can_write = true;
+ }
h->fd = openat (dfd, file, flags);
+ if (h->fd == -1 && !readonly) {
+ int saved_errno = errno;
+ flags = (flags & ~O_ACCMODE) | O_RDONLY;
+ h->fd = openat (dfd, file, flags);
+ if (h->fd == -1)
+ errno = saved_errno;
+ else
+ h->can_write = false;
+ }
if (h->fd == -1) {
nbdkit_error ("open: %s: %m", file);
if (dfd != -1)
@@ -464,6 +478,15 @@ file_get_size (void *handle)
}
}
+/* Check if file is read-only. */
+static int
+file_can_write (void *handle)
+{
+ struct handle *h = handle;
+
+ return h->can_write;
+}
+
/* Allow multiple parallel connections from a single client. */
static int
file_can_multi_conn (void *handle)
@@ -880,6 +903,7 @@ static struct nbdkit_plugin plugin = {
.open = file_open,
.close = file_close,
.get_size = file_get_size,
+ .can_write = file_can_write,
.can_multi_conn = file_can_multi_conn,
.can_trim = file_can_trim,
.can_fua = file_can_fua,
diff --git a/plugins/file/winfile.c b/plugins/file/winfile.c
index 6d1a6643..ec186255 100644
--- a/plugins/file/winfile.c
+++ b/plugins/file/winfile.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2021 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -106,6 +106,7 @@ winfile_dump_plugin (void)
struct handle {
HANDLE fh;
int64_t size;
+ bool is_readonly;
bool is_volume;
bool is_sparse;
};
@@ -126,6 +127,12 @@ winfile_open (int readonly)
fh = CreateFile (filename, flags, FILE_SHARE_READ|FILE_SHARE_WRITE,
NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+ if (fh == INVALID_HANDLE_VALUE && !readonly) {
+ flags &= ~GENERIC_WRITE;
+ readonly = true;
+ fh = CreateFile (filename, flags, FILE_SHARE_READ|FILE_SHARE_WRITE,
+ NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+ }
if (fh == INVALID_HANDLE_VALUE) {
nbdkit_error ("%s: error %lu", filename, GetLastError ());
return NULL;
@@ -179,15 +186,24 @@ winfile_open (int readonly)
}
h->fh = fh;
h->size = size.QuadPart;
+ h->is_readonly = readonly;
h->is_volume = is_volume;
h->is_sparse = is_sparse;
- nbdkit_debug ("%s: size=%" PRIi64 " is_volume=%s is_sparse=%s",
+ nbdkit_debug ("%s: size=%" PRIi64 " readonly=%s is_volume=%s is_sparse=%s",
filename, h->size,
+ readonly ? "true" : "false",
is_volume ? "true" : "false",
is_sparse ? "true" : "false");
return h;
}
+static int
+wintfile_can_write (void *handle)
+{
+ struct handle *h = handle;
+ return !h->is_readonly;
+}
+
static int
winfile_can_trim (void *handle)
{
@@ -425,6 +441,7 @@ static struct nbdkit_plugin plugin = {
.dump_plugin = winfile_dump_plugin,
.open = winfile_open,
+ .can_write = winfile_can_write,
.can_trim = winfile_can_trim,
.can_zero = winfile_can_zero,
.can_extents = winfile_can_extents,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 70898f20..e8c9c535 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,5 +1,5 @@
# nbdkit
-# Copyright (C) 2013-2020 Red Hat Inc.
+# Copyright (C) 2013-2021 Red Hat Inc.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -673,8 +673,14 @@ EXTRA_DIST += \
$(NULL)
# file plugin test.
-TESTS += test-file.sh
-EXTRA_DIST += test-file.sh
+TESTS += \
+ test-file.sh \
+ test-file-readonly.sh \
+ $(NULL)
+EXTRA_DIST += \
+ test-file.sh \
+ test-file-readonly.sh \
+ $(NULL)
LIBGUESTFS_TESTS += test-file-block
test_file_block_SOURCES = test-file-block.c test.h
diff --git a/tests/test-file-readonly.sh b/tests/test-file-readonly.sh
new file mode 100755
index 00000000..cde2bd65
--- /dev/null
+++ b/tests/test-file-readonly.sh
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2013-2021 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.
+
+# Test the file plugin on readonly files.
+
+source ./functions.sh
+set -e
+set -x
+
+requires_plugin file
+requires_nbdsh_uri
+requires truncate --version
+
+sock=$(mktemp -u /tmp/nbdkit-test-sock.XXXXXX)
+files="file-readonly.pid file-readonly.img $sock"
+rm -f $files
+cleanup_fn rm -f $files
+
+truncate -s 16384 file-readonly.img
+chmod a-w file-readonly.img
+start_nbdkit -P file-readonly.pid -U $sock file file-readonly.img
+
+# Try to test all the major functions supported by both
+# the Unix and Windows versions of the file plugin.
+nbdsh -u "nbd+unix://?socket=$sock" -c '
+assert h.is_read_only()
+assert not h.can_zero()
+
+buf = h.pread(8192, 0)
+assert buf == b"\0" * 8192
+
+if h.can_flush():
+ h.flush()
+'
--
2.30.1
3 years, 9 months
