All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@google.com>
Cc: linux-xtensa@linux-xtensa.org, youling 257 <youling257@gmail.com>,
	Vladimir Murzin <vladimir.murzin@arm.com>,
	linux-parisc@vger.kernel.org, linux-sh@vger.kernel.org,
	Takashi Iwai <tiwai@suse.de>, Robin Murphy <robin.murphy@arm.com>,
	Helge Deller <deller@gmx.de>,
	x86@kernel.org, Michal Simek <monstr@monstr.eu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-m68k@lists.linux-m68k.org, linuxppc-dev@lists.ozlabs.org,
	hch@lst.de, linux-arm-kernel@lists.infradead.org
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
Date: Mon, 04 Nov 2019 09:03:34 +0000	[thread overview]
Message-ID: <s5hbltsc8t5.wl-tiwai@suse.de> (raw)
In-Reply-To: <19c3f0ca-9b21-0c5e-894d-976594f38a86@google.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@google.com>
Cc: youling 257 <youling257@gmail.com>, Takashi Iwai <tiwai@suse.de>,
	linux-xtensa@linux-xtensa.org, Michal Simek <monstr@monstr.eu>,
	Vladimir Murzin <vladimir.murzin@arm.com>,
	linux-parisc@vger.kernel.org, linux-sh@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, Helge Deller <deller@gmx.de>,
	x86@kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-m68k@lists.linux-m68k.org,
	Robin Murphy <robin.murphy@arm.com>,
	linux-arm-kernel@lists.infradead.org, hch@lst.de
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
Date: Mon, 04 Nov 2019 10:03:34 +0100	[thread overview]
Message-ID: <s5hbltsc8t5.wl-tiwai@suse.de> (raw)
In-Reply-To: <19c3f0ca-9b21-0c5e-894d-976594f38a86@google.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@google.com>
Cc: linux-xtensa@linux-xtensa.org, youling 257 <youling257@gmail.com>,
	Vladimir Murzin <vladimir.murzin@arm.com>,
	linux-parisc@vger.kernel.org, linux-sh@vger.kernel.org,
	Takashi Iwai <tiwai@suse.de>, Robin Murphy <robin.murphy@arm.com>,
	Helge Deller <deller@gmx.de>,
	x86@kernel.org, Michal Simek <monstr@monstr.eu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-m68k@lists.linux-m68k.org, linuxppc-dev@lists.ozlabs.org,
	hch@lst.de, linux-arm-kernel@lists.infradead.org
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
Date: Mon, 04 Nov 2019 10:03:34 +0100	[thread overview]
Message-ID: <s5hbltsc8t5.wl-tiwai@suse.de> (raw)
In-Reply-To: <19c3f0ca-9b21-0c5e-894d-976594f38a86@google.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@google.com>
Cc: linux-xtensa@linux-xtensa.org, youling 257 <youling257@gmail.com>,
	Vladimir Murzin <vladimir.murzin@arm.com>,
	linux-parisc@vger.kernel.org, linux-sh@vger.kernel.org,
	Takashi Iwai <tiwai@suse.de>, Robin Murphy <robin.murphy@arm.com>,
	Helge Deller <deller@gmx.de>,
	x86@kernel.org, Michal Simek <monstr@monstr.eu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-m68k@lists.linux-m68k.org, linuxppc-dev@lists.ozlabs.org,
	hch@lst.de, linux-arm-kernel@lists.infradead.org
Subject: Re: ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_*
Date: Mon, 04 Nov 2019 10:03:34 +0100	[thread overview]
Message-ID: <s5hbltsc8t5.wl-tiwai@suse.de> (raw)
In-Reply-To: <19c3f0ca-9b21-0c5e-894d-976594f38a86@google.com>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-11-04  9:03 UTC|newest]

Thread overview: 161+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04  7:06 ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_* youling 257
2019-11-04  7:06 ` youling 257
2019-11-04  7:06 ` youling 257
2019-11-04  8:02 ` Takashi Iwai
2019-11-04  8:02   ` Takashi Iwai
2019-11-04  8:02   ` Takashi Iwai
2019-11-04  8:02   ` Takashi Iwai
2019-11-04  8:27   ` youling 257
2019-11-04  8:27     ` youling 257
2019-11-04  8:27     ` youling 257
2019-11-04  8:27     ` youling 257
2019-11-04  8:32     ` Greg Kroah-Hartman
2019-11-04  8:32       ` Greg Kroah-Hartman
2019-11-04  8:32       ` Greg Kroah-Hartman
2019-11-04  8:32       ` Greg Kroah-Hartman
2019-11-04  8:49       ` Greg Kroah-Hartman
2019-11-04  8:49         ` Greg Kroah-Hartman
2019-11-04  8:49         ` Greg Kroah-Hartman
2019-11-04  8:49         ` Greg Kroah-Hartman
2019-11-04  9:03         ` Takashi Iwai [this message]
2019-11-04  9:03           ` Takashi Iwai
2019-11-04  9:03           ` Takashi Iwai
2019-11-04  9:03           ` Takashi Iwai
  -- strict thread matches above, loose matches on Subject: below --
2019-08-05  9:11 [PATCH 4/7] " Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:22 ` Takashi Iwai
2019-08-05  9:22   ` Takashi Iwai
2019-08-05  9:22   ` Takashi Iwai
2019-08-05  9:22   ` Takashi Iwai
2019-08-05  9:22   ` Takashi Iwai
2019-08-06  5:29 ` Christoph Hellwig
2019-08-06  5:29   ` Christoph Hellwig
2019-08-06  5:29   ` Christoph Hellwig
2019-08-06  5:29   ` Christoph Hellwig
2019-08-06  5:29   ` Christoph Hellwig
2019-08-06  6:00 ` Takashi Iwai
2019-08-06  6:00   ` Takashi Iwai
2019-08-06  6:00   ` Takashi Iwai
2019-08-06  6:00   ` Takashi Iwai
2019-08-06  6:00   ` Takashi Iwai
2019-08-05  9:11 remove default fallbacks in dma_map_ops v2 Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` Christoph Hellwig
2019-08-05  9:11 ` [PATCH 1/7] dma-mapping: move the dma_get_sgtable API comments from arm to common code Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11 ` [PATCH 2/7] dma-mapping: explicitly wire up ->mmap and ->get_sgtable Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11 ` [PATCH 3/7] dma-mapping: add a dma_can_mmap helper Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11 ` [PATCH 5/7] m68knommu: add a pgprot_noncached stub Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11 ` [PATCH 6/7] dma-mapping: remove ARCH_NO_COHERENT_DMA_MMAP Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11 ` [PATCH 7/7] dma-mapping: provide a better default ->get_required_mask Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-05  9:11   ` Christoph Hellwig
2019-08-08 15:59 ` remove default fallbacks in dma_map_ops v3 Christoph Hellwig
2019-08-08 15:59   ` Christoph Hellwig
2019-08-08 15:59   ` Christoph Hellwig
2019-08-08 15:59   ` Christoph Hellwig
2019-08-08 15:59   ` Christoph Hellwig
2019-08-08 15:59   ` [PATCH 1/8] dma-mapping: provide a better default ->get_required_mask Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59   ` [PATCH 2/8] dma-mapping: move the dma_get_sgtable API comments from arm to common code Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 15:59     ` Christoph Hellwig
2019-08-08 16:00   ` [PATCH 3/8] dma-mapping: explicitly wire up ->mmap and ->get_sgtable Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00   ` [PATCH 4/8] dma-mapping: add a dma_can_mmap helper Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00   ` [PATCH 5/8] ALSA: pcm: use dma_can_mmap() to check if a device supports dma_mmap_* Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-11-04  7:26     ` youling257
2019-11-04  7:26       ` youling257
2019-11-04  7:26       ` youling257
2019-08-08 16:00   ` [PATCH 6/8] arm-nommu: call dma_mmap_from_dev_coherent directly Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-09  9:35     ` Sergei Shtylyov
2019-08-09  9:35       ` Sergei Shtylyov
2019-08-09  9:35       ` Sergei Shtylyov
2019-08-09  9:35       ` Sergei Shtylyov
2019-08-09  9:35       ` Sergei Shtylyov
2019-08-08 16:00   ` [PATCH 7/8] parisc: don't set ARCH_NO_COHERENT_DMA_MMAP Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-15  7:36     ` Christoph Hellwig
2019-08-15  7:36       ` Christoph Hellwig
2019-08-15  7:36       ` Christoph Hellwig
2019-08-15  7:36       ` Christoph Hellwig
2019-08-15  7:36       ` Christoph Hellwig
2019-08-15  9:25     ` James Bottomley
2019-08-15  9:25       ` James Bottomley
2019-08-15  9:25       ` James Bottomley
2019-08-15  9:25       ` James Bottomley
2019-08-15  9:25       ` James Bottomley
2019-08-15 10:50       ` Christoph Hellwig
2019-08-15 10:50         ` Christoph Hellwig
2019-08-15 10:50         ` Christoph Hellwig
2019-08-15 10:50         ` Christoph Hellwig
2019-08-15 10:50         ` Christoph Hellwig
2019-08-19 14:25         ` Christoph Hellwig
2019-08-19 14:25           ` Christoph Hellwig
2019-08-19 14:25           ` Christoph Hellwig
2019-08-19 14:25           ` Christoph Hellwig
2019-08-19 14:25           ` Christoph Hellwig
2019-08-08 16:00   ` [PATCH 8/8] dma-mapping: remove CONFIG_ARCH_NO_COHERENT_DMA_MMAP Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-08 16:00     ` Christoph Hellwig
2019-08-09  7:21     ` Geert Uytterhoeven
2019-08-09  7:21       ` Geert Uytterhoeven
2019-08-09  7:21       ` Geert Uytterhoeven
2019-08-09  7:21       ` Geert Uytterhoeven
2019-08-09  7:21       ` Geert Uytterhoeven
2019-08-29 15:38   ` remove default fallbacks in dma_map_ops v3 Christoph Hellwig
2019-08-29 15:38     ` Christoph Hellwig
2019-08-29 15:38     ` Christoph Hellwig
2019-08-29 15:38     ` Christoph Hellwig
2019-08-29 15:38     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hbltsc8t5.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=deller@gmx.de \
    --cc=gregkh@google.com \
    --cc=hch@lst.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=monstr@monstr.eu \
    --cc=robin.murphy@arm.com \
    --cc=vladimir.murzin@arm.com \
    --cc=x86@kernel.org \
    --cc=youling257@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.