Re: [libvirt] [PATCH 7/8] secret: Properly handle @def after virSecretObjAdd in driver
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