| Age | Commit message (Collapse) | Author |
|---|
|
[ Upstream commit 068fdad20454f815e61e6f6eb9f051a8b3120e88 ]
If the endpoint completion callback is call right after the ep_enabled flag
is cleared and before usb_ep_dequeue() is call, we could do a double free
on the request and the associated buffer.
Fix this by clearing ep_enabled after all the endpoint requests have been
dequeued.
Fixes: 7de8681be2cd ("usb: gadget: u_audio: Free requests only after callback")
Cc: stable <stable@vger.kernel.org>
Reported-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://lore.kernel.org/r/20210827092927.366482-1-jbrunet@baylibre.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 7de8681be2cde9f6953d3be1fa6ce05f9fe6e637 ]
As per the kernel doc for usb_ep_dequeue(), it states that "this
routine is asynchronous, that is, it may return before the completion
routine runs". And indeed since v5.0 the dwc3 gadget driver updated
its behavior to place dequeued requests on to a cancelled list to be
given back later after the endpoint is stopped.
The free_ep() was incorrectly assuming that a request was ready to
be freed after calling dequeue which results in a use-after-free
in dwc3 when it traverses its cancelled list. Fix this by moving
the usb_ep_free_request() call to the callback itself in case the
ep is disabled.
Fixes: eb9fecb9e69b0 ("usb: gadget: f_uac2: split out audio core")
Reported-and-tested-by: Ferry Toth <fntoth@gmail.com>
Reviewed-and-tested-by: Peter Chen <peter.chen@nxp.com>
Acked-by: Felipe Balbi <balbi@kernel.org>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://lore.kernel.org/r/20210118084642.322510-2-jbrunet@baylibre.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
The change protects almost the whole body of u_audio_iso_complete()
function by PCM stream lock, this is mainly sufficient to avoid a race
between USB request completion and stream termination, the change
prevents a possibility of invalid memory access in interrupt context
by memcpy():
Unable to handle kernel paging request at virtual address 00004e80
pgd = c0004000
[00004e80] *pgd=00000000
Internal error: Oops: 817 [#1] PREEMPT SMP ARM
CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G C 3.14.54+ #117
task: da180b80 ti: da192000 task.ti: da192000
PC is at memcpy+0x50/0x330
LR is at 0xcdd92b0e
pc : [<c029ef30>] lr : [<cdd92b0e>] psr: 20000193
sp : da193ce4 ip : dd86ae26 fp : 0000b180
r10: daf81680 r9 : 00000000 r8 : d58a01ea
r7 : 2c0b43e4 r6 : acdfb08b r5 : 01a271cf r4 : 87389377
r3 : 69469782 r2 : 00000020 r1 : daf82fe0 r0 : 00004e80
Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: 2b70804a DAC: 00000015
Process ksoftirqd/0 (pid: 3, stack limit = 0xda192238)
Also added a check for potential !runtime condition, commonly it is
done by PCM_RUNTIME_CHECK(substream) in the beginning, however this
does not completely prevent from oopses in u_audio_iso_complete(),
because the proper protection scheme must be implemented in PCM
library functions.
An example of *not fixed* oops due to substream->runtime->*
dereference by snd_pcm_running(substream) from
snd_pcm_period_elapsed(), where substream->runtime is gone while
waiting the substream lock:
Unable to handle kernel paging request at virtual address 6b6b6b6b
pgd = db7e4000
[6b6b6b6b] *pgd=00000000
CPU: 0 PID: 193 Comm: klogd Tainted: G C 3.14.54+ #118
task: db5ac500 ti: db60c000 task.ti: db60c000
PC is at snd_pcm_period_elapsed+0x48/0xd8 [snd_pcm]
LR is at snd_pcm_period_elapsed+0x40/0xd8 [snd_pcm]
pc : [<>] lr : [<>] psr: 60000193
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c5387d Table: 2b7e404a DAC: 00000015
Process klogd (pid: 193, stack limit = 0xdb60c238)
[<>] (snd_pcm_period_elapsed [snd_pcm]) from [<>] (udc_irq+0x500/0xbbc)
[<>] (udc_irq) from [<>] (ci_irq+0x280/0x304)
[<>] (ci_irq) from [<>] (handle_irq_event_percpu+0xa4/0x40c)
[<>] (handle_irq_event_percpu) from [<>] (handle_irq_event+0x3c/0x5c)
[<>] (handle_irq_event) from [<>] (handle_fasteoi_irq+0xc4/0x110)
[<>] (handle_fasteoi_irq) from [<>] (generic_handle_irq+0x20/0x30)
[<>] (generic_handle_irq) from [<>] (handle_IRQ+0x80/0xc0)
[<>] (handle_IRQ) from [<>] (gic_handle_irq+0x3c/0x60)
[<>] (gic_handle_irq) from [<>] (__irq_svc+0x44/0x78)
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[erosca: W/o this patch, with minimal instrumentation [1], I can
consistently reproduce BUG: KASAN: use-after-free [2]]
[1] Instrumentation to reproduce issue [2]:
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index a72295c953bb..bd0b308024fe 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -16,6 +16,7 @@
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
+#include <linux/delay.h>
#include "u_audio.h"
@@ -147,6 +148,8 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
spin_unlock_irqrestore(&prm->lock, flags);
+ udelay(500); //delay here to increase probability of parallel activities
+
/* Pack USB load in ALSA ring buffer */
pending = prm->dma_bytes - hw_ptr;
[2] After applying [1], below BUG occurs on Rcar-H3-Salvator-X board:
==================================================================
BUG: KASAN: use-after-free in u_audio_iso_complete+0x24c/0x520 [u_audio]
Read of size 8 at addr ffff8006cafcc248 by task swapper/0/0
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G WC 4.14.47+ #160
Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT)
Call trace:
[<ffff2000080925ac>] dump_backtrace+0x0/0x364
[<ffff200008092924>] show_stack+0x14/0x1c
[<ffff200008f8dbcc>] dump_stack+0x108/0x174
[<ffff2000083c71b8>] print_address_description+0x7c/0x32c
[<ffff2000083c78e8>] kasan_report+0x324/0x354
[<ffff2000083c6114>] __asan_load8+0x24/0x94
[<ffff2000021d1b34>] u_audio_iso_complete+0x24c/0x520 [u_audio]
[<ffff20000152fe50>] usb_gadget_giveback_request+0x480/0x4d0 [udc_core]
[<ffff200001860ab8>] usbhsg_queue_done+0x100/0x130 [renesas_usbhs]
[<ffff20000185f814>] usbhsf_pkt_handler+0x1a4/0x298 [renesas_usbhs]
[<ffff20000185fb38>] usbhsf_irq_ready+0x128/0x178 [renesas_usbhs]
[<ffff200001859cc8>] usbhs_interrupt+0x440/0x490 [renesas_usbhs]
[<ffff2000081a0288>] __handle_irq_event_percpu+0x594/0xa58
[<ffff2000081a07d0>] handle_irq_event_percpu+0x84/0x12c
[<ffff2000081a0928>] handle_irq_event+0xb0/0x10c
[<ffff2000081a8384>] handle_fasteoi_irq+0x1e0/0x2ec
[<ffff20000819e5f8>] generic_handle_irq+0x2c/0x44
[<ffff20000819f0d0>] __handle_domain_irq+0x190/0x194
[<ffff20000808177c>] gic_handle_irq+0x80/0xac
Exception stack(0xffff200009e97c80 to 0xffff200009e97dc0)
7c80: 0000000000000000 0000000000000000 0000000000000003 ffff200008179298
7ca0: ffff20000ae1c180 dfff200000000000 0000000000000000 ffff2000081f9a88
7cc0: ffff200009eb5960 ffff200009e97cf0 0000000000001600 ffff0400041b064b
7ce0: 0000000000000000 0000000000000002 0000000200000001 0000000000000001
7d00: ffff20000842197c 0000ffff958c4970 0000000000000000 ffff8006da0d5b80
7d20: ffff8006d4678498 0000000000000000 000000126bde0a8b ffff8006d4678480
7d40: 0000000000000000 000000126bdbea64 ffff200008fd0000 ffff8006fffff980
7d60: 00000000495f0018 ffff200009e97dc0 ffff200008b6c4ec ffff200009e97dc0
7d80: ffff200008b6c4f0 0000000020000145 ffff8006da0d5b80 ffff8006d4678498
7da0: ffffffffffffffff ffff8006d4678498 ffff200009e97dc0 ffff200008b6c4f0
[<ffff200008084034>] el1_irq+0xb4/0x12c
[<ffff200008b6c4f0>] cpuidle_enter_state+0x818/0x844
[<ffff200008b6c59c>] cpuidle_enter+0x18/0x20
[<ffff20000815f2e4>] call_cpuidle+0x98/0x9c
[<ffff20000815f674>] do_idle+0x214/0x264
[<ffff20000815facc>] cpu_startup_entry+0x20/0x24
[<ffff200008fb09d8>] rest_init+0x30c/0x320
[<ffff2000095f1338>] start_kernel+0x570/0x5b0
---<-snip->---
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Substream period size potentially can be changed in runtime, however
this is not accounted in the data copying routine, the change replaces
the cached value with an actual value from substream runtime.
As a side effect the change also removes a potential division by zero
in u_audio_iso_complete() function, if there is a race with
uac_pcm_hw_free(), which sets prm->period_size to 0.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
There is no necessity to copy PCM stream ring buffer area and size
properties to UAC private data structure, these values can be got
from substream itself.
The change gives more control on substream and avoid stale caching.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
In u_audio_iso_complete, the runtime hw_ptr is updated before the
data is actually copied over to/from the buffer/dma area. When
ALSA uses this hw_ptr, the data may not actually be available to
be used. This causes trash/stale audio to play/record. This
patch updates the hw_ptr after the data has been copied to avoid
this.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Fix below smatch (v0.5.0-4443-g69e9094e11c1) warnings:
drivers/usb/gadget/function/u_audio.c:607 g_audio_setup() warn: strcpy() 'pcm_name' of unknown size might be too large for 'pcm->name'
drivers/usb/gadget/function/u_audio.c:614 g_audio_setup() warn: strcpy() 'card_name' of unknown size might be too large for 'card->driver'
drivers/usb/gadget/function/u_audio.c:615 g_audio_setup() warn: strcpy() 'card_name' of unknown size might be too large for 'card->shortname'
Below commits performed a similar 's/strcpy/strlcpy/' rework:
* v2.6.31 commit 8372d4980fbc ("ALSA: ctxfi - Fix PCM device naming")
* v4.14 commit 003d3e70dbeb ("ALSA: ad1848: fix format string overflow warning")
* v4.14 commit 6d8b04de87e1 ("ALSA: cs423x: fix format string overflow warning")
Fixes: eb9fecb9e69b ("usb: gadget: f_uac2: split out audio core")
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Now that the SPDX tag is in all USB files, that identifies the license
in a specific and legally-defined manner. So the extra GPL text wording
can be removed as it is no longer needed at all.
This is done on a quest to remove the 700+ different ways that files in
the kernel describe the GPL license text. And there's unneeded stuff
like the address (sometimes incorrect) for the FSF which is never
needed.
No copyright headers or other non-license-description text was removed.
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Johan Hovold <johan@kernel.org>
Cc: Jerry Zhang <zhangjerry@google.com>
Cc: John Keeping <john@metanate.com>
Cc: Krzysztof Opasiak <k.opasiak@samsung.com>
Cc: Abdulhadi Mohamed <abdulahhadi2@gmail.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Janusz Dziedzic <januszx.dziedzic@linux.intel.com>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Vincent Pelletier <plr.vincent@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
It's good to have SPDX identifiers in all files to make it easier to
audit the kernel tree for correct licenses.
Update the drivers/usb/ and include/linux/usb* files with the correct
SPDX license identifier based on the license text in the file itself.
The SPDX identifier is a legally binding shorthand, which can be used
instead of the full boiler plate text.
This work is based on a script and data from Thomas Gleixner, Philippe
Ombredanne, and Kate Stewart.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Acked-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Make this const as it is only used during a copy operation.
Done using Coccinelle.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
snd_pcm_ops are not supposed to change at runtime. All functions
working with snd_pcm_ops provided by <sound/pcm.h> work with
const snd_pcm_ops. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Abstract the peripheral side ALSA sound card code from
the f_uac2 function into a component that can be called
by various functions, so the various flavors can be split
apart and selectively reused.
Visible changes:
- add uac_params structure to pass audio paramteres for
g_audio_setup
- make ALSA sound card's name configurable
- add [in/out]_ep_maxpsize
- allocate snd_uac_chip structure during g_audio_setup
- add u_audio_[start/stop]_[capture/playback] functions
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
Jerome Brunet
Jack Pham
Vladimir Zapolskiy
Joshua Frkuska
Eugeniu Rosca
Greg Kroah-Hartman
Bhumika Goyal
Arvind Yadav
Ruslan Bilovol