[bug report] scsi: aic79xx: Do not reference SCSI command when resetting device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Hannes Reinecke,

The patch c67e63800446: "scsi: aic79xx: Do not reference SCSI command
when resetting device" from Oct 2, 2023 (linux-next), leads to the
following Smatch static checker warning:

	drivers/scsi/aic7xxx/aic79xx_osm.c:1793 ahd_done()
	error: we previously assumed 'cmd' could be null (see line 1774)

drivers/scsi/aic7xxx/aic79xx_osm.c
    1758 void
    1759 ahd_done(struct ahd_softc *ahd, struct scb *scb)
    1760 {
    1761         struct scsi_cmnd *cmd;
    1762         struct          ahd_linux_device *dev;
    1763 
    1764         if ((scb->flags & SCB_ACTIVE) == 0) {
    1765                 printk("SCB %d done'd twice\n", SCB_GET_TAG(scb));
    1766                 ahd_dump_card_state(ahd);
    1767                 panic("Stopping for safety");
    1768         }
    1769         LIST_REMOVE(scb, pending_links);
    1770         cmd = scb->io_ctx;
    1771         dev = scb->platform_data->dev;
    1772         dev->active--;
    1773         dev->openings++;
    1774         if (cmd) {
                     ^^^
The patch adds this NULL check

    1775                 if ((cmd->result & (CAM_DEV_QFRZN << 16)) != 0) {
    1776                         cmd->result &= ~(CAM_DEV_QFRZN << 16);
    1777                         dev->qfrozen--;
    1778                 }
    1779         } else if (scb->flags & SCB_DEVICE_RESET) {
    1780                 if (ahd->platform_data->eh_done)
    1781                         complete(ahd->platform_data->eh_done);
    1782                 ahd_free_scb(ahd, scb);
    1783                 return;
    1784         }
    1785         ahd_linux_unmap_scb(ahd, scb);
    1786 
    1787         /*
    1788          * Guard against stale sense data.
    1789          * The Linux mid-layer assumes that sense
    1790          * was retrieved anytime the first byte of
    1791          * the sense buffer looks "sane".
    1792          */
--> 1793         cmd->sense_buffer[0] = 0;
                 ^^^^^^^^^^^^^^^^^
But if cmd is NULL we're going to crash anyway

    1794         if (ahd_get_transaction_status(scb) == CAM_REQ_INPROG) {
    1795 #ifdef AHD_REPORT_UNDERFLOWS
    1796                 uint32_t amount_xferred;
    1797 
    1798                 amount_xferred =
    1799                     ahd_get_transfer_length(scb) - ahd_get_residual(scb);
    1800 #endif
    1801                 if ((scb->flags & SCB_TRANSMISSION_ERROR) != 0) {
    1802 #ifdef AHD_DEBUG
    1803                         if ((ahd_debug & AHD_SHOW_MISC) != 0) {
    1804                                 ahd_print_path(ahd, scb);
    1805                                 printk("Set CAM_UNCOR_PARITY\n");
    1806                         }
    1807 #endif
    1808                         ahd_set_transaction_status(scb, CAM_UNCOR_PARITY);
    1809 #ifdef AHD_REPORT_UNDERFLOWS
    1810                 /*
    1811                  * This code is disabled by default as some
    1812                  * clients of the SCSI system do not properly
    1813                  * initialize the underflow parameter.  This
    1814                  * results in spurious termination of commands
    1815                  * that complete as expected (e.g. underflow is
    1816                  * allowed as command can return variable amounts
    1817                  * of data.
    1818                  */
    1819                 } else if (amount_xferred < scb->io_ctx->underflow) {
    1820                         u_int i;
    1821 
    1822                         ahd_print_path(ahd, scb);
    1823                         printk("CDB:");
    1824                         for (i = 0; i < scb->io_ctx->cmd_len; i++)
    1825                                 printk(" 0x%x", scb->io_ctx->cmnd[i]);
    1826                         printk("\n");
    1827                         ahd_print_path(ahd, scb);
    1828                         printk("Saw underflow (%ld of %ld bytes). "
    1829                                "Treated as error\n",
    1830                                 ahd_get_residual(scb),
    1831                                 ahd_get_transfer_length(scb));
    1832                         ahd_set_transaction_status(scb, CAM_DATA_RUN_ERR);
    1833 #endif
    1834                 } else {
    1835                         ahd_set_transaction_status(scb, CAM_REQ_CMP);
    1836                 }
    1837         } else if (ahd_get_transaction_status(scb) == CAM_SCSI_STATUS_ERROR) {
    1838                 ahd_linux_handle_scsi_status(ahd, cmd->device, scb);

No check here either

    1839         }
    1840 
    1841         if (dev->openings == 1
    1842          && ahd_get_transaction_status(scb) == CAM_REQ_CMP
    1843          && ahd_get_scsi_status(scb) != SAM_STAT_TASK_SET_FULL)
    1844                 dev->tag_success_count++;
    1845         /*
    1846          * Some devices deal with temporary internal resource
    1847          * shortages by returning queue full.  When the queue
    1848          * full occurrs, we throttle back.  Slowly try to get
    1849          * back to our previous queue depth.
    1850          */
    1851         if ((dev->openings + dev->active) < dev->maxtags
    1852          && dev->tag_success_count > AHD_TAG_SUCCESS_INTERVAL) {
    1853                 dev->tag_success_count = 0;
    1854                 dev->openings++;
    1855         }
    1856 
    1857         if (dev->active == 0)
    1858                 dev->commands_since_idle_or_otag = 0;
    1859 
    1860         if ((scb->flags & SCB_RECOVERY_SCB) != 0) {
    1861                 printk("Recovery SCB completes\n");
    1862                 if (ahd_get_transaction_status(scb) == CAM_BDR_SENT
    1863                  || ahd_get_transaction_status(scb) == CAM_REQ_ABORTED)
    1864                         ahd_set_transaction_status(scb, CAM_CMD_TIMEOUT);
    1865 
    1866                 if (ahd->platform_data->eh_done)
    1867                         complete(ahd->platform_data->eh_done);
    1868         }
    1869 
    1870         ahd_free_scb(ahd, scb);
    1871         ahd_linux_queue_cmd_complete(ahd, cmd);

Or here

    1872 }

regards,
dan carpenter



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux