2019-11-04 07:10:10

by youling 257

[permalink] [raw]
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*

This patch cause oops on android mainline kernel about gadget audio_source.
Androidx86 run on android mainline kernel.

[ 385.104963] android_work: sent uevent USB_STATE=CONNECTED
[ 385.109006] android_work: sent uevent USB_STATE=DISCONNECTED
[ 385.182024] android_work: sent uevent USB_STATE=CONNECTED
[ 385.184737] configfs-gadget gadget: high-speed config #1: b
[ 385.184921] android_work: sent uevent USB_STATE=CONFIGURED
[ 385.285268] BUG: kernel NULL pointer dereference, address: 0000000000000220
[ 385.285339] #PF: supervisor read access in kernel mode
[ 385.285374] #PF: error_code(0x0000) - not-present page
[ 385.285436] PGD 80000000791e6067 P4D 80000000791e6067 PUD 0
[ 385.285473] Oops: 0000 [#1] PREEMPT SMP PTI
[ 385.285509] CPU: 0 PID: 5780 Comm: Binder:1383_5 Tainted: G O
5.4.0-rc6-android-x86_64+ #1
[ 385.285571] Hardware name: Insyde ONDA Tablet/ONDA Tablet, BIOS
ONDA.D890HBBNR0A 03/11/2015
[ 385.285639] RIP: 0010:dma_can_mmap+0x5/0x30
[ 385.285675] Code: 74 11 e9 ae 98 b2 00 48 8b 05 9f 40 94 01 48 85 c0
75 e3 31 c0 c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 0f 1f 44
00 00 <48> 8b 87 20 02 00 00 48 85 c0 74 09 48 83 78 10 00 0f 95 c0 c3
48
[ 385.285762] RSP: 0018:ffffb39443b63b78 EFLAGS: 00010246
[ 385.285797] RAX: 0000000000000001 RBX: ffffa28b91756600 RCX: 0000000000000040
[ 385.285857] RDX: ffffffffb62b2a00 RSI: 0000000000000000 RDI: 0000000000000000
[ 385.285917] RBP: ffffa28bac69f800 R08: ffffffffb63141b0 R09: ffffa28bf9a34a88
[ 385.285952] R10: ffffffffb62b2be0 R11: ffffffffb62b2bd0 R12: 0000000000000008
[ 385.286013] R13: 0000000000000000 R14: ffffa28bacd736a8 R15: ffffa28bacd736c8
[ 385.286076] FS: 0000000000000000(0000) GS:ffffa28bfb600000(0063)
knlGS:00000000f5d81970
[ 385.286110] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 385.286171] CR2: 0000000000000220 CR3: 000000007b2f6000 CR4: 00000000001006f0
[ 385.286232] Call Trace:
[ 385.286275] snd_pcm_hw_constraints_complete+0x3e/0x1f0
[ 385.286314] snd_pcm_open_substream+0x94/0x140
[ 385.286377] snd_pcm_open+0xf0/0x240
[ 385.286416] ? wake_up_q+0x60/0x60
[ 385.286460] snd_pcm_playback_open+0x3d/0x60
[ 385.286533] chrdev_open+0xa2/0x1c0
[ 385.286574] ? cdev_put.part.0+0x20/0x20
[ 385.286615] do_dentry_open+0x13a/0x380
[ 385.286686] path_openat+0x588/0x15d0
[ 385.286728] do_filp_open+0x91/0x100
[ 385.286769] ? __check_object_size+0x136/0x147
[ 385.286840] do_sys_open+0x184/0x280
[ 385.286880] ? handle_mm_fault+0xd7/0x1c0
[ 385.286920] do_fast_syscall_32+0x8e/0x250
[ 385.286992] entry_SYSENTER_compat+0x7c/0x8e

[ 385.287302] CR2: 0000000000000220
[ 385.287391] ---[ end trace 73ffcefcbbe2b9a0 ]---
[ 385.296269] RIP: 0010:dma_can_mmap+0x5/0x30
[ 385.296337] Code: 74 11 e9 ae 98 b2 00 48 8b 05 9f 40 94 01 48 85 c0
75 e3 31 c0 c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 0f 1f 44
00 00 <48> 8b 87 20 02 00 00 48 85 c0 74 09 48 83 78 10 00 0f 95 c0 c3
48
[ 385.296407] RSP: 0018:ffffb39443b63b78 EFLAGS: 00010246
[ 385.296477] RAX: 0000000000000001 RBX: ffffa28b91756600 RCX: 0000000000000040
[ 385.296516] RDX: ffffffffb62b2a00 RSI: 0000000000000000 RDI: 0000000000000000
[ 385.296584] RBP: ffffa28bac69f800 R08: ffffffffb63141b0 R09: ffffa28bf9a34a88
[ 385.296654] R10: ffffffffb62b2be0 R11: ffffffffb62b2bd0 R12: 0000000000000008
[ 385.296693] R13: 0000000000000000 R14: ffffa28bacd736a8 R15: ffffa28bacd736c8
[ 385.296761] FS: 0000000000000000(0000) GS:ffffa28bfb600000(0063)
knlGS:00000000f5d81970
[ 385.296830] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 385.296867] CR2: 0000000000000220 CR3: 000000007b2f6000 CR4: 00000000001006f0
[ 385.296936] Kernel panic - not syncing: Fatal exception
[ 385.296985] Kernel Offset: 0x33e00000 from 0xffffffff81000000
(relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[ 385.305185] Rebooting in 5 seconds..

Revert it no the oops.

Revert "ALSA: pcm: use dma_can_mmap() to check if a
device supports dma_mmap_*"

This reverts commit 425da159707b271dc865d7e167ac104a0e60e4af.
---
sound/core/pcm_native.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 91c6ad58729f..61f6229c9124 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -220,12 +220,13 @@ static bool hw_support_mmap(struct
snd_pcm_substream *substream)
{
if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP))
return false;
-
- if (substream->ops->mmap ||
- substream->dma_buffer.dev.type != SNDRV_DMA_TYPE_DEV)
- return true;
-
- return dma_can_mmap(substream->dma_buffer.dev.dev);
+ /* architecture supports dma_mmap_coherent()? */
+#if defined(CONFIG_MMU) || !defined(CONFIG_HAS_DMA)
+ if (!substream->ops->mmap &&
+ substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)
+ return false;
+#endif
+ return true;
}

static int constrain_mask_params(struct snd_pcm_substream *substream,


2019-11-04 08:03:41

by Takashi Iwai

[permalink] [raw]
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*

On Mon, 04 Nov 2019 08:06:37 +0100,
youling 257 wrote:
>
> This patch cause oops on android mainline kernel about gadget audio_source.
> Androidx86 run on android mainline kernel.
>
> [ 385.104963] android_work: sent uevent USB_STATE=CONNECTED
> [ 385.109006] android_work: sent uevent USB_STATE=DISCONNECTED
> [ 385.182024] android_work: sent uevent USB_STATE=CONNECTED
> [ 385.184737] configfs-gadget gadget: high-speed config #1: b
> [ 385.184921] android_work: sent uevent USB_STATE=CONFIGURED
> [ 385.285268] BUG: kernel NULL pointer dereference, address: 0000000000000220
> [ 385.285339] #PF: supervisor read access in kernel mode
> [ 385.285374] #PF: error_code(0x0000) - not-present page
> [ 385.285436] PGD 80000000791e6067 P4D 80000000791e6067 PUD 0
> [ 385.285473] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 385.285509] CPU: 0 PID: 5780 Comm: Binder:1383_5 Tainted: G O
> 5.4.0-rc6-android-x86_64+ #1
> [ 385.285571] Hardware name: Insyde ONDA Tablet/ONDA Tablet, BIOS
> ONDA.D890HBBNR0A 03/11/2015
> [ 385.285639] RIP: 0010:dma_can_mmap+0x5/0x30
> [ 385.285675] Code: 74 11 e9 ae 98 b2 00 48 8b 05 9f 40 94 01 48 85 c0
> 75 e3 31 c0 c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 0f 1f 44
> 00 00 <48> 8b 87 20 02 00 00 48 85 c0 74 09 48 83 78 10 00 0f 95 c0 c3
> 48
> [ 385.285762] RSP: 0018:ffffb39443b63b78 EFLAGS: 00010246
> [ 385.285797] RAX: 0000000000000001 RBX: ffffa28b91756600 RCX: 0000000000000040
> [ 385.285857] RDX: ffffffffb62b2a00 RSI: 0000000000000000 RDI: 0000000000000000
> [ 385.285917] RBP: ffffa28bac69f800 R08: ffffffffb63141b0 R09: ffffa28bf9a34a88
> [ 385.285952] R10: ffffffffb62b2be0 R11: ffffffffb62b2bd0 R12: 0000000000000008
> [ 385.286013] R13: 0000000000000000 R14: ffffa28bacd736a8 R15: ffffa28bacd736c8
> [ 385.286076] FS: 0000000000000000(0000) GS:ffffa28bfb600000(0063)
> knlGS:00000000f5d81970
> [ 385.286110] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
> [ 385.286171] CR2: 0000000000000220 CR3: 000000007b2f6000 CR4: 00000000001006f0
> [ 385.286232] Call Trace:
> [ 385.286275] snd_pcm_hw_constraints_complete+0x3e/0x1f0
> [ 385.286314] snd_pcm_open_substream+0x94/0x140
> [ 385.286377] snd_pcm_open+0xf0/0x240
> [ 385.286416] ? wake_up_q+0x60/0x60
> [ 385.286460] snd_pcm_playback_open+0x3d/0x60
> [ 385.286533] chrdev_open+0xa2/0x1c0
> [ 385.286574] ? cdev_put.part.0+0x20/0x20
> [ 385.286615] do_dentry_open+0x13a/0x380
> [ 385.286686] path_openat+0x588/0x15d0
> [ 385.286728] do_filp_open+0x91/0x100
> [ 385.286769] ? __check_object_size+0x136/0x147
> [ 385.286840] do_sys_open+0x184/0x280
> [ 385.286880] ? handle_mm_fault+0xd7/0x1c0
> [ 385.286920] do_fast_syscall_32+0x8e/0x250
> [ 385.286992] entry_SYSENTER_compat+0x7c/0x8e
>
> [ 385.287302] CR2: 0000000000000220
> [ 385.287391] ---[ end trace 73ffcefcbbe2b9a0 ]---

Exactly which driver is hit? The code path is via hw_support_mmap()
and it's currently:

static bool hw_support_mmap(struct snd_pcm_substream *substream)
{
if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP))
return false;

if (substream->ops->mmap ||
substream->dma_buffer.dev.type != SNDRV_DMA_TYPE_DEV)
return true;

return dma_can_mmap(substream->dma_buffer.dev.dev);
}

so at least the driver has already set the SNDRV_DMA_TYPE_DEV
explicitly (it's non-zero) and some device object, but the device
object was invalid for dma_can_mmap() call.

This smells more like a driver-side issue, not in the core side.


thanks,

Takashi

2019-11-04 08:28:25

by youling 257

[permalink] [raw]
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*

This driver https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/gadget/function/f_audio_source.c

2019-11-04 16:02 GMT+08:00, Takashi Iwai <[email protected]>:

> Exactly which driver is hit? The code path is via hw_support_mmap()
> and it's currently:
>
> static bool hw_support_mmap(struct snd_pcm_substream *substream)
> {
> if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP))
> return false;
>
> if (substream->ops->mmap ||
> substream->dma_buffer.dev.type != SNDRV_DMA_TYPE_DEV)
> return true;
>
> return dma_can_mmap(substream->dma_buffer.dev.dev);
> }
>
> so at least the driver has already set the SNDRV_DMA_TYPE_DEV
> explicitly (it's non-zero) and some device object, but the device
> object was invalid for dma_can_mmap() call.
>
> This smells more like a driver-side issue, not in the core side.
>
>
> thanks,
>
> Takashi
>

2019-11-04 08:35:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*

On 11/4/19 9:27 AM, youling 257 wrote:
> This driver
https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/gadget/function/f_audio_source.c
>

The driver is broken and needs to be fixed. Please feel free to submit
patches to AOSP to do so as you can trigger this easily.

thanks,

greg k-h

2019-11-04 08:51:15

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*

On 11/4/19 9:32 AM, Greg Kroah-Hartman wrote:
> On 11/4/19 9:27 AM, youling 257 wrote:
> > This driver
>
https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/gadget/function/f_audio_source.c

>
> >
>
> The driver is broken and needs to be fixed. Please feel free to submit
> patches to AOSP to do so as you can trigger this easily.

Hm, maybe the driver isn't broken...

snd_pcm_lib_preallocate_pages_for_all() is called with
SNDRV_DMA_TYPE_DEV set, so that should be fine, and the only other
buffer I can see allocate here is with a call to
snd_pcm_lib_alloc_vmalloc_buffer() which _should_ be ok, right?

I don't see any buffers coming off the stack here, unless the gadget
controller is the one creating them?

thanks,

greg k-h

2019-11-04 09:06:51

by Takashi Iwai

[permalink] [raw]
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*

On Mon, 04 Nov 2019 09:49:43 +0100,
Greg Kroah-Hartman wrote:
>
> On 11/4/19 9:32 AM, Greg Kroah-Hartman wrote:
> > On 11/4/19 9:27 AM, youling 257 wrote:
> > > This driver
> >
> https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/gadget/function/f_audio_source.c
>
> >
> > >
> >
> > The driver is broken and needs to be fixed. Please feel free to submit
> > patches to AOSP to do so as you can trigger this easily.
>
> Hm, maybe the driver isn't broken...

It is :)

> snd_pcm_lib_preallocate_pages_for_all() is called with
> SNDRV_DMA_TYPE_DEV set, so that should be fine,

That's the cause. It passes NULL to the device object, which is
incorrect in anyway. I guess this used to work casually just because
x86 accepts the NULL device object as if an ISA device.

> and the only other
> buffer I can see allocate here is with a call to
> snd_pcm_lib_alloc_vmalloc_buffer() which _should_ be ok, right?

If the driver allocates the buffer via vmalloc (and the snd_* helper),
it shouldn't do preallocate in that way. That is, the correct fix
would be simply to drop snd_pcm_lib_preallocate_pages_for_all() call
from the driver code.

Also, a quick skimming showed that the driver needs to set
snd_pcm_lib_get_vmalloc_page to the snd_pcm_ops.page ops.

> I don't see any buffers coming off the stack here, unless the gadget
> controller is the one creating them?

That's the code before actually allocating the buffer itself. It
checks the availability of mmap support on the architecture, and the
helper code assumed a proper device object passed there via the
preallocation helper.


thanks,

Takashi