[libvirt] [PATCH 2/2] security: Add a new func use stat to get process DAC label

[Martin Kletzander]

On Mon, Dec 01, 2014 at 05:54:36PM +0800, Luyao Huang wrote:
>When use qemuProcessAttach to attach a qemu process, cannot
>get a right DAC label. Add a new func to get process label
>via stat func. Do not remove virDomainDefGetSecurityLabelDef
>before try to use stat to get process DAC label, because
>There are some other func call virSecurityDACGetProcessLabel.
>
>Signed-off-by: Luyao Huang <lhuang at redhat.com>
>---
> src/security/security_dac.c | 50 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
>diff --git a/src/security/security_dac.c b/src/security/security_dac.c
>index 85253af..2977f71 100644
>--- a/src/security/security_dac.c
>+++ b/src/security/security_dac.c
>@@ -1237,17 +1237,63 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
> }
>
> static int
>+virSecurityDACGetProcessLabelInternal(pid_t pid,
>+                                      virSecurityLabelPtr seclabel)
>+{
>+    struct stat sb;
>+    char *path = NULL;
>+    char *label = NULL;
>+    int ret = -1;
>+
>+    VIR_INFO("Getting DAC user and group on process '%d'", pid);
>+
>+    if (virAsprintf(&path, "/proc/%d", (int) pid) < 0)
>+        goto cleanup;
>+

This won't work on systems without /proc.

>+    if (stat(path, &sb) < 0)
>+        goto cleanup;
>+

Better use lstat.

>+    if (virAsprintf(&label, "+%u:+%u",
>+                    (unsigned int) sb.st_uid,
>+                    (unsigned int) sb.st_gid) < 0)
>+        goto cleanup;
>+
>+    if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL)
>+        goto cleanup;
>+    ret = 0;
>+
>+cleanup:
>+    VIR_FREE(path);
>+    VIR_FREE(label);
>+    return ret;
>+}
>+
>+static int
> virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>                               virDomainDefPtr def,
>-                              pid_t pid ATTRIBUTE_UNUSED,
>+                              pid_t pid,
>                               virSecurityLabelPtr seclabel)
> {
>     virSecurityLabelDefPtr secdef =
>         virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
>
>-    if (!secdef || !seclabel)
>+    if (!seclabel)

I wonder whether this won't screw up domain definitions that don't
want to have any seclabel set (those defined with XML), I need to
figure that out.

>         return -1;
>
>+    if (secdef == NULL) {
>+        virReportError(VIR_ERR_INTERNAL_ERROR,
>+                       _("missing label for DAC security "
>+                         "driver in domain %s"), def->name);
>+

This should probably be VIR_DEBUG or VIR_INFO, otherwise you report
error without erroring out (returning -1) and it gets saved for the
connection.

>+        if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0) {
>+            virReportError(VIR_ERR_INTERNAL_ERROR,
>+                           _("Cannot get process %d DAC label"),pid);
>+            return -1;

Also two errors will be reported if this fails.

Martin

>+        }
>+
>+        return 0;
>+    }
>+
>     if (secdef->label)
>         ignore_value(virStrcpy(seclabel->label, secdef->label,
>                                VIR_SECURITY_LABEL_BUFLEN));
>--
>1.8.3.1
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141201/3e12be7f/attachment-0001.sig>