Re: [PATCH v2] virtio-iommu: Fix the partial copy of probe request

[Eric Auger <eric.auger@redhat.com>]

Hi,

On 6/17/22 08:20, Zhenzhong Duan wrote:
> The structure of probe request doesn't include the tail, this leads
> to a few field missed to be copied. Currently this isn't an issue as
> those missed field belong to reserved field, just in case reserved
> field will be used in the future.
>
> Fixes: 1733eebb9e75b ("virtio-iommu: Implement RESV_MEM probe request")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
nice catch.

Reviewed-by: Eric Auger <eric.auger@redhat.com>

the spec is pretty confusing here though (virtio-v1.2-csd01.pdf) as it
presents the struct as follows:

struct virtio_iommu_req_probe {
struct virtio_iommu_req_head head; /* Device-readable */
le32 endpoint;
u8 reserved[64]; /* Device-writable */
u8 properties[probe_size];
struct virtio_iommu_req_tail tail;
};

Adding Jean in the loop ...

Thanks!

Eric




> ---
>  v2: keep bugfix change and drop cleanup change
>
>  hw/virtio/virtio-iommu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 7c122ab95780..195f909620ab 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -708,7 +708,8 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
>                                       uint8_t *buf)
>  {
>      struct virtio_iommu_req_probe req;
> -    int ret = virtio_iommu_iov_to_req(iov, iov_cnt, &req, sizeof(req));
> +    int ret = virtio_iommu_iov_to_req(iov, iov_cnt, &req,
> +                    sizeof(req) + sizeof(struct virtio_iommu_req_tail));
>  
>      return ret ? ret : virtio_iommu_probe(s, &req, buf);
>  }