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

Eric Blake eblake at redhat.com
Wed Sep 21 20:12:35 UTC 2011


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.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list