Re: [PATCH 03/21] soundwire: stream: Prevent m_rt_count underflow in sdw_stream_remove_master()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soundwire: stream: m_rt_count leak in sdw_stream_remove_master()
--

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

soundwire: stream: Prevent m_rt_count underflow in sdw_stream_remove_master()

This commit frees partially allocated master ports on allocation failure in
sdw_master_port_alloc() and gates the stream->m_rt_count decrement in
sdw_stream_remove_master() on sdw_master_port_allocated(m_rt).

> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 0b00fcdcab2d6..f225e5438ccfe 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c

[ ... ]

> @@ -2108,9 +2110,10 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
>  		if (m_rt->bus != bus)
>  			continue;
>  
> +		if (sdw_master_port_allocated(m_rt))
> +			stream->m_rt_count--;
>  		sdw_master_port_free(m_rt);
>  		sdw_master_rt_free(m_rt, stream);
> -		stream->m_rt_count--;

[Severity: Medium]
Can this leave stream->m_rt_count elevated when sdw_stream_add_master() is
called with num_ports == 0?

In sdw_stream_add_master(), when !sdw_master_port_allocated(m_rt) is true,
sdw_master_port_alloc(m_rt, num_ports) is called:

drivers/soundwire/stream.c:sdw_stream_add_master() {
	...
	if (!sdw_master_port_allocated(m_rt)) {
		ret = sdw_master_port_alloc(m_rt, num_ports);
		if (ret)
			goto alloc_error;

		stream->m_rt_count++;
	}
	...
}

When num_ports == 0 (for example via qcom_swrm_hw_params() ->
qcom_swrm_stream_alloc_ports() -> sdw_stream_add_master() when a codec DAI has
no active ports enabled), sdw_master_port_alloc() executes zero loop
iterations and returns 0 while leaving m_rt->port_list empty, and
sdw_stream_add_master() increments stream->m_rt_count.

When sdw_stream_remove_master() later tears down the master runtime,
sdw_master_port_allocated(m_rt) checks !list_empty(&m_rt->port_list), which is
false because m_rt->port_list is empty. As a result, stream->m_rt_count is not
decremented before m_rt is freed, causing subsequent sdw_stream_add_master()
calls on single-link buses (!bus->multi_link) to fail with -EINVAL or
miscounting active master runtimes in do_bank_switch().

Could sdw_master_runtime track whether sdw_stream_add_master() incremented
m_rt_count, or should the num_ports == 0 case be handled consistently between
sdw_stream_add_master() and sdw_stream_remove_master()?

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