On 08/07/2022 09:45, Dan Carpenter wrote:
On Fri, Jul 08, 2022 at 09:31:31AM +0100, Srinivas Kandagatla wrote:
Hi Dan,
On 08/07/2022 09:08, Dan Carpenter wrote:
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.
In SoundWire world device id 0 is special one and used for enumerating the SoundWire devices.
Only addresses from 1-11 are valid devids that can be assigned to devices by the host/controller.
This part of the code is reading the devids assigned by the hw auto-enumeration, So the loop start from 1 is correct here.
(Or possibly the other loops are buggy as well).
Atleast this code is fine, but I see other places where are starting from 0 which could be fixed but the SoundWire core will ignore the status for devid 0.
This code is *not* fine either because it should be < instead of <=.
It might be that we always hit a zero first and break so the bug might not affect users but it's still wrong.
I agree, Let me send a fix or send a v2.
--srini
regards, dan carpenter