From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55145) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1L6g-0007QN-02 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 18:31:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1L6c-0004jn-U8 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 18:31:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50634) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1L6c-0004is-NA for qemu-devel@nongnu.org; Mon, 31 Oct 2016 18:31:02 -0400 Date: Tue, 1 Nov 2016 00:30:59 +0200 From: "Michael S. Tsirkin" Message-ID: <20161101002844-mutt-send-email-mst@kernel.org> References: <20161022070041.23763-1-rafael.tinoco@canonical.com> <20161030192651.sh7gydgo6r4a42ml@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] vhost: secure vhost shared log files using argv paremeter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Rafael David Tinoco Cc: qemu-devel@nongnu.org, marcandre.lureau@redhat.com, 1626972@bugs.launchpad.net On Mon, Oct 31, 2016 at 08:35:33AM -0200, Rafael David Tinoco wrote: > On Sun, Oct 30, 2016 at 5:26 PM, Michael S. Tsirkin 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: /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. I think that the best approach is to allow passing in the fd, not the file path. If not passed, use memfd. -- MST