Our commit e13e8808f9 was way too generic. Currently, virtlogd is
used only for chardevs type of file and nothing else. True, we
must not relabel the path in this case, but we have to in all
other cases. For instance, if you want to have a physical console
attached to your guest:

    <console type='dev'>
      <source path='/dev/ttyS0'/>
      <target type='virtio' port='1'/>
    </console>

Starting such domain fails because qemu doesn't have access to
/dev/ttyS0 because we haven't relabelled the path.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/security/security_dac.c     | 8 ++++++--
 src/security/security_selinux.c | 8 ++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 79941f480..ca7a6af6d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1179,7 +1179,9 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
-    if (!chr_seclabel && chardevStdioLogd)
+    if (!chr_seclabel &&
+        dev_source->type == VIR_DOMAIN_CHR_TYPE_FILE &&
+        chardevStdioLogd)
         return 0;
 
     if (chr_seclabel && chr_seclabel->label) {
@@ -1261,7 +1263,9 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
-    if (!chr_seclabel && chardevStdioLogd)
+    if (!chr_seclabel &&
+        dev_source->type == VIR_DOMAIN_CHR_TYPE_FILE &&
+        chardevStdioLogd)
         return 0;
 
     switch ((virDomainChrType) dev_source->type) {
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 26137f6d8..2e3082b7a 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2199,7 +2199,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
-    if (!chr_seclabel && chardevStdioLogd)
+    if (!chr_seclabel &&
+        dev_source->type == VIR_DOMAIN_CHR_TYPE_FILE &&
+        chardevStdioLogd)
         return 0;
 
     if (chr_seclabel)
@@ -2274,7 +2276,9 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
-    if (!chr_seclabel && chardevStdioLogd)
+    if (!chr_seclabel &&
+        dev_source->type == VIR_DOMAIN_CHR_TYPE_FILE &&
+        chardevStdioLogd)
         return 0;
 
     switch (dev_source->type) {
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/22/2017 07:55 AM, Michal Privoznik wrote:
> Our commit e13e8808f9 was way too generic. Currently, virtlogd is
> used only for chardevs type of file and nothing else. True, we
> must not relabel the path in this case, but we have to in all
> other cases. For instance, if you want to have a physical console
> attached to your guest:
> 
>     <console type='dev'>
>       <source path='/dev/ttyS0'/>
>       <target type='virtio' port='1'/>
>     </console>
> 
> Starting such domain fails because qemu doesn't have access to
> /dev/ttyS0 because we haven't relabelled the path.

It's relabeled in my spell checker ;-)

> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/security/security_dac.c     | 8 ++++++--
>  src/security/security_selinux.c | 8 ++++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 

<sigh> - I guess I was on the right track in my review of :

https://www.redhat.com/archives/libvir-list/2017-June/msg00581.html

but couldn't quite convey enough context over my concern nor truly
follow "all" the various options for chardev's and source usage in the
formatdomain description (a different problem, but suffice to say
confusing at best).

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list