| Message ID | 20230727153925.15297-1-adiupina@astralinux.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | 53c700: add 'slot' check to NULL | expand |
On Thu, 2023-07-27 at 18:39 +0300, Alexandra Diupina wrote: > The 'slot' variable allows a NULL value. > It is necessary to add a check for a null > value to avoid dereferencing the null pointer. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Co-developed-by: Vladimir Telezhnikov <vtelezhnikov@astralinux.ru> > Signed-off-by: Vladimir Telezhnikov <vtelezhnikov@astralinux.ru> > Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru> > --- > drivers/scsi/53c700.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c > index e1e4f9d10887..8e5468d1733d 100644 > --- a/drivers/scsi/53c700.c > +++ b/drivers/scsi/53c700.c > @@ -1598,6 +1598,8 @@ NCR_700_intr(int irq, void *dev_id) > printk("scsi%d (%d:%d) PHASE MISMATCH > IN SEND MESSAGE %d remain, return %p[%04x], phase %s\n", host- > >host_no, pun, lun, count, (void *)temp, temp - hostdata->pScript, > sbcl_to_string(NCR_700_readb(host, SBCL_REG))); > #endif > resume_offset = hostdata->pScript + > Ent_SendMessagePhaseMismatch; > + } else if (!slot) { > + printk(KERN_ERR "53c700: SCSI DONE > HAS NULL SCp\n"); > } else if(dsp >= to32bit(&slot->pSG[0].ins) > && I don't believe anyone has ever hit this, but if slot were null, it would have to drop through to the else clause to get a bus reset to kick the device. If we do what you propose above, the driver would hang instead of crashing, which isn't a better outcome. Something like this. James --- diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c index e1e4f9d10887..5296a13404cf 100644 --- a/drivers/scsi/53c700.c +++ b/drivers/scsi/53c700.c @@ -1598,7 +1598,7 @@ NCR_700_intr(int irq, void *dev_id) printk("scsi%d (%d:%d) PHASE MISMATCH IN SEND MESSAGE %d remain, return %p[%04x], phase %s\n", host->host_no, pun, lun, count, (void *)temp, temp - hostdata->pScript, sbcl_to_string(NCR_700_readb(host, SBCL_REG))); #endif resume_offset = hostdata->pScript + Ent_SendMessagePhaseMismatch; - } else if(dsp >= to32bit(&slot->pSG[0].ins) && + } else if(slot && dsp >= to32bit(&slot->pSG[0].ins) && dsp <= to32bit(&slot->pSG[NCR_700_SG_SEGMENTS].ins)) { int data_transfer = NCR_700_readl(host, DBC_REG) & 0xffffff; int SGcount = (dsp - to32bit(&slot->pSG[0].ins))/sizeof(struct NCR_700_SG_List);
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c index e1e4f9d10887..8e5468d1733d 100644 --- a/drivers/scsi/53c700.c +++ b/drivers/scsi/53c700.c @@ -1598,6 +1598,8 @@ NCR_700_intr(int irq, void *dev_id) printk("scsi%d (%d:%d) PHASE MISMATCH IN SEND MESSAGE %d remain, return %p[%04x], phase %s\n", host->host_no, pun, lun, count, (void *)temp, temp - hostdata->pScript, sbcl_to_string(NCR_700_readb(host, SBCL_REG))); #endif resume_offset = hostdata->pScript + Ent_SendMessagePhaseMismatch; + } else if (!slot) { + printk(KERN_ERR "53c700: SCSI DONE HAS NULL SCp\n"); } else if(dsp >= to32bit(&slot->pSG[0].ins) && dsp <= to32bit(&slot->pSG[NCR_700_SG_SEGMENTS].ins)) { int data_transfer = NCR_700_readl(host, DBC_REG) & 0xffffff;
The 'slot' variable allows a NULL value. It is necessary to add a check for a null value to avoid dereferencing the null pointer. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Co-developed-by: Vladimir Telezhnikov <vtelezhnikov@astralinux.ru> Signed-off-by: Vladimir Telezhnikov <vtelezhnikov@astralinux.ru> Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru> --- drivers/scsi/53c700.c | 2 ++ 1 file changed, 2 insertions(+)