[libvirt] [PATCH] security: apparmor: Properly link with storage driver in helper program

Peter Krempa posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/2e18140b97623eee57ac5bc7c1253746d1d17f94.1500375034.git.pkrempa@redhat.com
src/Makefile.am               | 2 +-
src/security/virt-aa-helper.c | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
[libvirt] [PATCH] security: apparmor: Properly link with storage driver in helper program
Posted by Peter Krempa 6 years, 9 months ago
The refactor to split up storage driver into modules broke the apparmor
helper program, since that did not initialize the storage driver
properly and thus detection of the backing chain could not work.

Register the storage driver backends explicitly. Unfortunately it's now
necessary to link with the full storage driver to satisfy dependencies
of the loadable modules.
---
 src/Makefile.am               | 2 +-
 src/security/virt-aa-helper.c | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 399d031dd..e637dfd91 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -3281,7 +3281,7 @@ virt_aa_helper_LDADD =						\
 		libvirt.la					\
 		libvirt_conf.la					\
 		libvirt_util.la					\
-		libvirt_driver_storage_impl.la			\
+		libvirt_driver_storage.la			\
 		../gnulib/lib/libgnu.la
 if WITH_DTRACE_PROBES
 virt_aa_helper_LDADD += libvirt_probes.lo
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 695272076..a751d6deb 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -56,6 +56,7 @@
 #include "virgettext.h"

 #include "storage/storage_source.h"
+#include "storage/storage_backend.h"

 #define VIR_FROM_THIS VIR_FROM_SECURITY

@@ -926,6 +927,11 @@ get_files(vahControl * ctl)
         goto cleanup;
     }

+    if (virStorageBackendDriversRegister(false) < 0) {
+        vah_error(ctl, 0, _("failed to register storage driver backend"));
+        goto cleanup;
+    }
+
     for (i = 0; i < ctl->def->ndisks; i++) {
         virDomainDiskDefPtr disk = ctl->def->disks[i];

@@ -1283,6 +1289,8 @@ main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }

+    virFileActivateDirOverride(argv[0]);
+
     /* Initialize the log system */
     virLogSetFromEnv();

-- 
2.13.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security: apparmor: Properly link with storage driver in helper program
Posted by Christian Ehrhardt 6 years, 9 months ago
On Tue, Jul 18, 2017 at 12:51 PM, Peter Krempa <pkrempa@redhat.com> wrote:

> The refactor to split up storage driver into modules broke the apparmor
> helper program, since that did not initialize the storage driver
> properly and thus detection of the backing chain could not work.
>
> Register the storage driver backends explicitly. Unfortunately it's now
> necessary to link with the full storage driver to satisfy dependencies
> of the loadable modules.
>
>
Hi I tested:
- on master direct virt-aa-helper calls
- on Ubuntu+patch direct virt-aa-helper calls
- on Ubuntu+patch start of confined guest with disks that have BackingStores
- built for all architectures Ubuntu's libvirt supports (amd64 arm64 armhf
i386 ppc64el s390x)

All worked thanks to the patch (which was created in response to [1])
Please pick commit tags as you consider appropriate, I guess you won't need
all three, but I happily give them all to you :-)
Thanks a lot Peter!

Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

[1]: https://www.redhat.com/archives/libvir-list/2017-July/msg00604.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security: apparmor: Properly link with storage driver in helper program
Posted by Peter Krempa 6 years, 9 months ago
On Tue, Jul 18, 2017 at 18:35:55 +0200, Christian Ehrhardt wrote:
> On Tue, Jul 18, 2017 at 12:51 PM, Peter Krempa <pkrempa@redhat.com> wrote:
> 
> > The refactor to split up storage driver into modules broke the apparmor
> > helper program, since that did not initialize the storage driver
> > properly and thus detection of the backing chain could not work.
> >
> > Register the storage driver backends explicitly. Unfortunately it's now
> > necessary to link with the full storage driver to satisfy dependencies
> > of the loadable modules.
> >
> >
> Hi I tested:
> - on master direct virt-aa-helper calls
> - on Ubuntu+patch direct virt-aa-helper calls
> - on Ubuntu+patch start of confined guest with disks that have BackingStores
> - built for all architectures Ubuntu's libvirt supports (amd64 arm64 armhf
> i386 ppc64el s390x)
> 
> All worked thanks to the patch (which was created in response to [1])
> Please pick commit tags as you consider appropriate, I guess you won't need
> all three, but I happily give them all to you :-)
> Thanks a lot Peter!

No problem :) Sorry for breaking it in the first place.

> 
> Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

I tweaked this to the more commonly used "Reviewed-by"

> Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

and pushed this patch.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list