On Thu, Nov 19, 2015 at 05:38:25PM +0100, Pino Toscano wrote:
> When running commands in the mounted guest (using the "command" API, and
> APIs based on it), provide the /dev/null from the appliance as open fd
> for stdin. Commands usually assume stdin is open if they didn't close
> it explicitly, so this should avoid crashes or misbehavings due to that.
> ---
> daemon/command.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/daemon/command.c b/daemon/command.c
> index 1593de9..b2d84ca 100644
> --- a/daemon/command.c
> +++ b/daemon/command.c
> @@ -23,6 +23,8 @@
> #include <stdbool.h>
> #include <string.h>
> #include <sys/stat.h>
> +#include <sys/types.h>
> +#include <fcntl.h>
>
> #include "guestfs_protocol.h"
> #include "daemon.h"
> @@ -242,7 +244,7 @@ do_command (char *const *argv)
> {
> char *out;
> CLEANUP_FREE char *err = NULL;
> - int r;
> + int r, fd, flags;
> CLEANUP_BIND_STATE struct bind_state bind_state = { .mounted = false };
> CLEANUP_RESOLVER_STATE struct resolver_state resolver_state =
> { .mounted = false };
> @@ -259,6 +261,17 @@ do_command (char *const *argv)
> return NULL;
> }
>
> + /* Provide /dev/null as stdin for the command, since we want
> + * to make sure processes have an open stdin, and it is not
> + * possible to rely on the guest to provide it (Linux guests
> + * get /dev dynamically populated at runtime by udev).
> + */
> + fd = open ("/dev/null", O_RDONLY|O_CLOEXEC);
> + if (fd == -1) {
> + reply_with_perror ("/dev/null");
> + return NULL;
> + }
> +
> if (bind_mount (&bind_state) == -1)
> return NULL;
> if (enable_network) {
> @@ -266,8 +279,10 @@ do_command (char *const *argv)
> return NULL;
> }
>
> + flags = COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN | fd;
> +
> CHROOT_IN;
> - r = commandv (&out, &err, (const char * const *) argv);
> + r = commandvf (&out, &err, flags, (const char * const *) argv);
> CHROOT_OUT;
>
> free_bind_state (&bind_state);
Looks good. How about naming the variable 'dev_null_fd' or something,
so we know it's not just a temporary file descriptor variable?
Anyway, ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
