> On Nov 26, 2020, at 9:48 AM, Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
>
> On Tue, Nov 24, 2020 at 02:48:35PM -0500, Matt Coleman wrote:
>> + g_autofree char *addressString = g_strdup_printf("%u", disk->info.addr.drive.unit);
>
> Validate disk->info.type == DRIVE before accessing this field otherwise
> the union contents is undefined.
>> + if (hypervSetEmbeddedProperty(volumeResource, "ResourceType", resourceType) < 0)
>> + return -1;
>> +
>> + if (hypervSetEmbeddedProperty(volumeResource, "ResourceSubType",
>> + "Microsoft:Hyper-V:Virtual Hard Disk") < 0)
>> + return -1;
>
> The disk->device can be disk, or cdrom or floppy. So if you're hadcoding disk, then
> validate disk->device matches.
The logic in hypervDomainAttachStorageVolume() ensures that it only
calls hypervDomainAttachVirtualDisk() if disk->device is equal to
VIR_DOMAIN_DISK_DEVICE_DISK.
Should I add another check inside hypervDomainAttachVirtualDisk() or is
it acceptable to validate within hypervDomainAttachStorageVolume() as
long as no other functions call hypervDomainAttachVirtualDisk()?
If this is acceptable, I could also validate disk->info.type inside
hypervDomainAttachStorageVolume(), which would cover the other two
critiques.
>> + VIR_DEBUG("Now attaching disk image '%s' with address %d to bus %d of type %d",
>> + disk->src->path, disk->info.addr.drive.unit, disk->info.addr.drive.controller, disk->bus);
>
> Don't access the disk->info.addr until its type is validated
