Re: [libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files

2015-06-22 Thread Michał Dubiel
On 19 June 2015 at 21:30, Serge Hallyn serge.hal...@ubuntu.com wrote:

 Quoting Michal Dubiel (m...@semihalf.com):
  QEMU working in vhost-user mode communicates with the other end (i.e.
  some virtual router application) via unix domain sockets. This requires
  that permissions for the socket files are correctly written into
  /etc/apparmor.d/libvirt/libvirt-UUID.files.
 
  Signed-off-by: Michal Dubiel m...@semihalf.com
  ---
   src/security/virt-aa-helper.c | 24 +---
   1 file changed, 13 insertions(+), 11 deletions(-)
 
  diff --git a/src/security/virt-aa-helper.c
 b/src/security/virt-aa-helper.c
  index 35423b5..a097aa6 100644
  --- a/src/security/virt-aa-helper.c
  +++ b/src/security/virt-aa-helper.c
  @@ -592,19 +592,9 @@ valid_path(const char *path, const bool readonly)
 
   if (!virFileExists(path)) {
   vah_warning(_(path does not exist, skipping file type
 checks));
  -} else {
  -if (stat(path, sb) == -1)
  +} else if (stat(path, sb) == -1)
   return -1;

 Hi,

 Why keep this bit?  sb is not used later in the fn, and you
 already know that access(2) didn't return ENOENT.


You are right, it is not needed. Thanks for pointing this out. I will
update the patch accordingly.

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

Re: [libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files

2015-06-22 Thread Michał Dubiel

 Besides by changing from :

 } else {
 if ((x) == -1))
 return -1;

 to

 } else if ((x) == -1) {
 return -1;

 Everything else inside the else if after the return -1 becomes dead


Yes it becomes dead, but there is nothing left in the 'else if' block.

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

[libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files

2015-06-19 Thread Michal Dubiel
QEMU working in vhost-user mode communicates with the other end (i.e.
some virtual router application) via unix domain sockets. This requires
that permissions for the socket files are correctly written into
/etc/apparmor.d/libvirt/libvirt-UUID.files.

Signed-off-by: Michal Dubiel m...@semihalf.com
---
 src/security/virt-aa-helper.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35423b5..a097aa6 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -592,19 +592,9 @@ valid_path(const char *path, const bool readonly)
 
 if (!virFileExists(path)) {
 vah_warning(_(path does not exist, skipping file type checks));
-} else {
-if (stat(path, sb) == -1)
+} else if (stat(path, sb) == -1)
 return -1;
 
-switch (sb.st_mode  S_IFMT) {
-case S_IFSOCK:
-return 1;
-break;
-default:
-break;
-}
-}
-
 opaths = sizeof(override)/sizeof(*(override));
 
 npaths = sizeof(restricted)/sizeof(*(restricted));
@@ -1101,6 +1091,18 @@ get_files(vahControl * ctl)
 }
 }
 
+for (i = 0; i  ctl-def-nnets; i++) {
+if (ctl-def-nets[i] 
+ctl-def-nets[i]-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER 
+ctl-def-nets[i]-data.vhostuser) {
+virDomainChrSourceDefPtr vhu = ctl-def-nets[i]-data.vhostuser;
+
+if (vah_add_file_chardev(buf, vhu-data.nix.path, rw,
+   vhu-type) != 0)
+goto cleanup;
+}
+}
+
 if (ctl-def-virtType == VIR_DOMAIN_VIRT_KVM) {
 for (i = 0; i  ctl-def-nnets; i++) {
 virDomainNetDefPtr net = ctl-def-nets[i];
-- 
1.9.1

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


Re: [libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files

2015-06-19 Thread Serge Hallyn
Quoting Michal Dubiel (m...@semihalf.com):
 QEMU working in vhost-user mode communicates with the other end (i.e.
 some virtual router application) via unix domain sockets. This requires
 that permissions for the socket files are correctly written into
 /etc/apparmor.d/libvirt/libvirt-UUID.files.
 
 Signed-off-by: Michal Dubiel m...@semihalf.com
 ---
  src/security/virt-aa-helper.c | 24 +---
  1 file changed, 13 insertions(+), 11 deletions(-)
 
 diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
 index 35423b5..a097aa6 100644
 --- a/src/security/virt-aa-helper.c
 +++ b/src/security/virt-aa-helper.c
 @@ -592,19 +592,9 @@ valid_path(const char *path, const bool readonly)
  
  if (!virFileExists(path)) {
  vah_warning(_(path does not exist, skipping file type checks));
 -} else {
 -if (stat(path, sb) == -1)
 +} else if (stat(path, sb) == -1)
  return -1;

Hi,

Why keep this bit?  sb is not used later in the fn, and you
already know that access(2) didn't return ENOENT.

  
 -switch (sb.st_mode  S_IFMT) {
 -case S_IFSOCK:
 -return 1;
 -break;
 -default:
 -break;
 -}
 -}
 -
  opaths = sizeof(override)/sizeof(*(override));
  
  npaths = sizeof(restricted)/sizeof(*(restricted));
 @@ -1101,6 +1091,18 @@ get_files(vahControl * ctl)
  }
  }
  
 +for (i = 0; i  ctl-def-nnets; i++) {
 +if (ctl-def-nets[i] 
 +ctl-def-nets[i]-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER 
 +ctl-def-nets[i]-data.vhostuser) {
 +virDomainChrSourceDefPtr vhu = ctl-def-nets[i]-data.vhostuser;
 +
 +if (vah_add_file_chardev(buf, vhu-data.nix.path, rw,
 +   vhu-type) != 0)
 +goto cleanup;
 +}
 +}
 +
  if (ctl-def-virtType == VIR_DOMAIN_VIRT_KVM) {
  for (i = 0; i  ctl-def-nnets; i++) {
  virDomainNetDefPtr net = ctl-def-nets[i];
 -- 
 1.9.1
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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


Re: [libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files

2015-06-19 Thread John Ferlan


On 06/19/2015 03:30 PM, Serge Hallyn wrote:
 Quoting Michal Dubiel (m...@semihalf.com):
 QEMU working in vhost-user mode communicates with the other end (i.e.
 some virtual router application) via unix domain sockets. This requires
 that permissions for the socket files are correctly written into
 /etc/apparmor.d/libvirt/libvirt-UUID.files.

 Signed-off-by: Michal Dubiel m...@semihalf.com
 ---
  src/security/virt-aa-helper.c | 24 +---
  1 file changed, 13 insertions(+), 11 deletions(-)

 diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
 index 35423b5..a097aa6 100644
 --- a/src/security/virt-aa-helper.c
 +++ b/src/security/virt-aa-helper.c
 @@ -592,19 +592,9 @@ valid_path(const char *path, const bool readonly)
  
  if (!virFileExists(path)) {
  vah_warning(_(path does not exist, skipping file type checks));
 -} else {
 -if (stat(path, sb) == -1)
 +} else if (stat(path, sb) == -1)
  return -1;
 
 Hi,
 
 Why keep this bit?  sb is not used later in the fn, and you
 already know that access(2) didn't return ENOENT.
 

Besides by changing from :

} else {
if ((x) == -1))
return -1;

to

} else if ((x) == -1) {
return -1;

Everything else inside the else if after the return -1 becomes dead

John
  
 -switch (sb.st_mode  S_IFMT) {
 -case S_IFSOCK:
 -return 1;
 -break;
 -default:
 -break;
 -}
 -}
 -
  opaths = sizeof(override)/sizeof(*(override));
  
  npaths = sizeof(restricted)/sizeof(*(restricted));
 @@ -1101,6 +1091,18 @@ get_files(vahControl * ctl)
  }
  }
  
 +for (i = 0; i  ctl-def-nnets; i++) {
 +if (ctl-def-nets[i] 
 +ctl-def-nets[i]-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER 
 +ctl-def-nets[i]-data.vhostuser) {
 +virDomainChrSourceDefPtr vhu = 
 ctl-def-nets[i]-data.vhostuser;
 +
 +if (vah_add_file_chardev(buf, vhu-data.nix.path, rw,
 +   vhu-type) != 0)
 +goto cleanup;
 +}
 +}
 +
  if (ctl-def-virtType == VIR_DOMAIN_VIRT_KVM) {
  for (i = 0; i  ctl-def-nnets; i++) {
  virDomainNetDefPtr net = ctl-def-nets[i];
 -- 
 1.9.1

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

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