[nbdkit PATCH] connections: Don't use uninit memory on early client EOF
by Eric Blake
Fuzzing with afl found a bug where a 27 byte client sequence
can cause nbdkit to report a strange error message:
$ printf %s $'000\1IHAVEOPT000\6'$'000\7'$'000\1x00' | tr 0 '\0' |
./nbdkit -s memory size=1m >/dev/null
nbdkit: memory: error: client exceeded maximum number of options (32)
The culprit? The client is hanging up on a message boundary,
so conn->recv() returns 0 for EOF instead of -1 for read error,
and our code blindly continues on in a for loop (re-)parsing
the leftover data from the previous option.
Let's fix things to uniformly quit trying to negotiate with a client
that has early EOF at a message boundary, just as we do for one that
quits mid-field, with the one difference that we treat a message
boundary as a warning instead of an error because a client hanging up
after an option response that it didn't like (rather than sending
NBD_OPT_ABORT to inform the server that it won't be negotiating
further) is a surprisingly common behavior among some existing clients.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/connections.c | 60 ++++++++++++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 21 deletions(-)
diff --git a/src/connections.c b/src/connections.c
index 58ed6b0..577f466 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -600,6 +600,31 @@ send_newstyle_option_reply_info_export (struct connection *conn,
return 0;
}
+/* Sub-function during _negotiate_handshake_newstyle, to uniformly handle
+ * a client hanging up on a message boundary.
+ */
+static int __attribute__ ((format (printf, 4, 5)))
+conn_recv_full (struct connection *conn, void *buf, size_t len,
+ const char *fmt, ...)
+{
+ int r = conn->recv (conn, buf, len);
+ va_list args;
+
+ if (r == -1) {
+ va_start (args, fmt);
+ nbdkit_verror (fmt, args);
+ va_end (args);
+ return -1;
+ }
+ if (r == 0) {
+ /* During negotiation, client EOF on message boundary is less
+ * severe than failure in the middle of the buffer. */
+ debug ("client closed input socket, closing connection");
+ return -1;
+ }
+ return r;
+}
+
/* Sub-function of _negotiate_handshake_newstyle_options below. It
* must be called on all non-error paths out of the options for-loop
* in that function.
@@ -639,10 +664,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
const char *optname;
for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) {
- if (conn->recv (conn, &new_option, sizeof new_option) == -1) {
- nbdkit_error ("read: %m");
+ if (conn_recv_full (conn, &new_option, sizeof new_option,
+ "read: %m") == -1)
return -1;
- }
version = be64toh (new_option.version);
if (version != NEW_VERSION) {
@@ -675,10 +699,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
switch (option) {
case NBD_OPT_EXPORT_NAME:
- if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
+ if (conn_recv_full (conn, data, optlen,
+ "read: %s: %m", name_of_nbd_opt (option)) == -1)
return -1;
- }
/* Apart from printing it, ignore the export name. */
data[optlen] = '\0';
debug ("newstyle negotiation: %s: "
@@ -715,10 +738,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)
== -1)
return -1;
- if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
+ if (conn_recv_full (conn, data, optlen,
+ "read: %s: %m", name_of_nbd_opt (option)) == -1)
return -1;
- }
continue;
}
@@ -738,10 +760,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)
== -1)
return -1;
- if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
+ if (conn_recv_full (conn, data, optlen,
+ "read: %s: %m", name_of_nbd_opt (option)) == -1)
return -1;
- }
continue;
}
@@ -780,10 +801,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
case NBD_OPT_INFO:
case NBD_OPT_GO:
optname = name_of_nbd_opt (option);
- if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %s: %m", optname);
+ if (conn_recv_full (conn, data, optlen,
+ "read: %s: %m", optname) == -1)
return -1;
- }
if (optlen < 6) { /* 32 bit export length + 16 bit nr info */
debug ("newstyle negotiation: %s option length < 6", optname);
@@ -880,10 +900,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
/* Unknown option. */
if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1)
return -1;
- if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %m");
+ if (conn_recv_full (conn, data, optlen,
+ "read: %m") == -1)
return -1;
- }
}
/* Note, since it's not very clear from the protocol doc, that the
@@ -931,10 +950,9 @@ _negotiate_handshake_newstyle (struct connection *conn)
}
/* Client now sends us its 32 bit flags word ... */
- if (conn->recv (conn, &conn->cflags, sizeof conn->cflags) == -1) {
- nbdkit_error ("read: %m");
+ if (conn_recv_full (conn, &conn->cflags, sizeof conn->cflags,
+ "read: %m") == -1)
return -1;
- }
conn->cflags = be32toh (conn->cflags);
/* ... which we check for accuracy. */
debug ("newstyle negotiation: client flags: 0x%x", conn->cflags);
--
2.17.2
5 years, 5 months
why virt-v2v in-place option not support in centos 7?
by Zhang Huan
Hi All,
As the in-place help said,
--in-place
Do not create an output virtual machine in the target hypervisor. Instead, adjust the guest OS in the source VM to run in the input hypervisor.
This mode is meant for integration with other toolsets, which take the responsibility of converting the VM configuration, providing for rollback in case of errors, transforming the storage, etc.
I want to do v2v modifications in the source disk image, but the v2v give the error:
virt-v2v -v -x -i disk /dev/vdh --in-place
virt-v2v: error: --in-place cannot be used in RHEL 7
could any one give me some help?
why —in-place option disabled in RHEL7, is there a older version enabled this feture?
Thanks very much for help!
Best Wishes.
5 years, 5 months
[PATCH nbdkit v2 0/4] tests: Test export flags (eflags).
by Richard W.M. Jones
v1 was here:
https://www.redhat.com/archives/libguestfs/2018-December/thread.html#00123
v2:
- Document "-" instead of "script=-" and use it in the test; and
verify this also works on FreeBSD; and verify that it doesn't
depend on the particular behaviour of our wrapper script and should
work with installed nbdkit too.
- Fix handling of zero flags parameter.
- Add "to stdout" for can_fua.
- "In future" -> "In the future," in various places.
- Make the test work on FreeBSD by avoiding use of endian swaps.
- Use socat "cool-write" to avoid occasional EPIPE error seen in tests.
- Retest on Linux & FreeBSD.
- Extra commit for a small documentation change in nbdkit-sh-plugin.
Rich.
5 years, 5 months
[nbdkit PATCH] maint: Adjust cleaning rules
by Eric Blake
'make distcheck' calls us out for leaving files behind after
'make distclean' that were not present in the tarball. Either
these files are expensive enough that end users should not be
required to regenerate them (so they should be distributed),
or they should be cleaned when a user asks to get back to the
pristine tarball state.
Automake suggests this hierarchy of cleaning:
mostlyclean: .o and other obvious build artifacts
clean: everything that 'make' builds without rerunning configure
distclean: everything that 'configure' builds
maintainerclean: things that requires special tooling to rebuild
By that definition, things that are expensive to rebuild, but
which are built by make and not configure, belong in distclean
rather than maintainerclean (this includes the TLS temporary
files, as building them consumes host entropy); and things which
are not terribly expensive to rebuild can live in clean. I
didn't find any of the files called out as being hard enough to
regenerate to make it worth including in the tarball, which
would be the only reason to keep such files in maintainerclean.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/Makefile.am | 2 +-
tests/Makefile.am | 30 ++++++++++++++++++------------
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 1772c66..6b712e3 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -123,4 +123,4 @@ filter-links.pod: $(top_srcdir)/common-rules.mk
$(srcdir)/make-links.sh filter 1 $(filters) > $@-t
mv $@-t $@
-MAINTAINERCLEANFILES = plugin-links.pod filter-links.pod
+DISTCLEANFILES = plugin-links.pod filter-links.pod
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4c7b59c..55db593 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -32,7 +32,8 @@
include $(top_srcdir)/common-rules.mk
-MAINTAINERCLEANFILES =
+CLEANFILES =
+DISTCLEANFILES =
EXTRA_DIST = \
make-pki.sh \
@@ -214,9 +215,10 @@ keys.psk: $(srcdir)/make-psk.sh
SRCDIR=$(srcdir) $(srcdir)/make-psk.sh
# Keys are expensive to recreate so only delete them when we do
-# ‘make maintainer-clean’.
-MAINTAINERCLEANFILES += keys.psk
-maintainer-clean-local:
+# ‘make distclean’.
+DISTCLEANFILES += keys.psk
+distclean-local: distclean-local-tls
+distclean-local-tls:
rm -rf pki
#----------------------------------------------------------------------
@@ -226,7 +228,7 @@ if HAVE_PLUGINS
# Common data shared by multiple tests
check_DATA += file-data
-MAINTAINERCLEANFILES += file-data
+CLEANFILES += file-data
file-data:
rm -f $@ $@-t
for f in `$(SEQ) 1 512`; do echo -ne '\x01\x02\x03\x04\x05\x06\x07\x08'; done > $@-t
@@ -286,7 +288,7 @@ endif HAVE_LIBGUESTFS
# common disk image shared with several tests
if HAVE_GUESTFISH
check_DATA += disk
-MAINTAINERCLEANFILES += disk
+CLEANFILES += disk
disk:
rm -f $@ test1.img
@@ -340,7 +342,7 @@ if HAVE_GUESTFISH
LIBGUESTFS_TESTS += test-ext2
check_DATA += ext2.img
-MAINTAINERCLEANFILES += ext2.img
+CLEANFILES += ext2.img
ext2.img: disk
rm -f $@ $@-t
@@ -385,7 +387,7 @@ if HAVE_GUESTFISH
LIBGUESTFS_TESTS += test-gzip
check_DATA += disk.gz
-MAINTAINERCLEANFILES += disk.gz
+CLEANFILES += disk.gz
test_gzip_SOURCES = test-gzip.c test.h
test_gzip_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS)
@@ -454,7 +456,7 @@ test_random_LDADD = libtest.la $(LIBGUESTFS_LIBS)
# split files plugin test.
check_DATA += split1 split2 split3
-MAINTAINERCLEANFILES += split1 split2 split3
+CLEANFILES += split1 split2 split3
split1: file-data
rm -f $@ $@-t
dd if=$< of=$@-t bs=1 count=100
@@ -528,6 +530,10 @@ test-ocaml-plugin.so: test_ocaml_plugin.cmx ../plugins/ocaml/libnbdkitocaml.la .
-output-obj -runtime-variant _pic -o $@ \
NBDKit.cmx $< \
-cclib -L../plugins/ocaml/.libs -cclib -lnbdkitocaml
+CLEANFILES += \
+ test_ocaml_plugin.cmx \
+ test_ocaml_plugin.cmi \
+ test-ocaml-plugin.so
endif HAVE_OCAML
@@ -585,7 +591,7 @@ endif HAVE_RUBY
# Shell (sh) plugin test.
LIBGUESTFS_TESTS += test-shell
check_DATA += test-shell.img
-MAINTAINERCLEANFILES += test-shell.img
+CLEANFILES += test-shell.img
test_shell_SOURCES = test-lang-plugins.c test.h
test_shell_CFLAGS = \
@@ -732,7 +738,7 @@ TESTS += test-nozero.sh
# offset filter test.
check_DATA += offset-data
-MAINTAINERCLEANFILES += offset-data
+CLEANFILES += offset-data
LIBGUESTFS_TESTS += test-offset
offset-data:
@@ -765,7 +771,7 @@ if HAVE_GUESTFISH
LIBGUESTFS_TESTS += test-xz
check_DATA += disk.xz
-MAINTAINERCLEANFILES += disk.xz
+CLEANFILES += disk.xz
test_xz_SOURCES = test-xz.c test.h
test_xz_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS)
--
2.17.2
5 years, 5 months
[PATCH] v2v: update docs for VMware roles (RHBZ#1530967)
by Pino Toscano
Update the list of permissions needed for VMware vCenter 6.5.
---
v2v/virt-v2v-input-vmware.pod | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/v2v/virt-v2v-input-vmware.pod b/v2v/virt-v2v-input-vmware.pod
index 636652e50..bc17cd913 100644
--- a/v2v/virt-v2v-input-vmware.pod
+++ b/v2v/virt-v2v-input-vmware.pod
@@ -546,7 +546,8 @@ write to libvirt or any other supported target.
Instead of using the vCenter Administrator role, you can create a
custom non-administrator role to perform the conversion. You will
-however need to give it a minimum set of permissions as follows:
+however need to give it a minimum set of permissions as follows
+(using VMware vCenter 6.5):
=over 4
@@ -566,10 +567,12 @@ Enable (check) the following objects:
- Validate session
Virtual Machine:
+ Interaction:
+ - Guest operating system management by VIX API
Provisioning:
- Allow disk access
- Allow read-only disk access
- - Guest Operating system management by VIX API
+ - Allow virtual machine download
=back
--
2.17.2
5 years, 5 months
[PATCH v2 2/2] v2v: Copy static IP address information over for Windows guests (RHBZ#1626503).
by Richard W.M. Jones
v1 was here with much discussion:
https://www.redhat.com/archives/libguestfs/2018-December/msg00048.html
v2:
- Fix the case where there are multiple interfaces. Note this does
not preserve order correctly (see patch for comment on why that
is a hard problem).
- Preserve name servers.
This patch is still for discussion only. I'd like to see what might
be done to get this upstream in a way that will not change existing
cloud-related uses of virt-v2v. Perhaps this should only be
operational with a command line flag, or triggered only for particular
input and output modes.
For those interested, this is what the Powershell script ends up
looking like:
----------------------------------------------------------------------
Set-PSDebug -Trace 1
# Wait for the netkvm (virtio-net) driver to become active.
$adapters = @()
While (-Not $adapters) {
Start-Sleep -Seconds 5
$adapters = (Get-NetAdapter | Where DriverFileName -eq "netkvm.sys").InterfaceAlias | Sort-Object
Write-Host "adapters = '$adapters'"
}
New-NetIPAddress -InterfaceAlias $adapters[0] -IPAddress "10.0.2.15" -DefaultGateway "10.0.2.2" -PrefixLength 8
Set-DnsClientServerAddress -InterfaceAlias $adapters[0] -ServerAddresses ("10.0.2.3")
New-NetIPAddress -InterfaceAlias $adapters[1] -IPAddress "10.0.2.16" -DefaultGateway "10.0.2.2" -PrefixLength 8
Set-DnsClientServerAddress -InterfaceAlias $adapters[1] -ServerAddresses ("10.0.2.3")
----------------------------------------------------------------------
Rich.
5 years, 5 months
v2v: -o rhv-upload: Upload via NBD
by Nir Soffer
I posted this patch for vdsm, adding NBD APIs:
https://gerrit.ovirt.org/c/96079/
The main purpose of these APIs are enabling incremental restore, but they
also
enable a more efficient rhv-upload via NBD, and importing to thin disks,
which is
not possible with current solution.
The same idea can work for KubeVirt or other targets, minimizing specific
target code.
Here is how rhv-upload can work using NBD:
1. rhv-upload plugin separated to pre and post scripts
- pre - prepare disk and start image transfer
- post - finialize image transfer, create vm, etc.
2. rhr-upload-pre plugin create a transfer with transport="nbd"
POST /imagetransfers
<image_transfer>
<disk id="123"/>
<direction>upload</direction>
<format>raw</format>
<transport>nbd</transport>
</image_transfer>
Engine does not implement <transport> yet, but this should be an easy
change.
This will use the new NBD APIs to export the disk using NBD over unix
socket.
We can support later also NBD over TLS/PSK.
Engine will return NBD url in transfer_url:
<transfer_url>nbd:unix:/run/vdsm/nbd/<transfer_uuid>.sock</tansfer_url>
3. v2v use the trasfer_url to start qem-img with the NBD server:
nbdkit (vddk) -> qemu-img convert -> qemu-nbd
Note that nbdkit is removed from the rhv side of the pipeline. This is
expected to
improve the throughput significantly, since imageio is not very good with
lot of
small requests generated by qemu-img.
4. rhv-upload-post script invoked to complete the transfer
What do you think?
Nir
5 years, 5 months
[nbdkit PATCH] build: Allow 'make install' into non-root --prefix: bash-completion
by Eric Blake
In general, autotooled packages are supposed to allow
'./configure --prefix=$HOME/subdir' as a way to then get
'make install' to run as non-root. In fact, 'make distcheck'
tests that this scenario works; alas, we fail due to:
/usr/bin/install -c -m 644 ../../../bash/nbdkit '/usr/share/bash-completion/completions'
/usr/bin/install: cannot remove '/usr/share/bash-completion/completions/nbdkit': Permission denied
The culprit? We use pkg-config to ask bash-completion where it
would install user completion scripts. bash-completion.pc states:
prefix=/usr
completionsdir=${prefix}/share/bash-completion/completions
but pkg-config --variable defaults to flattening the ${prefix}
in our use of the PKG_CHECK_VAR() macro, which in turn means
that './configure --prefix=$HOME/subdir' still uses an absolute
path pointing to a root-owned directory (rather than one relative
to our desired ${prefix}) in our definition of bashcompsdir.
The solution? Tell pkg-config to NOT flatten the prefix variable.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'm posting this one for review, particularly since it means someone
running 'make install' with a non-root --prefix will now be stuck
with bash completion libraries that won't be loaded by default (the
user will have to probably edit their ~/.bashrc to load things), but
that still seems better than the alternative of failing to install
because the installation wants to override root-owned files in spite
of the non-root --prefix.
'make distcheck' still doesn't pass for me; the next failure is
due to attempts to overwrite root-owned ocaml files. Similar symptoms
as what this patch addressed for bashcompsdir, but there we aren't
using pkg-config --variable. Instaed, our definition of ocamllibdir
(based on ${OCAMLLIB} in m4/ocaml.m4) is coming from 'ocamlc -where',
and I have no idea how to coerce that to print a relative pathname,
or even if it is common to have ocaml libraries installed in a non-root
home directory but which can still be picked up by ocaml at runtime.
configure.ac | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 33ed4d5..8c97ffb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -337,7 +337,15 @@ PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [
bash_completion=yes
AC_MSG_CHECKING([for bash-completions directory])
m4_ifdef([PKG_CHECK_VAR],[
- PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir])
+ dnl PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir])
+ dnl Unfortunately, PKG_CHECK_VAR flattens references to ${prefix},
+ dnl which in turn means './configure --prefix=...' to a location
+ dnl writable by non-root still fails when it comes to installing
+ dnl the bash completions.
+ dnl So here, we just run the same command as the macro would, but
+ dnl with an override for prefix to keep 'make distcheck' happy.
+ bashcompdir=`$PKG_CONFIG --define-variable=prefix='${prefix}' \
+ --variable=completionsdir bash-completion 2>/dev/null`
])
AS_IF([test -z "$bashcompdir"], [
bashcompdir="${sysconfdir}/bash_completion.d"
--
2.17.2
5 years, 5 months
[nbdkit PATCH] build: Install ocaml files relative to --prefix
by Eric Blake
Rather than always trying to install ocaml files into $(OCAMLLIBS),
which is likely to be root-owned and therefore fail during a
'./configure --prefix=$HOME/subdir', we instead choose to always
install relative to $(prefix) and let distro packagers take steps
post-install to move the distro's pre-built copy into the correct
location for the distro. This fixes a 'make distcheck' failure.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/ocaml/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plugins/ocaml/Makefile.am b/plugins/ocaml/Makefile.am
index b95f255..bbde5e1 100644
--- a/plugins/ocaml/Makefile.am
+++ b/plugins/ocaml/Makefile.am
@@ -39,7 +39,7 @@ EXTRA_DIST = \
if HAVE_OCAML
-ocamllibdir = $(OCAMLLIB)
+ocamllibdir = $(libdir)/ocaml
ocamllib_DATA = NBDKit.mli NBDKit.cmi NBDKit.cmx NBDKit.o
NBDKit.cmi: NBDKit.mli
--
2.17.2
5 years, 5 months
