On 10/17/23 15:38, Dan Carpenter wrote:
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
This has been fixed with the latest version.
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
Correct, needs to be fixed.
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
Needs to be fixed, too.
Will be sending a patch.
Cheers,
Hannes
