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

[Michal Privoznik]

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 at 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