[PATCH 2/2] storage: Fix returning of locked objects from 'virStoragePoolObjListSearch'

Peter Krempa posted 2 patches 2 years, 6 months ago
[PATCH 2/2] storage: Fix returning of locked objects from 'virStoragePoolObjListSearch'
Posted by Peter Krempa 2 years, 6 months ago
'virStoragePoolObjListSearch' explicitly documents that it's returning
a pointer to a locked and ref'd pool that maches the lookup function.

This was not the case as in commit 0c4b391e2a9 (released in
libvirt-8.3.0) the code was accidentally converted to use 'VIR_LOCK_GUARD'
which auto-unlocked it when leaving the scope, even when the code was
originally "leaking" the lock.

Revert the corresponding conversion and add a comment that this function
is intentionally leaking a locked object.

Fixes: 0c4b391e2a9
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2221851
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/virstorageobj.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 7010e97d61..59fa5da372 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -454,11 +454,16 @@ virStoragePoolObjListSearchCb(const void *payload,
     virStoragePoolObj *obj = (virStoragePoolObj *) payload;
     struct _virStoragePoolObjListSearchData *data =
         (struct _virStoragePoolObjListSearchData *)opaque;
-    VIR_LOCK_GUARD lock = virObjectLockGuard(obj);

+    virObjectLock(obj);
+
+    /* If we find the matching pool object we must return while the object is
+     * locked as the caller wants to return a locked object. */
     if (data->searcher(obj, data->opaque))
         return 1;

+    virObjectUnlock(obj);
+
     return 0;
 }

-- 
2.41.0
Re: [PATCH 2/2] storage: Fix returning of locked objects from 'virStoragePoolObjListSearch'
Posted by Ján Tomko 2 years, 6 months ago
On a Tuesday in 2023, Peter Krempa wrote:
>'virStoragePoolObjListSearch' explicitly documents that it's returning
>a pointer to a locked and ref'd pool that maches the lookup function.
>
>This was not the case as in commit 0c4b391e2a9 (released in
>libvirt-8.3.0) the code was accidentally converted to use 'VIR_LOCK_GUARD'
>which auto-unlocked it when leaving the scope, even when the code was
>originally "leaking" the lock.
>
>Revert the corresponding conversion and add a comment that this function
>is intentionally leaking a locked object.
>
>Fixes: 0c4b391e2a9
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2221851
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/virstorageobj.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano