On Thu, Aug 01, 2019 at 10:42:58PM -0500, Eric Blake wrote: > setenv() is not async-signal-safe and as such should not be used > between fork/exec of a multi-threaded app: if one thread is > manipulating the current environment (which may entail obtaining a > malloc() mutex) when another thread calls fork(), the resulting > child's attempt to use setenv() could deadlock or see a broken environ > because the thread owning the lock no longer exists to release it. > Besides, it is more efficient to update the environment exactly once > in the parent, rather than after every fork(). > > While at it, check for (unlikely) failure of setenv. > > Signed-off-by: Eric Blake <[email protected]> > --- > plugins/sh/call.c | 3 --- > plugins/sh/sh.c | 6 ++++++ > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/plugins/sh/call.c b/plugins/sh/call.c > index 871de5c6..9b8b48e2 100644 > --- a/plugins/sh/call.c > +++ b/plugins/sh/call.c > @@ -127,9 +127,6 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to stdin > */ > /* Restore SIGPIPE back to SIG_DFL, since shell can't undo SIG_IGN */ > signal (SIGPIPE, SIG_DFL); > > - /* Set $tmpdir for the script. */ > - setenv ("tmpdir", tmpdir, 1); > - > execvp (argv[0], (char **) argv); > perror (argv[0]); > _exit (EXIT_FAILURE); > diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c > index 737c38cf..e3d3c2f1 100644 > --- a/plugins/sh/sh.c > +++ b/plugins/sh/sh.c > @@ -60,6 +60,12 @@ sh_load (void) > nbdkit_error ("mkdtemp: /tmp: %m"); > exit (EXIT_FAILURE); > } > + /* Set $tmpdir for the script. */ > + if (setenv ("tmpdir", tmpdir, 1) == -1) { > + nbdkit_error ("setenv: tmpdir=%s: %m", tmpdir); > + exit (EXIT_FAILURE); > + } > + > nbdkit_debug ("sh: load: tmpdir: %s", tmpdir); > }
This sets $tmpdir in the whole nbdkit process ... which may or may not be a problem. It's probably not a problem in reality, but I wonder if it's better to to use ‘execvpe’ (or more portably but more difficult to use ‘execve’) to set the environment in the exec call? I guess we'd have to copy and extend environ in the parent since we want to pass existing environment variables through. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
