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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxx>
>> ---
>>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]