[bug report] scsi: myrs: Add Mylex RAID controller (SCSI interface)

[bug report] scsi: myrs: Add Mylex RAID controller (SCSI interface)

[Dan Carpenter]

Hello Hannes Reinecke,

The patch 8a8606895947: "scsi: myrs: Add Mylex RAID controller (SCSI
interface)" from Jan 24, 2018, leads to the following static checker
warning:

	drivers/scsi/myrs.c:479 myrs_get_event()
	warn: right shifting more than type allows 16 vs 16

drivers/scsi/myrs.c
   461  static unsigned char
   462  myrs_get_event(myrs_hba *cs, unsigned short event_num,
                                     ^^^^^^^^^^^^^^^^^^^^^^^^

   463                 myrs_event *event_buf)
   464  {
   465          struct pci_dev *pdev = cs->pdev;
   466          dma_addr_t event_addr;
   467          myrs_cmdblk *cmd_blk = &cs->mcmd_blk;
   468          myrs_cmd_mbox *mbox = &cmd_blk->mbox;
   469          myrs_sgl *sgl;
   470          unsigned char status;
   471  
   472          event_addr = dma_map_single(&pdev->dev, event_buf,
   473                                      sizeof(myrs_event), DMA_FROM_DEVICE);
   474          if (dma_mapping_error(&pdev->dev, event_addr))
   475                  return DAC960_V2_AbnormalCompletion;
   476  
   477          mbox->GetEvent.opcode = DAC960_V2_IOCTL;
   478          mbox->GetEvent.dma_size = sizeof(myrs_event);
   479          mbox->GetEvent.evnum_upper = event_num >> 16;
                                             ^^^^^^^^^^^^^^^

This is always going to be zero.

   480          mbox->GetEvent.ctlr_num = 0;
   481          mbox->GetEvent.ioctl_opcode = DAC960_V2_GetEvent;
   482          mbox->GetEvent.evnum_lower = event_num & 0xFFFF;
   483          sgl = &mbox->GetEvent.dma_addr;
   484          sgl->sge[0].sge_addr = event_addr;
   485          sgl->sge[0].sge_count = mbox->GetEvent.dma_size;
   486          myrs_exec_cmd(cs, cmd_blk);
   487          status = cmd_blk->status;
   488          dma_unmap_single(&pdev->dev, event_addr,
   489                           sizeof(myrs_event), DMA_FROM_DEVICE);
   490  
   491          return status;
   492  }

This warning is probably a false positive which you can ignore but what
made me question it was looking at the caller:

drivers/scsi/myrs.c
  2222  static void myrs_monitor(struct work_struct *work)
  2223  {
  2224          myrs_hba *cs = container_of(work, myrs_hba, monitor_work.work);
  2225          struct Scsi_Host *shost = cs->host;
  2226          myrs_ctlr_info *info = cs->ctlr_info;
  2227          unsigned int epoch = cs->fwstat_buf->epoch;
  2228          unsigned long interval = MYRS_PRIMARY_MONITOR_INTERVAL;
  2229          unsigned char status;
  2230  
  2231          dev_dbg(&shost->shost_gendev, "monitor tick\n");
  2232  
  2233          status = myrs_get_fwstatus(cs);
  2234  
  2235          if (cs->needs_update) {
  2236                  cs->needs_update = false;
  2237                  mutex_lock(&cs->cinfo_mutex);
  2238                  status = myrs_get_ctlr_info(cs);
  2239                  mutex_unlock(&cs->cinfo_mutex);
  2240          }
  2241          if (cs->fwstat_buf->next_evseq - cs->next_evseq > 0) {
  2242                  status = myrs_get_event(cs, cs->next_evseq,
                                                    ^^^^^^^^^^^^^^
This is an int.

  2243                                          cs->event_buf);
  2244                  if (status == DAC960_V2_NormalCompletion) {
  2245                          myrs_log_event(cs, cs->event_buf);
  2246                          cs->next_evseq++;
                                ^^^^^^^^^^^^^^^^
And I guess this is where we set cs->next_evseq.

  2247                          interval = 1;
  2248                  }
  2249          }

regards,
dan carpenter

Re: [bug report] scsi: myrs: Add Mylex RAID controller (SCSI interface)

[Martin K. Petersen]

Hannes,

> The patch 8a8606895947: "scsi: myrs: Add Mylex RAID controller (SCSI
> interface)" from Jan 24, 2018, leads to the following static checker
> warning:

I dropped the two Mylex patches from 4.17/scsi-queue for now. Please fix
the issues reported and repost.

-- 
Martin K. Petersen	Oracle Linux Engineering

[bug report] scsi: myrs: Add Mylex RAID controller (SCSI interface)

[Dan Carpenter]

Hello Hannes Reinecke,

The patch 77266186397c: "scsi: myrs: Add Mylex RAID controller (SCSI
interface)" from Oct 17, 2018 (linux-next), leads to the following
Smatch static checker warning:

	drivers/scsi/myrs.c:1508 disable_enclosure_messages_store()
	warn: no lower bound on 'value' rl='s32min-2'

drivers/scsi/myrs.c
    1494 static ssize_t disable_enclosure_messages_store(struct device *dev,
    1495                 struct device_attribute *attr, const char *buf, size_t count)
    1496 {
    1497         struct scsi_device *sdev = to_scsi_device(dev);
    1498         struct myrs_hba *cs = shost_priv(sdev->host);
    1499         int value, ret;
                 ^^^^^^^^^

    1500 
    1501         ret = kstrtoint(buf, 0, &value);
    1502         if (ret)
    1503                 return ret;
    1504 
    1505         if (value > 2)

Smatch is complaining about negative values of value.  Why is 2 allowed
when ->disable_enc_msg is a bool?  We have a kstrtobool() function as
well, btw.

This is from an unpublished Smatch check but I'm working on it to get
it ready to release.

    1506                 return -EINVAL;
    1507 
--> 1508         cs->disable_enc_msg = value;
    1509         return count;
    1510 }

regards,
dan carpenter