This patch fixes the regression with using named pipes for qemu serial
devices noted in:
https://bugzilla.redhat.com/show_bug.cgi?id=740478
The problem was that, while new code in libvirt looks for a single
bidirectional fifo of the name given in the config, then relabels that
and continues without looking for / relabelling the two unidirectional
fifos named ${name}.in and ${name}.out, qemu looks in the opposite
order. So if the user had naively created all three fifos, libvirt
would relabel the bidirectional fifo to allow qemu access, but qemu
would attempt to use the two unidirectional fifos and fail (because it
didn't have proper permissions/rights).
The solution implemented here is to always look for and chown/relabel
both types of fifos, then let qemu decide which one it wants to use
(in the unusual case that both are present). If one of the types is
present, libvirt will silently ignore when the other type is missing
(since that will be the most common case), but if neither type is
present, there will be an error logged about failing to relabel/chown
one of the bidirectional pipes. (Likewise, if one direction of the
unidirectional pipes is present but the other is missing, this will
also result in an error log).
(Note commit d37c6a3a (which first appeared in libvirt-0.9.2) added
the code that checked for a bidirectional fifo. Prior to that commit,
bidirectional fifos fdor serial devices didn't work unless the fifo
was pre-owned/labelled such that qemu could access it.)
---
src/security/security_dac.c | 39 ++++++++++++++++++++++++++++++---------
src/security/security_selinux.c | 39 ++++++++++++++++++++++++++++++---------
2 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index af02236..f97d2d6 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -397,6 +397,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
char *in = NULL, *out = NULL;
+ bool found_bipipe = false;
int ret = -1;
switch (dev->type) {
@@ -406,19 +407,28 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
+ /* look for / chown both bidirectional pipe and dual uni-directional
+ * pipes if found; let the hypervisor decide which to use.
+ */
if (virFileExists(dev->data.file.path)) {
+ found_bipipe = true;
if (virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group)< 0)
goto done;
- } else {
- if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
- (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
- virReportOOMError();
- goto done;
- }
- if ((virSecurityDACSetOwnership(in, priv->user, priv->group)< 0) ||
- (virSecurityDACSetOwnership(out, priv->user, priv->group)< 0))
- goto done;
}
+ if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
+ (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
+ virReportOOMError();
+ goto done;
+ }
+ if (found_bipipe&&
+ !(virFileExists(in) || virFileExists(out))) {
+ ret = 0;
+ goto done;
+ }
+ if ((virSecurityDACSetOwnership(in, priv->user, priv->group)< 0) ||
+ (virSecurityDACSetOwnership(out, priv->user, priv->group)< 0))
+ goto done;
+
ret = 0;
break;
@@ -438,6 +448,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainChrSourceDefPtr dev)
{
char *in = NULL, *out = NULL;
+ bool found_bipipe = false;
int ret = -1;
switch (dev->type) {
@@ -447,11 +458,21 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
+ if (virFileExists(dev->data.file.path)) {
+ found_bipipe = true;
+ if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path)< 0)
+ goto done;
+ }
if ((virAsprintf(&out, "%s.out", dev->data.file.path)< 0) ||
(virAsprintf(&in, "%s.in", dev->data.file.path)< 0)) {
virReportOOMError();
goto done;
}
+ if (found_bipipe&&
+ !(virFileExists(in) || virFileExists(out))) {
+ ret = 0;
+ goto done;
+ }
if ((virSecurityDACRestoreSecurityFileLabel(out)< 0) ||
(virSecurityDACRestoreSecurityFileLabel(in)< 0))
goto done;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 0807a34..e7a18a6 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -794,6 +794,7 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm,
{
const virSecurityLabelDefPtr secdef =&vm->def->seclabel;
char *in = NULL, *out = NULL;
+ bool found_bipipe = false;
int ret = -1;
if (secdef->norelabel)
@@ -806,19 +807,28 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm,
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
+ /* look for / chown both bidirectional pipe and dual uni-directional
+ * pipes if found; let the hypervisor decide which to use.
+ */
if (virFileExists(dev->data.file.path)) {
+ found_bipipe = true;
if (SELinuxSetFilecon(dev->data.file.path, secdef->imagelabel)< 0)
goto done;
- } else {
- if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
- (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
- virReportOOMError();
- goto done;
- }
- if ((SELinuxSetFilecon(in, secdef->imagelabel)< 0) ||
- (SELinuxSetFilecon(out, secdef->imagelabel)< 0))
- goto done;
}
+ if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
+ (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
+ virReportOOMError();
+ goto done;
+ }
+ if (found_bipipe&&
+ !(virFileExists(in) || virFileExists(out))) {
+ ret = 0;
+ goto done;
+ }
+ if ((SELinuxSetFilecon(in, secdef->imagelabel)< 0) ||
+ (SELinuxSetFilecon(out, secdef->imagelabel)< 0))
+ goto done;
+
ret = 0;
break;
@@ -840,6 +850,7 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm,
{
const virSecurityLabelDefPtr secdef =&vm->def->seclabel;
char *in = NULL, *out = NULL;
+ bool found_bipipe = false;
int ret = -1;
if (secdef->norelabel)
@@ -853,11 +864,21 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm,
ret = 0;
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
+ if (virFileExists(dev->data.file.path)) {
+ found_bipipe = true;
+ if (SELinuxRestoreSecurityFileLabel(dev->data.file.path)< 0)
+ goto done;
+ }
if ((virAsprintf(&out, "%s.out", dev->data.file.path)< 0) ||
(virAsprintf(&in, "%s.in", dev->data.file.path)< 0)) {
virReportOOMError();
goto done;
}
+ if (found_bipipe&&
+ !(virFileExists(in) || virFileExists(out))) {
+ ret = 0;
+ goto done;
+ }
if ((SELinuxRestoreSecurityFileLabel(out)< 0) ||
(SELinuxRestoreSecurityFileLabel(in)< 0))
goto done;