[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd

On 06/26/2012 05:10 AM, Daniel P. Berrange wrote:
On Fri, Jun 22, 2012 at 02:36:07PM -0400, Corey Bryant wrote:
libvirt's sVirt security driver provides SELinux MAC isolation for
Qemu guest processes and their corresponding image files.  In other
words, sVirt uses SELinux to prevent a QEMU process from opening
files that do not belong to it.

sVirt provides this support by labeling guests and resources with
security labels that are stored in file system extended attributes.
Some file systems, such as NFS, do not support the extended
attribute security namespace, and therefore cannot support sVirt

A solution to this problem is to provide fd passing support, where
libvirt opens files and passes file descriptors to QEMU.  This,
along with SELinux policy to prevent QEMU from opening files, can
provide image file isolation for NFS files stored on the same NFS

This patch series adds the pass-fd QMP monitor command, which allows
an fd to be passed via SCM_RIGHTS, and returns the received file
descriptor.  Support is also added to the block layer to allow QEMU
to dup the fd when the filename is of the /dev/fd/X format.  This
is useful if MAC policy prevents QEMU from opening specific types
of files.

I was thinking about some of the sources complexity when using
FD passing from libvirt and wanted to raise one idea for discussion
before we continue.

With this proposed series, we have usage akin to:

   1. pass_fd FDSET={M} -> returns a string "/dev/fd/N" showing QEMU's
      view of the FD
   2. drive_add file=/dev/fd/N
   3. if failure:
        close_fd "/dev/fd/N"

My problem is that none of this FD passing is "transactional".
If libvirtd crashes or otherwise fails between steps 1 & 2,
a FD is left open in QEMU.  If libvirtd gets the failure
detection wrong in step 2 (eg sees a I/O failure on the monitor,
but from QEMU's pov drive_add succeeed), we could end up
telling QEMU to close an FD that it is still using for a
drive. Likewise if libvirtd fails/crashes between steps 2 & 3
we might not clean up after failure.

I see what you're saying, but if libvirt crashes it seems like there are bigger issues going on than a leaked fd. If libvirt fails, then it should call closefd to prevent leakage.

I don't know if it really buys that much of an advantage though. I think one major advantage to having separate commands is that other commands can use an fd passed by pass-fd, not just drive_add.

These aren't new problems with pass_fd - they existed with
getfd too of course.

If we were designing this interface with no regard for the
historical practice in QEMU, then I feel like we would not
even bother to have either 'pass_fd' or 'getfd'. We would
pass the FD(s) directly with the "drive_add" command.

Given that we have decided that attaching special semantics
to filenames matching "/dev/fd/N" is OK, then I feel we could
go one better, and allow the FD to be passed with the "drive_add"
(or other) commands directly. All we need do is define slightly
different semantics for "/dev/fd/N". Instead of it meaning
"use the process FD numbered N", we can define it to mean
"use the n'th FD set in the current context". The "context"
would be populated with all FDs received with the monitor
current command.

So now from a client's POV you'd have a flow like

    * drive_add "file=/dev/fd/N"  FDSET={N}

IIUC then drive_add would loop and pass each fd in the set via SCM_RIGHTS?

And in QEMU you'd have something like

    * handle_monitor_command
         - recvmsg all FDs, and stash them in a thread local "FDContext"
         - invoke monitor command handler
               - Sees file=/dev/fd/N
               - Fetch /dev/fd/N from "FDContext"
               - If success remove /dev/fd/N from "FDContext"
         - close() all FDs left in "FDContext"

The key point with this is that because the FDs are directly
associated with a monitor command, QEMU can /guarantee/ that
FDs are never leaked, regardless of client behaviour.

Wouldn't this leak fds if libvirt crashed part way through sending the set of fds?


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]