[libvirt] [PATCH 01/11] nodedev: Fix crash in libvirtd on vHBA creation path

[John Ferlan]

On 01/02/2017 09:16 AM, Ján Tomko wrote:
> On Fri, Nov 18, 2016 at 09:26:27AM -0500, John Ferlan wrote:
>> Providing XML such as:
>>
>> <device>
>>  <name>vhba</name>
>>  <capability type='scsi_host'>
>>    <capability type='fc_host'>
>>    </capability>
>>  </capability>
>> </device>
>>
>> would crash libvirt because the '<parent>' isn't a required field, but
>> for vHBA creation it's expected (day 1 issue - see commit id '81d0ffbc').
>> The nodedev.rng added in commit id '2c22a68c' has this as an optional
>> field.
>> NB: On normal udev discovery if a parent field wasn't found, it would be
>> set to "computer" by udevSetParent, so this is a somewhat unique path.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/conf/node_device_conf.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index 1cd0baf..bf5b22f 100644
>> --- a/src/conf/node_device_conf.c
>> +++ b/src/conf/node_device_conf.c
>> @@ -118,7 +118,7 @@ virNodeDeviceObjPtr
>> virNodeDeviceFindByName(virNodeDeviceObjListPtr devs,
>>
>>     for (i = 0; i < devs->count; i++) {
>>         virNodeDeviceObjLock(devs->objs[i]);
>> -        if (STREQ(devs->objs[i]->def->name, name))
>> +        if (STREQ_NULLABLE(devs->objs[i]->def->name, name))
> 
> 
> Is this needed after patch 3/11?
> 

Doesn't seem so - perhaps a bit of paranoia kicked in... Cannot remember
for sure since it's been so long...

> I presume we do not store objects with NULL names in devs. In that case
> the callers should avoid calling this with NULL name - what is the point
> of iterating over the array in that case?

Good assumption as objs[i]->def->name is the key for node devices (they
don't have uuids).

Good question - I have a faint recollection of tripping across this no
parent path based on some historical comment somewhere that said one
doesn't have to provide the parent in the nodedev-create XML... So I
went back, tried again with just this patch applied the create fails with:

error: Failed to create node device from vhba_nodedev.xml
error: internal error: Could not find parent device for 'new device'

Once patch 3 is applied the creation occurs "as advertised" (that is if
you don't supply a parent the code will find "a" parent that this vport
capable.

Whether this ever actually worked I don't recall and didn't chase.

In any case, I'll drop this patch (for now) and can post a followup that
would handle a NULL 'name' better (just return NULL immediately rather
than going through the whole loop needlessly).

John
> 
> Jan
> 
>>             return devs->objs[i];
>>         virNodeDeviceObjUnlock(devs->objs[i]);
>>     }