[libvirt] [PATCH 1/3] storage: Resolve Coverity FORWARD_NULL

[Laine Stump]

On 05/14/2015 08:35 AM, John Ferlan wrote:
>
> On 05/13/2015 02:43 PM, Jiri Denemark wrote:
>> On Wed, May 13, 2015 at 12:32:20 -0400, John Ferlan wrote:
>>> Coverity points out it's possible for one of the virCommand{Output|Error}*
>>> API's to have not allocated 'output' and/or 'error' in which case the
>>> strstr comparison will cause a NULL deref
>>>
>>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>>> ---
>>>  src/storage/storage_backend_disk.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
>>> index 6394dac..5c2c49a 100644
>>> --- a/src/storage/storage_backend_disk.c
>>> +++ b/src/storage/storage_backend_disk.c
>>> @@ -412,7 +412,7 @@ virStorageBackendDiskFindLabel(const char* device)
>>>  
>>>      /* if parted succeeds we have a valid partition table */
>>>      ret = virCommandRun(cmd, NULL);
>>> -    if (ret < 0) {
>>> +    if (ret < 0 && output && error) {
>>>          if (strstr(output, "unrecognised disk label") ||
>>>              strstr(error, "unrecognised disk label")) {
>>>              ret = 1;
>> This doesn't seem to be correct if either output or error is NULL and
>> the other one is non-NULL. I'm too lazy to check if it's possible or
>> not, but I think we should change this code in the following way and be
>> safe:
>>
>>     if (ret < 0) {
>>         if ((output && strstr(output, "unrecognised disk label")) ||
>>             (error && strstr(error, "unrecognised disk label"))) {
>>             ret = 1;
>>
>> Jirka
>>
> Sure - seems reasonable, although I suspect if allocation of memory for
> the output buffer fails, then allocation of memory for the error buffer
> will fail too, but just in case it succeeds...

In case there's any question - ACK to Jirka's version of the patch.