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

Michal Privoznik mprivozn at redhat.com
Thu Nov 12 18:54:25 UTC 2020


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 at huawei.com>
> ---
>   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?

Michal




More information about the libvir-list mailing list