Re: [PATCH] test driver: don't unlock pool after freeing it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 16, 2015 at 17:14:28 -0400, David Mansfield wrote:
> The attached patch (taken from my modified Fedora 22 source rpm, 
> 1.2.13.1-2.fc22, sorry),  fixes a case where, in the test driver, memory 
> is accessed after it's freed.
> 
> Patch applies to latest git with:
> 
> Hunk #1 succeeded at 4395 (offset -469 lines).
> 
> The illegal access was found using valgrind.
> 
> The function testStoragePoolUndefine() calls virStoragePoolObjRemove() 
> (which seems to free the memory for that pool structure) and then in the 
> cleanup stanza calls virStoragePoolObjUnlock() which tampers with the 
> freed structure.
> 
> -- 
> Thanks,
> David Mansfield

> From: David Mansfield <dmansfield@xxxxxxxxx>
> Date: Tue, 15 Sep 2015 10:05:56 -0400
> Subject: [PATCH] test driver: don't unlock pool after freeing it 
> 
> The test driver was unlocking the pool object after it had been
> freed causing memory corruption.
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index a386270..c2256dc 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4864,8 +4864,10 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
>      ret = 0;
>  
>   cleanup:
> -    if (privpool)
> -        virStoragePoolObjUnlock(privpool);
> +    // privpool cannot be unlocked because memory for it has been
> +    // freed by the virStoragePoolObjRemove call above
> +    // if (privpool)
> +    //     virStoragePoolObjUnlock(privpool);

This cannot be just commented out since the code above is allowing only
inactive pools to be deleted and jumps to the cleanup label otherwise.
With this approach the lock would stay locked and the test driver would
deadlock the next time you are going to reference the same pool.

The right fix here is to clear the 'privpool' pointer after we know that
the pool was removed.

As a note, line comments are not allowed in libvirt.

Should I post the correct patch in your name or do you want to submit it
yourself?

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]