Re: [Qemu-devel] [PATCH] vhost: secure vhost shared log files using argv paremeter

[Rafael David Tinoco <rafael.tinoco@canonical.com>]

On Sun, Oct 30, 2016 at 5:26 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Oct 22, 2016 at 07:00:41AM +0000, Rafael David Tinoco wrote:
> > Commit 31190ed7 added a migration blocker in vhost_dev_init() to
> > check if memfd would succeed. It is better if this blocker first
> > checks if vhost backend requires shared log. This will avoid a
> > situation where a blocker is added inappropriately (e.g. shared
> > log allocation fails when vhost backend doesn't support it).
>
> Sounds like a bugfix but I'm not sure. Can this part be split
> out in a patch by itself?

Already sent some days ago (and pointed by Marc today).

> > Commit: 35f9b6e added a fallback mechanism for systems not supporting
> > memfd_create syscall (started being supported since 3.17).
> >
> > Backporting memfd_create might not be accepted for distros relying
> > on older kernels. Nowadays there is no way for security driver
> > to discover memfd filename to be created: <tmpdir>/memfd-XXXXXX.
> >
> > Also, because vhost log file descriptors can be passed to other
> > processes, after discussion, we thought it is best to back mmap by
> > using files that can be placed into a specific directory: this commit
> > creates "vhostlog" argv parameter for such purpose. This will allow
> > security drivers to operate on those files appropriately.
> >
> > Argv examples:
> >
> >     -netdev tap,id=net0,vhost=on
> >     -netdev tap,id=net0,vhost=on,vhostlog=/tmp/guest.log
> >     -netdev tap,id=net0,vhost=on,vhostlog=/tmp
> >
> > For vhost backends supporting shared logs, if vhostlog is non-existent,
> > or a directory, random files are going to be created in the specified
> > directory (or, for non-existent, in tmpdir). If vhostlog is specified,
> > the filepath is always used when allocating vhost log files.
>
> When vhostlog is not specified, can we just use memfd as we did?
>

This was my approach on a "pastebin" example before this patch (in the
discussion thread we had). Problem goes back to when vhost log file
descriptor is shared with some vhost-user implementation - like the
interface allows to - and the security driver labelling issue. IMO,
yes, we could let vhostlog to specify a log file, and, if not
specified, assume memfd is ok to be used.

Please let me know if you - and Marc - want me to keep using memfd.
I'll create the mmap-file tests and files in a different commit, like
Marc has asked for, and will propose the patch again by the end of
this week.