Obtaining a screenshot via virDomainScreenshot() works like this:
  1) we create a temp file, label it, then
  2) tell QEMU to store the screenshot into it, and
  3) finally, open the file for transfer via virStream

Since the file is just temporary and even explicitly unlinked at
the end, no seclabel restoration is done. This makes perfect
sense for security models which attach a label to file itself
(DAC, SELinux) because the label is gone with the file. However,
for models where a list of files and allowed actions is kept on a
side (AppArmor) this approach means we just append files into the
profile and never remove them. In turn, the file grows and policy
update takes longer with each entry.

Restore the seclabel for AppArmor's sake.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_driver.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fb63e6550f..0c6645ed89 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3423,8 +3423,13 @@ qemuDomainScreenshot(virDomainPtr dom,
 
  endjob:
     VIR_FORCE_CLOSE(tmp_fd);
-    if (unlink_tmp)
+    if (unlink_tmp) {
+        /* This may look pointless, since we're removing the file anyways, but
+         * it's crucial for AppArmor. Otherwise these temp files would
+         * accumulate in the domain's profile. */
+        qemuSecurityDomainRestorePathLabel(driver, vm, tmp);
         unlink(tmp);
+    }
 
     qemuDomainObjEndJob(vm);
 
-- 
2.35.1

On Thu, Jun 02, 2022 at 15:14:25 +0200, Michal Privoznik wrote:
> Obtaining a screenshot via virDomainScreenshot() works like this:
>   1) we create a temp file, label it, then
>   2) tell QEMU to store the screenshot into it, and
>   3) finally, open the file for transfer via virStream
> 
> Since the file is just temporary and even explicitly unlinked at
> the end, no seclabel restoration is done. This makes perfect
> sense for security models which attach a label to file itself
> (DAC, SELinux) because the label is gone with the file. However,
> for models where a list of files and allowed actions is kept on a
> side (AppArmor) this approach means we just append files into the
> profile and never remove them. In turn, the file grows and policy
> update takes longer with each entry.
> 
> Restore the seclabel for AppArmor's sake.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_driver.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>