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

[Jean-Philippe Brucker]

On Thu, Jun 23, 2022 at 10:31:52AM +0800, 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.
> 
> Changed 4th parameter of virtio_iommu_iov_to_req() to receive size
> of device-readable part.
> 
> Fixes: 1733eebb9e75b ("virtio-iommu: Implement RESV_MEM probe request")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  v3: moved "- sizeof(struct virtio_iommu_req_tail)" to 
> virtio_iommu_handle_req() per Jean
>  v2: keep bugfix change and drop cleanup change
> 
>  hw/virtio/virtio-iommu.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 7c122ab95780..08b227e828f8 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -675,11 +675,10 @@ static int virtio_iommu_probe(VirtIOIOMMU *s,
>  
>  static int virtio_iommu_iov_to_req(struct iovec *iov,
>                                     unsigned int iov_cnt,
> -                                   void *req, size_t req_sz)
> +                                   void *req, size_t payload_sz)
>  {
> -    size_t sz, payload_sz = req_sz - sizeof(struct virtio_iommu_req_tail);
> +    size_t sz = iov_to_buf(iov, iov_cnt, 0, req, payload_sz);
>  
> -    sz = iov_to_buf(iov, iov_cnt, 0, req, payload_sz);
>      if (unlikely(sz != payload_sz)) {
>          return VIRTIO_IOMMU_S_INVAL;
>      }
> @@ -692,7 +691,8 @@ static int virtio_iommu_handle_ ## __req(VirtIOIOMMU *s,  
>               \
>                                           unsigned int iov_cnt)          \
>  {                                                                       \
>      struct virtio_iommu_req_ ## __req 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_ ## __req(s, &req);                 \
>  }
> -- 
> 2.25.1
>