Re: [PATCH v2 06/10] soundwire: qcom: Prevent pconfig buffer overflow in qcom_swrm_stream_alloc_ports()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soundwire: qcom: qcom_swrm_stream_alloc_ports() rollback clears active ports
--

commit ff22f63970718cc2818a7a2729f4f9674b452688
Author: Lee Jones <lee@kernel.org>

soundwire: qcom: Prevent pconfig buffer overflow in qcom_swrm_stream_alloc_ports()

This commit bounds-checks nports against maxport before writing to
pconfig[nports] in qcom_swrm_stream_alloc_ports(), and clears pconfig[].num
bits from port_mask on error.

> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 6e0c5fb10efad..29130745c8097 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -1350,8 +1350,10 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>  				else
>  					pn = find_first_zero_bit(port_mask, maxport);
>  
> -				if (pn >= maxport) {
> +				if (pn >= maxport || nports >= maxport) {
>  					dev_err(ctrl->dev, "All ports busy\n");
> +					while (nports--)
> +						clear_bit(pconfig[nports].num, port_mask);
>  					return -EBUSY;
>  				}

[Severity: Medium]
Can this rollback loop clear bits in ctrl->port_mask that were already in use
before qcom_swrm_stream_alloc_ports() was called?

When slave->m_port_map[p_rt->num] is non-zero, qcom_swrm_stream_alloc_ports()
assigns pn = m_port directly without checking whether bit pn was already set
in ctrl->port_mask:

drivers/soundwire/qcom.c:qcom_swrm_stream_alloc_ports() {
	...
	m_port = slave->m_port_map[p_rt->num];
	/* Port numbers start from 1 - 14*/
	if (m_port)
		pn = m_port;
	else
		pn = find_first_zero_bit(port_mask, maxport);
	...
	set_bit(pn, port_mask);
	pconfig[nports].num = pn;
}

Because ctrl->port_mask is shared across all DAIs on the controller (accessed
via qcom_swrm_hw_params() -> qcom_swrm_stream_alloc_ports()), bit m_port may
already be set by another active DAI stream or by an earlier slave port
sharing the same master port.

If a subsequent port in the loop hits the error condition (pn >= maxport ||
nports >= maxport), the new while (nports--) loop unconditionally calls
clear_bit(pconfig[nports].num, port_mask), clearing that pre-existing bit in
ctrl->port_mask and allowing future find_first_zero_bit() calls to hand out an
in-use master port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103095445.81414-1-lee@kernel.org?part=6