Hello Paolo, all
+-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+
| > - 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?
The outer while loop below is controlled by 'data_length'. The cast in MIN
truncates a large(64bit) value of 'data_length' to zero(0), thus setting
'chunk_size' to zero, which results in infinite loop as 'data_length' remains
unchanged(> 0).
Second, Removing cast below results in 'chunk_size' being set to 'sg.resid',
for large(>32bit) values of 'data_length'. Which results in an infinite loop
because the inner 'while(!sg.resid)' loop takes forever to read non-zero
values into 'sg.resid'.
Above type change ensures that outer while loop is not entered if
'data_length' is zero. And removing cast ensures that inner while(!sg.resid)
loop does not have run forever, ie. till large(64 bit) 'data_length' becomes
zero.
Looking at the 'vmw_pvscsi.c' Linux kernel driver, 'dataLen' seems to be set
to an unsigned 32 bit 'bufflen' value.
| > 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);
| > }
| >
|
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
