[libvirt] [PATCH] RFC: security: Make sure to decrease ref count label value

[Michal Privoznik]

On 8/1/19 7:45 PM, Stefan Berger wrote:
> On 7/29/19 5:17 AM, Michal Privoznik wrote:
>> On 7/26/19 8:38 PM, Stefan Berger wrote:
>>> I noticed that if a domain fails to restore, the ref count in the xattr
>>> 'trusted.libvirt.security.ref_selinux' keeps on increasing indefinitely
>>> and the VM will never restore even if the root cause for the restore
>>> failure has been removed. The reason seems to be that the code to 
>>> decrease
>>> the ref count never gets called because the block above it fails due
>>> to virSecuritySELinuxTransactionAppend() failing. The simple solution
>>> seems to be to revert the order in which things are done.
>>>
>>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>>> ---
>>>   src/security/security_selinux.c | 19 +++++++++++--------
>>>   1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> So what was the root cause, did you ever find out?
> 
> 
> The root cause of the failure was basically me (intentionally) changing 
> the password for the encrypted vTPM to a wrong password and then QEMU 
> cannot *resume* 'swtpm' since it cannot decrypt the state. The operation 
> on virt-manager level was a restore of a saved VM.
 >
 > <snip/>
> 
> The root cause was as described above and I wanted to see how QEMU+SWTPM 
> react if I do this.
> 
> 
> What I noticed was that the ...TransactionAppend() failed and didn't get 
> to the code to decrease the ref count, which then ended up increasing 
> forever. So I created this patch, which did indeed solve this particular 
> problem but then caused Signal 11's once I provoked an error with 
> starting (rather than resuming) a VM where again I had changed the 
> password (iirc).
> 

Ah, so TransactionAppend() failed? Huh, looking into the code that can 
happen only in case of OOM at which case all the bets are off IMO. But 
then again, getting malloc() to return NULL is not easy and certainly 
won't happen in the default setup. So I'm wondering what else could make 
TransactionAppend() fail.

Michal