Re: [Qemu-devel] [PATCH] scsi: pvscsi: request descriptor data_length to 32 bit

[Paolo Bonzini]

On 03/09/2016 08:33, P J P wrote:
> From: Prasad J Pandit <address@hidden>
> 
> In PVSCSI paravirtual SCSI bus, the request descriptor data
> length is defined to be 64 bit. While building SG list from
> a request descriptor, it gets truncated to 32bit in routine
> 'pvscsi_convert_sglist'. This could lead to an infinite loop
> situation for arbitrarily large 'dataLen' values. Define local
> variable 'data_length' to be 32 bit, to avoid it.
> 
> Reported-by: Li Qiang <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  hw/scsi/vmw_pvscsi.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 4245c15..4d38330 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -629,7 +629,7 @@ static void
>  pvscsi_convert_sglist(PVSCSIRequest *r)
>  {
>      int chunk_size;
> -    uint64_t data_length = r->req.dataLen;
> +    uint32_t data_length = r->req.dataLen;

Why is this needed if you remove the cast in MIN, below?

Paolo

>      PVSCSISGState sg = r->sg;
>      while (data_length) {
>          while (!sg.resid) {
> @@ -637,8 +637,7 @@ pvscsi_convert_sglist(PVSCSIRequest *r)
>              trace_pvscsi_convert_sglist(r->req.context, r->sg.dataAddr,
>                                          r->sg.resid);
>          }
> -        assert(data_length > 0);
> -        chunk_size = MIN((unsigned) data_length, sg.resid);
> +        chunk_size = MIN(data_length, sg.resid);
>          if (chunk_size) {
>              qemu_sglist_add(&r->sgl, sg.dataAddr, chunk_size);
>          }
>