[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH for 5.7.0 3/3] qemu_blockjob: Restore seclabels more frequently on job events



On 8/30/19 3:42 PM, Peter Krempa wrote:
On Fri, Aug 30, 2019 at 15:19:08 +0200, Michal Privoznik wrote:
If a block job reaches failed/cancelled state, or is completed
without pivot then qemu no longer uses the mirror image. Since
we've set its seclabels we must restore them back to avoid
leaking perms/XATTRs.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
  src/qemu/qemu_blockjob.c | 25 +++++++++++++++++++++++--
  1 file changed, 23 insertions(+), 2 deletions(-)



@@ -1124,7 +1138,8 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
static void
-qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
+                                           virDomainObjPtr vm,
                                             qemuBlockJobDataPtr job)
  {
      VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name, vm->def->name);
@@ -1132,6 +1147,12 @@ qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
      if (!job->disk)
          return;
+ /* QEMU no longer uses the image, so we can restore its label. */
+    if (qemuSecurityRestoreImageLabel(driver, vm, job->disk->mirror, true) < 0) {
+        VIR_WARN("Unable to restore security labels on vm %s disk %s",
+                 vm->def->name, NULLSTR(job->disk->mirror->path));

So, here this must return the security labels to readonly state rather
than removing it completely as this is still used.

That is not implemented :-( Our virSecuritySELinuxRestoreImageLabel() acts like 'restorecon $path' which is not what we need here. You know what? I'll remove the XATTRs in all three cases (at least for the time being) as it's safe to do so (the worst thing is that we won't restore the original label - but we weren't doing that anyway). For the first two (legacy mode) we can't know if somebody else is not using the backing chain, and in this blockdev case we could know that but that would require bigger fix.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]