similar to: [PATCH nbdkit 1/2] server: Call .get_ready before redirecting stdin/stdout to /dev/null.

Enhance the testsuite to ensure we don't regress with recent changes to stdin/out handling. This adds: - test-single-sh.sh: prove that 'nbdkit -s sh script' is viable - test-stdio.sh: create plugin that checks stdin/out match /dev/null, then run it with -s, --run, -f Signed-off-by: Eric Blake <eblake@redhat.com> --- tests/Makefile.am | 23 ++++++

If you have a plugin which either creates background threads itself or uses a library that creates background threads, it turns out you cannot create these in .get_ready (or earlier). The reason is that nbdkit forks when either daemonizing itself or using the --run option, and fork cancels all the background threads in the child process (the daemonized or captive nbdkit). The only good solution

I like this change. I think we were overloading the config_complete method before to do two different things (complete configuration; do any allocation/housekeeping necessary before we can start serving). The only questions in my mind are whether we want this before 1.18, and whether the name ("get_ready") is a good one. Rich.

In v2: - use int instead of bool in the public header - split the tests from the code - don't overload test-layers; instead, add new tests - add a missing fflush exposed by the new tests - other minor cleanups Eric Blake (3): server: Add nbdkit_stdio_safe server: Sanitize stdin/out before running plugin code server: More tests of stdin/out handling docs/nbdkit-plugin.pod |

On 6/28/20 4:35 PM, Richard W.M. Jones wrote: > For completeness here are the other ways I investigated: > > * Make generic read, write, zero, trim, extents operations on local > files. The file-like plugins would consume those directly, after > doing any adjustments eg for offset. Unfortunately the operations > that you have to write are not very "pure"

VDDK creates background threads. fork kills these, resulting in the library hanging or failing completely in certain configurations. --- plugins/vddk/vddk.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index e6639b4d..a481e8bf 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -431,12

The next patch wants to add a filter that will prevent DoS attacks from a plaintext client; to be successful, the filter must guarantee that nbdkit did not settle on SERIALIZE_CONNECTIONS. The easiest way to solve this is to expose the final thread model to .get_ready, which is after the point where .config_complete may have altered it, and before any connections are permitted. Signed-off-by:

On Fri, Aug 07, 2020 at 05:00:52PM -0500, Eric Blake wrote: > The next patch wants to add a filter that will prevent DoS attacks > from a plaintext client; to be successful, the filter must guarantee > that nbdkit did not settle on SERIALIZE_CONNECTIONS. The easiest way > to solve this is to expose the final thread model to .get_ready, which > is after the point where

On 8/10/20 8:01 AM, Richard W.M. Jones wrote: > On Fri, Aug 07, 2020 at 05:00:52PM -0500, Eric Blake wrote: >> The next patch wants to add a filter that will prevent DoS attacks >> from a plaintext client; to be successful, the filter must guarantee >> that nbdkit did not settle on SERIALIZE_CONNECTIONS. The easiest way >> to solve this is to expose the final thread

On Mon, Aug 10, 2020 at 08:13:31AM -0500, Eric Blake wrote: > On 8/10/20 8:01 AM, Richard W.M. Jones wrote: > >On Fri, Aug 07, 2020 at 05:00:52PM -0500, Eric Blake wrote: > >>The next patch wants to add a filter that will prevent DoS attacks > >>from a plaintext client; to be successful, the filter must guarantee > >>that nbdkit did not settle on

Probably needs a bit of cleanup, but seems like it is generally the right direction. One thing I've noticed is that the expect test randomly (but rarely) hangs :-( I guess something is racey but I don't know what at the moment. Rich.

On Sun, Jun 28, 2020 at 4:03 PM Richard W.M. Jones <rjones@redhat.com> wrote: ... > + > +static int > +tar_get_ready (void) > +{ > + FILE *fp; > + CLEANUP_FREE char *cmd = NULL; > + size_t len = 0; > + bool scanned_ok; > + char s[256]; > + > + /* Construct the tar command to examine the tar file. */ > + fp = open_memstream (&cmd, &len); >

In v7: everything should work now! The re-exec code is slightly simplified, with Rich's suggestion to pass the original LD_LIBRARY_PATH rather than just the prefix being added, and I've now finished wiring up the initial dlopen() check into code that correctly computes the right prefix dir to add to LD_LIBRARY_PATH. Eric Blake (1): vddk: Add re-exec with altered environment Richard

This has been broken since we added the reexec code (commit 155af3107292c351d54ed42c732f4a67bb9aa910) because it tried to read the password twice (before and after the reexec) failing the second time because stdin had already been reopened on /dev/null. Virt-v2v used this feature, but I will change virt-v2v instead. --- plugins/vddk/nbdkit-vddk-plugin.pod | 7 +------ plugins/vddk/vddk.c

In the next patch, we want to get rid of the requirement for the user to supply LD_LIBRARY_PATH pointing to vddk libs, if we can derive it ourselves from libdir. However, VDDK itself requires LD_LIBRARY_PATH to be set (because it tries to load libraries that in turn depend on a bare library name, which no manner of dlopen() hacking can work around, and implementing la_objsearch() is no better for

Unfortunately this series doesn't work with the real VDDK library :-( I see: nbdkit: debug: vddk: config_complete nbdkit: debug: dlopen shim prefix set to /home/rjones/tmp/vddk-6.7.3/vmware-vix-disklib-distrib/lib64 Then a little bit later during VDDK initialization it fails with: nbdkit: debug: VixDiskLib: Could not load default plugins from

On 6/2/20 7:27 AM, Richard W.M. Jones wrote: > Use an extensible buffer (a vector<char>) when reading > /proc/self/cmdline. > > Tidy up some error messages. > --- > plugins/vddk/reexec.c | 57 ++++++++++++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 22 deletions(-) > > @@ -80,42 +95,40 @@ perform_reexec (const char *env, const char

On 6/2/20 9:33 AM, Richard W.M. Jones wrote: > On Tue, Jun 02, 2020 at 09:22:49AM -0500, Eric Blake wrote: >> On 6/2/20 7:27 AM, Richard W.M. Jones wrote: >>> Use an extensible buffer (a vector<char>) when reading >>> /proc/self/cmdline. >>> >>> Tidy up some error messages. >>> --- >>> plugins/vddk/reexec.c | 57

I believe this roughly implements Nir's proposal here: https://www.redhat.com/archives/libguestfs/2020-August/msg00028.html Unfortunately it doesn't work for me. It actually slows things down quite a lot, for reasons I don't understand. Note the adjustment of the pool-max parameter and how it affects the total time. The results are quite reproducible. $ ./nbdkit -r -U - vddk

As shown in the previous patch, plugins may choose to use stdin or stdout during .config. But from .get_ready onwards, well-written plugins shouldn't be needing any further use of stdin/out. We already swapped stdin/out to /dev/null while daemonizing, but did not do do during -f or --run, which leads to some surprising inconsistency when trying to debug a plugin that works in the foreground