[libvirt] [PATCH] locking: avoid dereferencing a NULL pointer

[Alex Jia]

On 09/22/2011 04:12 AM, Eric Blake wrote:
> On 09/21/2011 12:10 PM, ajia at redhat.com wrote:
>> From: Alex Jia<ajia at redhat.com>
>>
>> * src/locking/lock_driver_sanlock.c: in fact, virStrcpy calls
>>    virStrncpy(dest, src, strlen(src), destbytes) then return result,
>>    if 'path' is NULL, it means 'src' is also NULL, strlen(NULL) will
>>    dereference a NULL pointer, which probably causes a segmentation 
>> fault.
>>
>> Signed-off-by: Alex Jia<ajia at redhat.com>
>> ---
>>   src/locking/lock_driver_sanlock.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/locking/lock_driver_sanlock.c 
>> b/src/locking/lock_driver_sanlock.c
>> index 2d72510..8c6d873 100644
>> --- a/src/locking/lock_driver_sanlock.c
>> +++ b/src/locking/lock_driver_sanlock.c
>> @@ -158,7 +158,7 @@ static int virLockManagerSanlockSetupLockspace(void)
>>       memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, 
>> SANLK_NAME_LEN);
>>       ls.host_id = 0; /* Doesn't matter for initialization */
>>       ls.flags = 0;
>> -    if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) {
>> +    if (!path || !virStrcpy(ls.host_id_disk.path, path, 
>> SANLK_PATH_LEN)) {
>>           virLockError(VIR_ERR_INTERNAL_ERROR,
>>                        _("Lockspace path '%s' exceeded %d characters"),
>>                        path, SANLK_PATH_LEN);
>
> NACK.  The prior virAsprintf guarantees that path is non-NULL at this 
> point.  Rather, the real problem that Coverity is complaining about 
> here is that the only way to get to the error_unlink: label is if path 
> is already non-NULL, so that the 'if (path)' in that label is redundant.
>
Yeah, you're right, I will commit a new patch, because this subject is 
inappropriate, so I haven't naming v2 for this patch, the new patch 
subject is "[libvirt] [PATCH] locking: remove redundant codes".

Thanks,
Alex