On Wed, Mar 18, 2020 at 06:32:16PM +0100, Michal Privoznik wrote: > When a QEMU process dies in the middle of a hotplug, then we fail > to restore the seclabels on the device. The problem is that if > the thread doing hotplug locks the domain object first and thus > blocks the thread that wants to do qemuProcessStop(), the > seclabel cleanup code will see vm->pid still set and mount > namespace used and therefore try to enter the namespace > represented by the PID. But the PID is gone really and thus > entering will fail and no restore is done. What we can do is to > try enter the namespace (if requested to do so) but if entering > fails, fall back to no NS mode. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814481 > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/security/security_dac.c | 16 ++++++++++++---- > src/security/security_selinux.c | 16 ++++++++++++---- > 2 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 9046b51004..11fff63bc7 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -640,15 +640,23 @@ virSecurityDACTransactionCommit(virSecurityManagerPtr mgr G_GNUC_UNUSED, > > list->lock = lock; > > + if (pid != -1) { > + rc = virProcessRunInMountNamespace(pid, > + virSecurityDACTransactionRun, > + list); > + if (rc < 0) { > + if (virGetLastErrorCode() == VIR_ERR_SYSTEM_ERROR) > + pid = -1; > + else > + goto cleanup; > + } > + } > + > if (pid == -1) { > if (lock) > rc = virProcessRunInFork(virSecurityDACTransactionRun, list); > else > rc = virSecurityDACTransactionRun(pid, list); > - } else { > - rc = virProcessRunInMountNamespace(pid, > - virSecurityDACTransactionRun, > - list); > } > > if (rc < 0) > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index c94f31727c..8aeb6e45a5 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -1163,15 +1163,23 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr G_GNUC_UNUSED, > > list->lock = lock; > > + if (pid != -1) { > + rc = virProcessRunInMountNamespace(pid, > + virSecuritySELinuxTransactionRun, > + list); > + if (rc < 0) { > + if (virGetLastErrorCode() == VIR_ERR_SYSTEM_ERROR) > + pid = -1; > + else > + goto cleanup; > + } > + } > + > if (pid == -1) { > if (lock) > rc = virProcessRunInFork(virSecuritySELinuxTransactionRun, list); > else > rc = virSecuritySELinuxTransactionRun(pid, list); > - } else { > - rc = virProcessRunInMountNamespace(pid, > - virSecuritySELinuxTransactionRun, > - list); > } > > if (rc < 0) > -- > 2.24.1 > Reviewed-by: Pavel Mores <pmores@xxxxxxxxxx>
