[bug report] soundwire: qcom: Check device status before reading devid

Dan Carpenter dan.carpenter at oracle.com
Fri Jul 8 10:08:14 CEST 2022


Hello Srinivas Kandagatla,

The patch aa1262ca6695: "soundwire: qcom: Check device status before
reading devid" from Jul 6, 2022, leads to the following Smatch static
checker warning:

	drivers/soundwire/qcom.c:484 qcom_swrm_enumerate()
	error: buffer overflow 'ctrl->status' 11 <= 11

drivers/soundwire/qcom.c
    471 static int qcom_swrm_enumerate(struct sdw_bus *bus)
    472 {
    473         struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
    474         struct sdw_slave *slave, *_s;
    475         struct sdw_slave_id id;
    476         u32 val1, val2;
    477         bool found;
    478         u64 addr;
    479         int i;
    480         char *buf1 = (char *)&val1, *buf2 = (char *)&val2;
    481 
    482         for (i = 1; i <= SDW_MAX_DEVICES; i++) {
                     ^^^^^
This a loop that starts from 1 instead of 0.  I looked at the
surrounding context and it seems like it should be a normal loop that
starts at 0 and goes to < SDW_MAX_DEVICES.

(Or possibly the other loops are buggy as well).

    483                 /* do not continue if the status is Not Present  */
--> 484                 if (!ctrl->status[i])

So this is off by one and reads one element beyond the end of the loop.

    485                         continue;
    486 
    487                 /*SCP_Devid5 - Devid 4*/
    488                 ctrl->reg_read(ctrl, SWRM_ENUMERATOR_SLAVE_DEV_ID_1(i), &val1);
    489 
    490                 /*SCP_Devid3 - DevId 2 Devid 1 Devid 0*/
    491                 ctrl->reg_read(ctrl, SWRM_ENUMERATOR_SLAVE_DEV_ID_2(i), &val2);
    492 
    493                 if (!val1 && !val2)
    494                         break;
    495 
    496                 addr = buf2[1] | (buf2[0] << 8) | (buf1[3] << 16) |
    497                         ((u64)buf1[2] << 24) | ((u64)buf1[1] << 32) |
    498                         ((u64)buf1[0] << 40);
    499 
    500                 sdw_extract_slave_id(bus, addr, &id);
    501                 found = false;
    502                 /* Now compare with entries */
    503                 list_for_each_entry_safe(slave, _s, &bus->slaves, node) {
    504                         if (sdw_compare_devid(slave, id) == 0) {
    505                                 qcom_swrm_set_slave_dev_num(bus, slave, i);
    506                                 found = true;
    507                                 break;
    508                         }
    509                 }
    510 
    511                 if (!found) {
    512                         qcom_swrm_set_slave_dev_num(bus, NULL, i);
    513                         sdw_slave_add(bus, &id, NULL);
    514                 }
    515         }
    516 
    517         complete(&ctrl->enumeration);
    518         return 0;
    519 }

regards,
dan carpenter


More information about the Alsa-devel mailing list