Right now we're using the virRun() convenience API, but that doesn't allow the kind of control we want. Use the virCommand APIs directly instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/virt-aa-helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index ad9a7dda94..c5080f698a 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -170,7 +170,9 @@ parserCommand(const char *profile_name, const char cmd) const char * const argv[] = { "/sbin/apparmor_parser", flag, profile, NULL }; - if ((ret = virRun(argv, &status)) != 0 || + VIR_AUTOPTR(virCommand) command = virCommandNewArgs(argv); + + if ((ret = virCommandRun(command, &status)) != 0 || (WIFEXITED(status) && WEXITSTATUS(status) != 0)) { if (ret != 0) { vah_error(NULL, 0, _("failed to run apparmor_parser")); -- 2.21.0

The way we're processing the return status, using WIFEXITED() and friends, only works when we have the raw return status; however, virCommand defaults to processing the return status for us. Call virCommandRawStatus() before virCommandRun() so that we get the raw return status and the logic can actually work. This results in guest startup failures caused by AppArmor issues being reported much earlier: for example, if virt-aa-helper exits with an error we're now reporting error: internal error: cannot load AppArmor profile 'libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6' instead of the misleading error: internal error: Process exited prior to exec: libvirt: error : unable to set AppArmor profile 'libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6' for '/usr/bin/qemu-system-x86_64': No such file or directory Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index c5080f698a..60c9b75980 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -172,6 +172,7 @@ parserCommand(const char *profile_name, const char cmd) }; VIR_AUTOPTR(virCommand) command = virCommandNewArgs(argv); + virCommandRawStatus(command); if ((ret = virCommandRun(command, &status)) != 0 || (WIFEXITED(status) && WEXITSTATUS(status) != 0)) { if (ret != 0) { -- 2.21.0

Since commit 432faf259b696043ee5d7e8f657d855419a9a3fa Author: Michal Privoznik <mprivozn@redhat.com> Date: Tue Jul 2 19:49:51 2019 +0200 virCommand: use procfs to learn opened FDs When spawning a child process, between fork() and exec() we close all file descriptors and keep only those the caller wants us to pass onto the child. The problem is how we do that. Currently, we get the limit of opened files and then iterate through each one of them and either close() it or make it survive exec(). This approach is suboptimal (although, not that much in default configurations where the limit is pretty low - 1024). We have /proc where we can learn what FDs we hold open and thus we can selectively close only those. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> v5.5.0-173-g432faf259b programs using the virCommand APIs on Linux need read access to /proc/self/fd, or they will fail like error : virCommandWait:2796 : internal error: Child process (LIBVIRT_LOG_OUTPUTS=3:stderr /usr/lib/libvirt/virt-aa-helper -c -u libvirt-b20e9a8e-091a-45e0-8823-537119e98bc6) unexpected exit status 1: libvirt: error : cannot open directory '/proc/self/fd': Permission denied virt-aa-helper: error: apparmor_parser exited with error Update the AppArmor profile for virt-aa-helper so that read access to the relevant path is granted. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/security/apparmor/usr.lib.libvirt.virt-aa-helper | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper index bf6bd297d1..d81dddef30 100644 --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper @@ -17,6 +17,10 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { owner @{PROC}/[0-9]*/status r, @{PROC}/filesystems r, + # Used when internally running another command (namely apparmor_parser) + @{PROC}/self/fd r, + @{PROC}/@{pid}/fd r, + /etc/libnl-3/classid r, # for gl enabled graphics -- 2.21.0