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

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

  Hello Dmitry,

+-- On Tue, 13 Sep 2016, Dmitry Fleytman wrote --+
| Better put here:
| 
| if(elemcnt++ >= PVSCSI_MAX_SG_ELEM) {
|     return;
| }
|
| And ditch additional conditions from while clauses.
| This way you’ll have less compare operations and avoid
| adding the same element twice when leaving because of too long SG list.

  Okay.

| Also this is not a good idea to send truncated requests
| when SG list becomes too long. I’d prefer to return error
| code from pvscsi_convert_sglist() and handle it as appropriate.

  This could be a separate patch. Should pvscsi_build_sglist() destroy the 
sglist on error? 

@@ -656,7 +660,9 @@ pvscsi_build_sglist(PVSCSIState *s, PVSCSIRequest *r)
 
     pci_dma_sglist_init(&r->sgl, d, 1);
     if (r->req.flags & PVSCSI_FLAG_CMD_WITH_SG_LIST) {
-        pvscsi_convert_sglist(r);
+        if (pvscsi_convert_sglist(r) < 0) {
+            qemu_sglist_destroy(&r->sgl);
+        }
     } else {
         qemu_sglist_add(&r->sgl, r->req.dataAddr, r->req.dataLen);
     }

I'll send both patches together once you confirm.

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