[libvirt] [PATCH v2] qemu: Properly label FDs when restoring domain with static label

[Martin Kletzander]

On Thu, May 29, 2014 at 10:42:37AM -0400, Shivaprasad G Bhat wrote:
>The restore of a saved image file fails when the selinux context is static.
>The libvirt has to set the conext of save image file handle to that of
>the guest before handing off the FD to qemu.
>
>Signed-off-by: Shivaprasad G Bhat <shivaprasadbhat at gmail.com>
>---
> src/qemu/qemu_process.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 124fe28..47d1f7d 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -4052,14 +4052,14 @@ int qemuProcessStart(virConnectPtr conn,
>          */
>         struct stat stdin_sb;
>
>-        VIR_DEBUG("setting security label on pipe used for migration");
>+        VIR_DEBUG("setting security label on fd used for migration or restore");
>
>         if (fstat(stdin_fd, &stdin_sb) < 0) {
>             virReportSystemError(errno,
>                                  _("cannot stat fd %d"), stdin_fd);
>             goto cleanup;
>         }
>-        if (S_ISFIFO(stdin_sb.st_mode) &&
>+        if ((S_ISFIFO(stdin_sb.st_mode) || S_ISREG(stdin_sb.st_mode)) &&
>             virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, stdin_fd) < 0)
>             goto cleanup;
>     }
>

Sorry for being so uncertain, but this does not look like what needs to
be done.

Few lines before this code there is virSecurityManagerSetAllLabel()
called.  If the domain is starting with an fd that is not a fifo (thus
already pointing right to the file), the file path is in stdin_path and
that same path should be labeled inside virSecurityManagerSetAllLabel().

I'm not certain this needs fixing as I haven't seen that error with a
scenario that should cause it.

So there are few options what is wrong:

 a) some newer selinux keeps the label on the fd pointing to path even
    when path was relabelled (IIRC it does not happen with older
    versions),

 b) or we have a bug in our code that the path does not get relabelled,
    but it should not be relabelled here,

 c) even if it needs to be relabelled here in the code, the first part
    for the condition you created is effectively always true.  Unless
    resuming from, I don't know, block device or something, in which
    case it would fail as well.

I'd love to make the code fixed, but I'd like to know what is the
scenario that you are trying to fix here.  Maybe the code is exactly as
it needs to be, but I'd like to see an explanation of that in the commit
message if that's the case).  In that case I don't understand why does
it fail with static selinux context only.

Also make sure (with dumpxml) that your machine does not have
relabel="no" in the specification.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140602/4bd1ab6a/attachment-0001.sig>