Re: [Qemu-devel] [PATCH v3] scsi: pvscsi: avoid infinite loop while building SG list

[P J P]

+-- On Tue, 6 Sep 2016, P J P wrote --+
| From: Prasad J Pandit <address@hidden>
| 
| In PVSCSI paravirtual SCSI bus, pvscsi_convert_sglist can take a very
| long time or go into an infinite loop due to two different bugs:
| 
| 1) 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 large 'dataLen' values, when data_length is cast to uint32_t
| and chunk_size becomes always zero.  Fix this by removing the incorrect
| cast.
| 
| 2) pvscsi_get_next_sg_elem can be called arbitrarily many times if the
| element has a zero length.  Get out of the loop early when this happens,
| by introducing an upper limit on the number of SG list elements.
| 
| Reported-by: Li Qiang <address@hidden>
| Signed-off-by: Prasad J Pandit <address@hidden>
| ---
|  hw/scsi/vmw_pvscsi.c | 11 ++++++-----
|  1 file changed, 6 insertions(+), 5 deletions(-)
| 
| Update as per:
|   -> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01172.html
| 
| diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
| index 4245c15..babac5a 100644
| --- a/hw/scsi/vmw_pvscsi.c
| +++ b/hw/scsi/vmw_pvscsi.c
| @@ -40,6 +40,8 @@
|  #define PVSCSI_MAX_DEVS                   (64)
|  #define PVSCSI_MSIX_NUM_VECTORS           (1)
|  
| +#define PVSCSI_MAX_SG_ELEM                2048
| +
|  #define PVSCSI_MAX_CMD_DATA_WORDS \
|      (sizeof(PVSCSICmdDescSetupRings)/sizeof(uint32_t))
|  
| @@ -628,17 +630,16 @@ pvscsi_queue_pending_descriptor(PVSCSIState *s, 
SCSIDevice **d,
|  static void
|  pvscsi_convert_sglist(PVSCSIRequest *r)
|  {
| -    int chunk_size;
| +    uint32_t chunk_size, elmcnt = 0;
|      uint64_t data_length = r->req.dataLen;
|      PVSCSISGState sg = r->sg;
| -    while (data_length) {
| -        while (!sg.resid) {
| +    while (data_length && elmcnt < PVSCSI_MAX_SG_ELEM) {
| +        while (!sg.resid && elmcnt++ < PVSCSI_MAX_SG_ELEM) {
|              pvscsi_get_next_sg_elem(&sg);
|              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);
|          }


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