Re: [PATCH v10 1/3] dt-bindings: soundwire: qcom: Increase max data ports to 17

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

On 7/1/26 5:43 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incorrect search size in find_first_zero_bit prevents -EBUSY on port exhaustion and causes silent double-allocation of the highest port.
> - [High] The dt-bindings now allow up to 17 SoundWire data ports, but the corresponding driver internal structures have a hardcoded limit of 15 DAIs, leading to a buffer overflow.

These issues are addressed in this series:

https://lore.kernel.org/linux-arm-msm/20260701193006.4113-1-srinivas.kandagatla@oss.qualcomm.com/T/#t

--srini
> --
> 
> commit bfff7ab83551c0958297b1c39b25931747061203
> Author: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> 
> dt-bindings: soundwire: qcom: Increase max data ports to 17
> 
> Bump the maxItems from 16 to 17 for all qcom,ports-* properties to
> accommodate SoundWire controllers v3.1.0 with 17 data ports.
> 
>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> index 9447a2f371b56..8e6973fa229c9 100644
>> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> @@ -90,7 +90,7 @@ properties:
>>        or applicable for the respective data port.
>>        More info in MIPI Alliance SoundWire 1.0 Specifications.
>>      minItems: 3
>> -    maxItems: 16
>> +    maxItems: 17
> 
> [Severity: High]
> This is a pre-existing issue, but does increasing the number of ports to 17
> expose an out-of-bounds write in the qcom soundwire driver?
> 
> In drivers/soundwire/qcom.c, SWRM_MAX_DAIS is hardcoded to 15:
> 
> #define SWRM_MAX_DAIS						0xF
> 
> Which bounds the sruntime array in struct qcom_swrm_ctrl:
> 
> 	struct sdw_stream_runtime *sruntime[SWRM_MAX_DAIS];
> 
> If a device is configured with 17 ports, will this lead to an out-of-bounds
> write when setting up streams on the 16th or 17th port?
> 
> drivers/soundwire/qcom.c:qcom_swrm_set_sdw_stream() {
> 	...
> 	ctrl->sruntime[dai->id] = stream;
> 	...
> }
> 
> [Severity: High]
> This is a pre-existing issue, but the port exhaustion check in the driver
> appears to have a logic error that could be impacted by changing port counts.
> 
> In drivers/soundwire/qcom.c:qcom_swrm_stream_alloc_ports():
> 
> 	pn = find_first_zero_bit(port_mask, maxport);
> 
> 	if (pn > maxport) {
> 		dev_err(ctrl->dev, "All ports busy\n");
> 		return -EBUSY;
> 	}
> 	set_bit(pn, port_mask);
> 
> Since find_first_zero_bit() returns the size parameter (maxport) if no zero
> bits are found, the check if (pn > maxport) will be false when all ports
> are busy.
> 
> Will this cause the driver to bypass the -EBUSY check and repeatedly assign
> the highest port instead of handling resource exhaustion properly?
>