Re: [PATCH v2] selinux label: restore all labels when some labels fail to set

On 11/12/20 3:07 PM, Jin Yan wrote: > When migration fails, qemuMigrationDstPrepareAny will call qemuProcessStop > to restore labels only after all labels are successfully set. If some labels > fail to set, the labels that have been set will not be restored. > > Signed-off-by: Jin Yan <jinyan12(a)huawei.com&gt; > --- > src/qemu/qemu_security.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c > index 3bda96272c..0cb90c840a 100644 > --- a/src/qemu/qemu_security.c > +++ b/src/qemu/qemu_security.c > @@ -51,16 +51,24 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver, > incomingPath, > priv->chardevStdioLogd, > migrated) < 0) > - goto cleanup; > + goto restorelabel; > > if (virSecurityManagerTransactionCommit(driver->securityManager, > pid, priv->rememberOwner) < 0) > - goto cleanup; > + goto restorelabel; > > ret = 0; > + > cleanup: > virSecurityManagerTransactionAbort(driver->securityManager); > return ret; > + > + restorelabel: > + virSecurityManagerRestoreAllLabel(driver->securityManager, > + vm->def, > + migrated, > + priv->chardevStdioLogd); > + goto cleanup; > } > > > I don't think this is correct. Firstly, this restores labels for ALL paths, and not just the failed ones (which messes up seclabel remembering and its refcounting), but more importantly: 1) rollback within one secdriver is handled in .transactionCommit callback, well virSecurity*TransactionRun() actually, 2) rollback for other secdrivers after one failed is handled in virSecurityStackSetAllLabel(). Is this not working properly? What version do you run?