[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 04/12] Add vhost-user-backend
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [RFC v2 04/12] Add vhost-user-backend |
Date: |
Fri, 8 Jun 2018 09:43:25 +0100 |
User-agent: |
Mutt/1.9.5 (2018-04-13) |
On Fri, Jun 08, 2018 at 12:34:15AM +0200, Marc-André Lureau wrote:
> Hi
>
> On Mon, Jun 4, 2018 at 11:36 AM, Daniel P. Berrangé <address@hidden> wrote:
> > On Fri, Jun 01, 2018 at 06:27:41PM +0200, Marc-André Lureau wrote:
> >> Create a vhost-user-backend object that holds a connection to a
> >> vhost-user backend and can be referenced from virtio devices that
> >> support it. See later patches for input & gpu usage.
> >>
> >> A chardev can be specified to communicate with the vhost-user backend,
> >> ex: -chardev socket,id=char0,path=/tmp/foo.sock -object
> >> vhost-user-backend,id=vuid,chardev=char0.
> >>
> >> Alternatively, an executable with its arguments may be given as 'cmd'
> >> property, ex: -object
> >> vhost-user-backend,id=vui,cmd="./vhost-user-input /dev/input..". The
> >> executable is then spawn and, by convention, the vhost-user socket is
> >> passed as fd=3. It may be considered a security breach to allow
> >> creating processes that may execute arbitrary executables, so this may
> >> be restricted to some known executables (via signature etc) or
> >> directory.
> >
> > Passing a binary and args as a string blob.....
> >
> >> +static int
> >> +vhost_user_backend_spawn_cmd(VhostUserBackend *b, int vhostfd, Error
> >> **errp)
> >> +{
> >> + int devnull = open("/dev/null", O_RDWR);
> >> + pid_t pid;
> >> +
> >> + assert(!b->child);
> >> +
> >> + if (!b->cmd) {
> >> + error_setg_errno(errp, errno, "Missing cmd property");
> >> + return -1;
> >> + }
> >> + if (devnull < 0) {
> >> + error_setg_errno(errp, errno, "Unable to open /dev/null");
> >> + return -1;
> >> + }
> >> +
> >> + pid = qemu_fork(errp);
> >> + if (pid < 0) {
> >> + close(devnull);
> >> + return -1;
> >> + }
> >> +
> >> + if (pid == 0) { /* child */
> >> + int fd, maxfd = sysconf(_SC_OPEN_MAX);
> >> +
> >> + dup2(devnull, STDIN_FILENO);
> >> + dup2(devnull, STDOUT_FILENO);
> >> + dup2(vhostfd, 3);
> >> +
> >> + signal(SIGINT, SIG_IGN);
> >
> > Why ignore SIGINT ? Surely we want this extra process to be killed
> > someone ctrl-c's the parent QEMU.
>
> leftover, removed
>
> >
> >> +
> >> + for (fd = 4; fd < maxfd; fd++) {
> >> + close(fd);
> >> + }
> >> +
> >> + execlp("/bin/sh", "sh", "-c", b->cmd, NULL);
> >
> > ...which is then interpreted by the shell is a recipe for security
> > flaws. There needs to be a way to pass the command + arguments
> > to QEMU as an argv[] we can directly exec without involving the
> > shell.
> >
>
> For now, I use g_shell_parse_argv(). Do you have a better idea?
Accept individual args at the cli level is far preferrable - we don't
want anything to be parsing shell strings:
vhost-user-backend,id=vui,binary=/sbin/vhost-user-input,arg=/dev/input,arg=foo,arg=bar
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [Qemu-devel] [RFC v2 01/12] chardev: avoid crash if no associated address, (continued)
[Qemu-devel] [RFC v2 06/12] vhost-user: add vhost_user_input_get_config(), Marc-André Lureau, 2018/06/01
[Qemu-devel] [RFC v2 07/12] libvhost-user: export vug_source_new, Marc-André Lureau, 2018/06/01
[Qemu-devel] [RFC v2 05/12] vhost-user: split vhost_user_read(), Marc-André Lureau, 2018/06/01