Re: [Qemu-devel] [PATCH v1] lsi53c895a: check message length value is valid

[Paolo Bonzini <pbonzini@redhat.com>]

On 26/10/2018 22:55, Peter Maydell wrote:
>> +    assert(len <= LSI_MAX_MSGIN_LEN);
>>      pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len);
>>      /* Linux drivers rely on the last byte being in the SIDL.  */
>>      s->sidl = s->msg[len - 1];
> Is it possible to get here with len == 0 ?

No, all calls to

    lsi_set_phase(s, PHASE_MI);

are followed or preceded by lsi_add_msg_byte.  But an assertion is good
to add.  What do you think of squashing this on top:

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 3a40e62853..72d85c42dd 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -865,9 +865,9 @@ static void lsi_do_msgin(LSIState *s)
     trace_lsi_do_msgin(s->dbc, s->msg_len);
     s->sfbr = s->msg[0];
     len = s->msg_len;
+    assert(len >= 0 && len <= LSI_MAX_MSGIN_LEN);
     if (len > s->dbc)
         len = s->dbc;
-    assert(len <= LSI_MAX_MSGIN_LEN);
     pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len);
     /* Linux drivers rely on the last byte being in the SIDL.  */
     s->sidl = s->msg[len - 1];
@@ -1706,8 +1706,10 @@
         break;
     case 0x58: /* SBDL */
         /* Some drivers peek at the data bus during the MSG IN phase.  */
-        if ((s->sstat1 & PHASE_MASK) == PHASE_MI)
+        if ((s->sstat1 & PHASE_MASK) == PHASE_MI) {
+            assert(s->msg_len >= 0);
             return s->msg[0];
+        }
         ret = 0;
         break;
     case 0x59: /* SBDL high */


Paolo