Re: [Qemu-devel] [PATCH qemu v7] memory/iommu: QOM'fy IOMMU MemoryRegion

[Paolo Bonzini <pbonzini@redhat.com>]

On 08/06/2017 16:28, Alexey Kardashevskiy wrote:
>>> +void memory_region_init_iommu_type(const char *mrtypename,
>>> +                                   IOMMUMemoryRegion *iommu_mr,
>>> +                                   Object *owner,
>>> +                                   const MemoryRegionIOMMUOps *ops,
>>> +                                   const char *name,
>>> +                                   uint64_t size)
>>>  {
>>> -    memory_region_init(mr, owner, name, size);
>>> -    mr->iommu_ops = ops,
>>> +    struct MemoryRegion *mr;
>>> +    size_t instance_size = object_type_get_instance_size(mrtypename);
>>> +
>>> +    object_initialize(iommu_mr, instance_size, mrtypename);
>> This looks exceedingly dangerous.  AIUI, the entire purpose of the
>> size parameter to object_initialize() (which can certainly get the
>> instance size from the type, just as you do) is to verify that the
>> buffer you're initializing actually has space for the object type
>> you're putting there.
>>
>> By looking up the instance size yourself and passing it to
>> object_initialize(), you're disabling that check.
>>
>> If someone allocates an array of plain IOMMUMemoryRegion structures,
>> then uses this to initialize a derived IOMMU MR type with more fields,
>> the user will get no warning that something is wrong before the memory
>> corruption starts.
> Hm. How can I fix it then for a generic case? Pass the actual amount of
> bytes occupied by *iommu_mr?

Yes, like object_initialize.

Paolo