[libvirt] [PATCH v2] udev: fix crash in libudev logging

[John Ferlan]

On 06/14/2013 06:54 AM, Ján Tomko wrote:
> On 06/14/2013 11:18 AM, Michal Privoznik wrote:
>> On 14.06.2013 10:53, Guannan Ren wrote:
>>> On 06/13/2013 09:56 PM, Ján Tomko wrote:
>>>> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
>>>> +    const char *format = NULL;
>>>> +
>>>> +    virBufferAdd(&buf, fmt, -1);
>>>> +    virBufferTrim(&buf, "\n", -1);


Coverity analysis uncovered a complaint in last nights run after
this was pushed. The virBufferTrim() call has "other" places where
the return status is checked, but this call doesn't. Using cscope
I also see that virNetSSHCheckHostKey() in virnetsshsession.c
does not check return status.


FILE: src/node_device/node_device_udev.c

Coverity error seen in the output:
	ERROR: CHECKED_RETURN
	FUNCTION: udevLogFunction

Coverity Events:
372
    virBufferAdd(&buf, fmt, -1);

                             Calling function "virBufferTrim(virBufferPtr,
 (1) Event check_return:     char const *, int)" without checking return
                             value (as is done elsewhere 5 out of 6 times).
                             No check of the return value of
  (7) Event unchecked_value: "virBufferTrim(&buf, "\n", -
                             1)".
                             [example_checked][example_checked]
Also see events:             [example_checked][example_checked]
                             [example_checked]
------------------------------------------------------------



>>>> +
>>>
>>>           We need a virBufferError checking here before formatting into
>>> a string.
> 
> virBufferContentAndReset already checks for the error and returns NULL if
> there was one. The only possible error here is ENOMEM.
> 
>>
>> In which case we want to got with fmt as provided.
>>
>> Michal
>>
> 
> If we aren't going to report the buffer error, I think checking format for
> NULL should be enough.
> 
> I'll squash this in before pushing:
> 
> diff --git a/src/node_device/node_device_udev.c
> b/src/node_device/node_device_udev.c
> index b462549..bb58415 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -374,10 +374,9 @@ udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED,
> 
>      format = virBufferContentAndReset(&buf);
> 
> -    if (format)
> -        virLogVMessage(VIR_LOG_FROM_LIBRARY,
> -                       virLogPriorityFromSyslog(priority),
> -                       file, line, fn, NULL, format, args);
> +    virLogVMessage(VIR_LOG_FROM_LIBRARY,
> +                   virLogPriorityFromSyslog(priority),
> +                   file, line, fn, NULL, format ? format : fmt, args);
> 
>      VIR_FREE(format);
>  }
> 
> 
> Jan
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>