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

[P J P <ppandit@redhat.com>]

+-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+
| No, that's not what happens.  chunk_size is set to sg.resid, after which:
| 
|         sg.dataAddr += chunk_size;
|         data_length -= chunk_size;
|         sg.resid -= chunk_size;
| 
| The loop is reentered with sg.resid == 0, it calls into
| pvscsi_get_next_sg_elem and this sets sg.resid to a nonzero value.  It's
| not an infinite loop.

  Yes, true; But 'pvscsi_get_next_sg_elem' does not return non-zero 'sg.resid' 
each time. In fact, it returns more zeros and thus the loop iterates 
infinitely. When I ran it with 64 bit 'data_length' and without cast, after 
some time, the inner loop gets stuck and does not seem to read non-zero values 
into 'sg.resid'.

Is there limit to number of SG elements?

| The driver is irrelevant.  If the data_length is an uint64_t you need to
| ensure that a 64 bit buffer is processed correctly.  Here you are
| truncating it, which is wrong and will cause a buffer underrun.

  Yes. I thought truncation in MIN was intentional, considering the driver 
sets 'dataLen' to 32 bit value.

If we are to go with 64 bit 'data_length', how long should the inner while 
loop run?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F