[libvirt] [PATCH 7/8] secret: Properly handle @def after virSecretObjAdd in driver

[Michal Privoznik]

On 07/13/2017 06:54 PM, John Ferlan wrote:
> 
> 
> On 07/11/2017 11:52 AM, Michal Privoznik wrote:
>> On 06/03/2017 03:27 PM, John Ferlan wrote:
>>> Since the virSecretObjListAdd technically consumes @def on success,
>>> the secretDefineXML should fetch the @def from the object afterwards
>>> and manage as an @objdef and set @def = NULL immediately.
>>
>> Really? virSecretObjListAdd sets ->def pointer in the object, but it
>> doesn't touch the definition otherwise. So I think a caller is safe to
>> continue using the pointer.
>>
>> Michal
>>
> 
> Let's consider the code before my change...
> 
> @def is added to the @obj
> 
> "Something" causes us to jump to the "restore_backup:" label and @backup
> == NULL.
> 
> That means virSecretObjListRemove runs and because @obj has @def it ends
> up calling virSecretDefFree

The only way this can happen is when @obj has refcnt == 1. Because then
unref() calls dispose() which calls virSecretDefFree(). However, @obj
will have at least 2 references when entering restore_backup label. In
virSecretObjListAdd() when creating the new object via virSecretObjNew()
obj has refcnt = 1, and then we ref the object again. But wait a second:
if the object is already in both of the hash tables we return that
reference and don't increase the refcnt! So in the end,
virSecretObjListAdd() can return an object with refcnt == 1 and refcnt
== 2. This is obviously wrong and root cause of the problem you are
seeing. As I describe in the other e-mail, this breaks refcounting and
needs to be fixed.

Michal