[PATCH v3] usb: gadget: f_uac1_legacy: cancel playback work and drain buffers on free

[PATCH v3] usb: gadget: f_uac1_legacy: cancel playback work and drain buffers on free

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

f_audio_free() tears down the f_audio structure without cancelling
playback_work or draining the play_queue.  The pending work can fire
after the structure is freed, and queued audio buffers leak.

Cancel the work, walk the play_queue to free every queued buffer, and
free copy_buf when it was not already queued.  When f_audio_set_alt()
is called with alt 0 it queues copy_buf to play_queue without clearing
audio->copy_buf, so the drain detects this and clears the pointer to
prevent a double free.  Use IS_ERR_OR_NULL for the final copy_buf check
because f_audio_buffer_alloc() returns ERR_PTR on failure and
f_audio_set_alt() stores the result directly in audio->copy_buf.

Fixes: c6994e6f067c ("USB: gadget: add USB Audio Gadget driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---

Changes in v3:
- Fix double-free of copy_buf: when f_audio_set_alt(alt=0) queues
  copy_buf to play_queue without clearing audio->copy_buf, the drain
  now detects this and clears the pointer before freeing
- Change copy_buf NULL check to IS_ERR_OR_NULL because
  f_audio_buffer_alloc() returns ERR_PTR(-ENOMEM) on failure

Changes in v2:
- Drain play_queue and free copy_buf in addition to cancelling work

 drivers/usb/gadget/function/f_uac1_legacy.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index 7c8d60c7e87d1..88c816b4ba119 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -9,6 +9,7 @@
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/err.h>
 #include <linux/device.h>
 #include <linux/atomic.h>
 
@@ -942,7 +943,17 @@ static void f_audio_free(struct usb_function *f)
 {
 	struct f_audio *audio = func_to_audio(f);
 	struct f_uac1_legacy_opts *opts;
-
+	struct f_audio_buf *buf, *tmp;
+
+	cancel_work_sync(&audio->playback_work);
+	list_for_each_entry_safe(buf, tmp, &audio->play_queue, list) {
+		list_del(&buf->list);
+		if (buf == audio->copy_buf)
+			audio->copy_buf = NULL;
+		f_audio_buffer_free(buf);
+	}
+	if (!IS_ERR_OR_NULL(audio->copy_buf))
+		f_audio_buffer_free(audio->copy_buf);
 	gaudio_cleanup(&audio->card);
 	opts = container_of(f->fi, struct f_uac1_legacy_opts, func_inst);
 	kfree(audio);
-- 
2.47.3