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

John Ferlan jferlan at redhat.com
Thu May 14 12:35:56 UTC 2015



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...

John




More information about the libvir-list mailing list