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

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. v2 add support freeBSD. v3 use snprintf instead of VirAsprintf and move the error settings in virSecurityDACGetProcessLabelInternal. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/security/security_dac.c | 85 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..8fd1e6e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -22,6 +22,12 @@ #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> +#include <errno.h> + +#ifdef __FreeBSD__ +# include <sys/sysctl.h> +# include <sys/user.h> +#endif #include "security_dac.h" #include "virerror.h" @@ -1236,17 +1242,90 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; } +#ifdef __linux__ +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct stat sb; + char *path = 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; + + if (lstat(path, &sb) < 0) { + virReportSystemError(errno, + _("unable to get PID %d uid and gid via stat"), + pid); + goto cleanup; + } + + snprintf(seclabel->label, VIR_SECURITY_LABEL_BUFLEN, + "+%u:+%u", (unsigned int) sb.st_uid, (unsigned int) sb.st_gid); + ret = 0; + +cleanup: + VIR_FREE(path); + return ret; +} +#elif defined(__FreeBSD__) +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct kinfo_proc p; + int mib[4]; + size_t len = 4; + + sysctlnametomib("kern.proc.pid", mib, &len); + + len = sizeof(struct kinfo_proc); + mib[3] = pid; + + if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) { + virReportSystemError(errno, + _("unable to get PID %d uid and gid via sysctl"), + pid); + return -1; + } + + snprintf(seclabel->label, VIR_SECURITY_LABEL_BUFLEN, + "+%u:+%u", (unsigned int) p.ki_ruid, (unsigned int) p.ki_rgid); + + return 0; +} +#else +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + "Cannot get proccess DAC label for pid %d on this platform", + (int) pid); + return -1; +} +#endif + 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) - return -1; + if (secdef == NULL) { + VIR_DEBUG("missing label for DAC security " + "driver in domain %s", def->name); + + if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0) + return -1; + return 0; + } if (secdef->label) ignore_value(virStrcpy(seclabel->label, secdef->label, -- 1.8.3.1

On 12/05/2014 01:20 AM, 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.
v2 add support freeBSD. v3 use snprintf instead of VirAsprintf and move the error settings in virSecurityDACGetProcessLabelInternal.
v2 and v3 comments belong...
Signed-off-by: Luyao Huang <lhuang@redhat.com> ---
...here, where they are visible to reviewers, but will not be included in git history (when the committer uses 'git am', anything after --- is stripped). A year from now, we won't care how many preliminary versions of a patch went to the list, only the one version that is in git.
src/security/security_dac.c | 85 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 3 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..8fd1e6e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -22,6 +22,12 @@ #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> +#include <errno.h>
Isn't errno.h implicitly included by our generic headers, such as "internal.h"? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 12/06/2014 12:50 AM, Eric Blake wrote:
On 12/05/2014 01:20 AM, 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.
v2 add support freeBSD. v3 use snprintf instead of VirAsprintf and move the error settings in virSecurityDACGetProcessLabelInternal.
v2 and v3 comments belong...
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- ...here, where they are visible to reviewers, but will not be included in git history (when the committer uses 'git am', anything after --- is stripped). A year from now, we won't care how many preliminary versions of a patch went to the list, only the one version that is in git. Got it, i will move the version information after --- in next version
thanks a lot for your help :)
src/security/security_dac.c | 85 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 3 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..8fd1e6e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -22,6 +22,12 @@ #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> +#include <errno.h> Isn't errno.h implicitly included by our generic headers, such as "internal.h"? Yes, i notice errno.h included in internal.h, and internal.h include in virerror.h. So this place include is unnecessary, thanks for pointing out.
participants (2)
-
Eric Blake
-
Luyao Huang