Re: [Qemu-devel] [RFC v8 10/18] virtio-iommu: Implement probe request

[Jean-Philippe Brucker]

Hi Eric,

A few issues creeped in when the resv_mem structure changed

On 09/11/2018 11:29, Eric Auger wrote:
> +#define SUPPORTED_PROBE_PROPERTIES (\
> +    VIRTIO_IOMMU_PROBE_T_NONE | \
> +    VIRTIO_IOMMU_PROBE_T_RESV_MEM)

You might be missing "1 <<" here, since the properties types are normal
values

[...]
> +/**
> + * virtio_iommu_fill_resv_mem_prop - Add a RESV_MEM probe
> + * property into the probe request buffer
> + *
> + * @key: interval handle
> + * @value: handle to the reserved memory region
> + * @data: handle to the probe request buffer state
> + */
> +static gboolean virtio_iommu_fill_resv_mem_prop(gpointer key,
> +                                                gpointer value,
> +                                                gpointer data)
> +{
> +    struct virtio_iommu_probe_resv_mem *resv =
> +        (struct virtio_iommu_probe_resv_mem *)value;
> +    struct virtio_iommu_probe_property *prop;
> +    struct virtio_iommu_probe_resv_mem *current;
> +    viommu_property_buffer *bufstate = (viommu_property_buffer *)data;
> +    size_t size = sizeof(*resv), total_size;
> +    uint8_t *prop_value;
> +
> +    total_size = size + sizeof(*prop);

size already contains sizeof(*prop)

> +
> +    if (bufstate->filled + total_size >= VIOMMU_PROBE_SIZE) {
> +        bufstate->error = true;
> +        /* get the traversal stopped by returning true */
> +        return true;
> +    }
> +    prop = (struct virtio_iommu_probe_property *)
> +                (bufstate->start + bufstate->filled);
> +    prop->type = cpu_to_le16(VIRTIO_IOMMU_PROBE_T_RESV_MEM) &
> +                    VIRTIO_IOMMU_PROBE_T_MASK;
> +    prop->length = cpu_to_le16(size);

Should be size-4

> +
> +    prop_value = (uint8_t *)prop + 4;
> +    current = (struct virtio_iommu_probe_resv_mem *)prop_value;
> +    *current = *resv;

*resv includes the property header, but *current doesn't, so the resv
property is corrupted.

Thanks,
Jean