Hello Tomas Henzl,
Commit 3668651def2c ("scsi: mpi3mr: Sanitise num_phys") from Feb 26,
2024 (linux-next), leads to the following Smatch static checker
warning:
drivers/scsi/mpi3mr/mpi3mr_transport.c:1371 mpi3mr_sas_port_add()
warn: array off by one? 'mr_sas_node->phy[i]'
drivers/scsi/mpi3mr/mpi3mr_transport.c
1352 mr_sas_port->hba_port = hba_port;
1353 mpi3mr_sas_port_sanity_check(mrioc, mr_sas_node,
1354 mr_sas_port->remote_identify.sas_address, hba_port);
1355
1356 if (mr_sas_node->num_phys > sizeof(mr_sas_port->phy_mask) * 8)
1357 ioc_info(mrioc, "max port count %u could be too high\n",
1358 mr_sas_node->num_phys);
1359
1360 for (i = 0; i < mr_sas_node->num_phys; i++) {
1361 if ((mr_sas_node->phy[i].remote_identify.sas_address !=
1362 mr_sas_port->remote_identify.sas_address) ||
1363 (mr_sas_node->phy[i].hba_port != hba_port))
1364 continue;
1365
1366 if (i > sizeof(mr_sas_port->phy_mask) * 8) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This check is wrong. It should be >=. But also ->phy_mask is a u64
when probably it should be a u32.
1367 ioc_warn(mrioc, "skipping port %u, max allowed value is %zu\n",
1368 i, sizeof(mr_sas_port->phy_mask) * 8);
1369 goto out_fail;
1370 }
--> 1371 list_add_tail(&mr_sas_node->phy[i].port_siblings,
1372 &mr_sas_port->phy_list);
1373 mr_sas_port->num_phys++;
1374 mr_sas_port->phy_mask |= (1 << i);
^^^^^^
There are a bunch of "1 << i" shifts in this file and they'll shift wrap
if i >= 32. Then the ->phy_mask is tested with ffs() which takes an
int. So everything above bit 31 is not going to work.
1375 }
1376
1377 if (!mr_sas_port->num_phys) {
1378 ioc_err(mrioc, "failure at %s:%d/%s()!\n",
1379 __FILE__, __LINE__, __func__);
1380 goto out_fail;
1381 }
regards,
dan carpenter
