[PATCH] soundwire: fix double free of pointer

[PATCH] soundwire: fix double free of pointer

[Daniil Dulov]

If sdw_ml_sync_bank_switch() returns error not on the first iteration,
it leads to freeing prevously freed memory. So, set the pointer to NULL
after each successful bank switch.

Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
---
 drivers/soundwire/stream.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 304ff2ee7d75..d650e6f0f8e7 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -833,6 +833,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 				"multi link bank switch failed: %d\n", ret);
 			goto error;
 		}
+		bus->defer_msg.msg = NULL;
 
 		if (multi_link)
 			mutex_unlock(&bus->msg_lock);
-- 
2.25.1

Re: [PATCH] soundwire: fix double free of pointer

[Pierre-Louis Bossart]

On 2/11/24 09:09, Daniil Dulov wrote:
> If sdw_ml_sync_bank_switch() returns error not on the first iteration,
> it leads to freeing prevously freed memory. So, set the pointer to NULL
> after each successful bank switch.
> 
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
> ---
>  drivers/soundwire/stream.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 304ff2ee7d75..d650e6f0f8e7 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -833,6 +833,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
>  				"multi link bank switch failed: %d\n", ret);
>  			goto error;
>  		}
> +		bus->defer_msg.msg = NULL;
>  
>  		if (multi_link)
>  			mutex_unlock(&bus->msg_lock);

Not following what the issue is...

On success, sdw_ml_sync_bank_switch() frees the buffers with

	if (bus->defer_msg.msg) {
		kfree(bus->defer_msg.msg->buf);
		kfree(bus->defer_msg.msg);
		bus->defer_msg.msg = NULL;
	}

So if there is an issue on the second iteration, then the loop will
detect already freed memory in the previous iteration and skip it:

                /* Check if bank switch was successful */
		ret = sdw_ml_sync_bank_switch(bus);
		if (ret < 0) {
			dev_err(bus->dev,
				"multi link bank switch failed: %d\n", ret);
			goto error;
		}

error:
	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
		bus = m_rt->bus;
		if (bus->defer_msg.msg) { <<<< TEST FOR FREED MEMORY
			kfree(bus->defer_msg.msg->buf);
			kfree(bus->defer_msg.msg);
			bus->defer_msg.msg = NULL;
		}
	}

It could very well be that I need more coffee on this post-SuperBowl
Monday morning, but I just don't see the problem.