mbox series

[0/2,v4] ALSA: pcm: anonymous dup implementation

Message ID 20190204093910.23878-1-perex@perex.cz (mailing list archive)
Headers show
Series ALSA: pcm: anonymous dup implementation | expand

Message

Jaroslav Kysela Feb. 4, 2019, 9:39 a.m. UTC
This patchset contains the anonymous dup implementation with permissions
checking for the ALSA's PCM interface in kernel to enable the restricted
DMA sound buffer sharing for the restricted tasks.

The code was tested through qemu and it seems to be pretty stable.

The initial tinyalsa implementation can be found here:

  https://github.com/perexg/tinyalsa/commits/anondup

v2 of the patches:

- change clone parameter to subdevice number for the pcm attach
- change SNDRV_PCM_PERM_MAX to SNDRV_PCM_PERM_MASK
- the tinyalsa implementation was a little updated (restructured)

v3 of the patches:

- group integer declarations in snd_pcm_anonymous_dup()
- replaced substream->pcm with pcm in snd_pcm_anonymous_dup()
- added SNDRV_PCM_PERM_RW check for read/write/readv/writev syscalls

v4 of the patches:

- more simple restriction control (only two modes - full/buffer)
- the tinyalsa implementation follows this change

Cc: Phil Burk <philburk@google.com>
Cc: Zach Riggle <riggle@google.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Baolin Wang <baolin.wang@linaro.org>

Jaroslav Kysela (2):
  ALSA: pcm: implement the anonymous dup (inode file descriptor)
  ALSA: pcm: implement the mmap buffer mode for the anonymous dup

 include/sound/pcm.h         | 10 +++--
 include/uapi/sound/asound.h |  6 ++-
 sound/core/oss/pcm_oss.c    |  2 +-
 sound/core/pcm.c            | 13 +++---
 sound/core/pcm_compat.c     |  1 +
 sound/core/pcm_native.c     | 97 ++++++++++++++++++++++++++++++++++++++++++---
 6 files changed, 110 insertions(+), 19 deletions(-)

Comments

Phil Burk Feb. 7, 2019, 5:41 p.m. UTC | #1
Thanks for these patches for tinyalsa.  Looks great!

I made a few minor comments on the code layout.
https://github.com/perexg/tinyalsa/commit/f62b953f37693e8426ee4c20e53baae757ba1026
Can you see them?

Phil Burk

On Mon, Feb 4, 2019 at 1:39 AM Jaroslav Kysela <perex@perex.cz> wrote:

> This patchset contains the anonymous dup implementation with permissions
> checking for the ALSA's PCM interface in kernel to enable the restricted
> DMA sound buffer sharing for the restricted tasks.
>
> The code was tested through qemu and it seems to be pretty stable.
>
> The initial tinyalsa implementation can be found here:
>
>   https://github.com/perexg/tinyalsa/commits/anondup
>
> v2 of the patches:
>
> - change clone parameter to subdevice number for the pcm attach
> - change SNDRV_PCM_PERM_MAX to SNDRV_PCM_PERM_MASK
> - the tinyalsa implementation was a little updated (restructured)
>
> v3 of the patches:
>
> - group integer declarations in snd_pcm_anonymous_dup()
> - replaced substream->pcm with pcm in snd_pcm_anonymous_dup()
> - added SNDRV_PCM_PERM_RW check for read/write/readv/writev syscalls
>
> v4 of the patches:
>
> - more simple restriction control (only two modes - full/buffer)
> - the tinyalsa implementation follows this change
>
> Cc: Phil Burk <philburk@google.com>
> Cc: Zach Riggle <riggle@google.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Baolin Wang <baolin.wang@linaro.org>
>
> Jaroslav Kysela (2):
>   ALSA: pcm: implement the anonymous dup (inode file descriptor)
>   ALSA: pcm: implement the mmap buffer mode for the anonymous dup
>
>  include/sound/pcm.h         | 10 +++--
>  include/uapi/sound/asound.h |  6 ++-
>  sound/core/oss/pcm_oss.c    |  2 +-
>  sound/core/pcm.c            | 13 +++---
>  sound/core/pcm_compat.c     |  1 +
>  sound/core/pcm_native.c     | 97
> ++++++++++++++++++++++++++++++++++++++++++---
>  6 files changed, 110 insertions(+), 19 deletions(-)
>
> --
> 2.13.6
>
Jaroslav Kysela Feb. 7, 2019, 5:53 p.m. UTC | #2
Dne 7.2.2019 v 18:41 Phil Burk napsal(a):
> Thanks for these patches for tinyalsa.  Looks great!
> 
> I made a few minor comments on the code layout. 
> https://github.com/perexg/tinyalsa/commit/f62b953f37693e8426ee4c20e53baae757ba1026
> Can you see them?

Yes, I updated the anondup branch with the suggested changes (except the
else comment which I don't understand - it's just copy-n-paste of the
origin).

					Thanks,
						Jaroslav
Phil Burk Feb. 8, 2019, 3:57 p.m. UTC | #3
> (except the else comment which I don't understand - it's just
copy-n-paste of the origin).

OK. No problem. I see that was already there.

Thanks for fixing the indentation change.

Phil Burk


On Thu, Feb 7, 2019 at 9:53 AM Jaroslav Kysela <perex@perex.cz> wrote:

> Dne 7.2.2019 v 18:41 Phil Burk napsal(a):
> > Thanks for these patches for tinyalsa.  Looks great!
> >
> > I made a few minor comments on the code layout.
> >
> https://github.com/perexg/tinyalsa/commit/f62b953f37693e8426ee4c20e53baae757ba1026
> > Can you see them?
>
> Yes, I updated the anondup branch with the suggested changes (except the
> else comment which I don't understand - it's just copy-n-paste of the
> origin).
>
>                                         Thanks,
>                                                 Jaroslav
>
> --
> Jaroslav Kysela <perex@perex.cz>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
>
Mark Brown March 26, 2019, 2:09 p.m. UTC | #4
On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
> This patchset contains the anonymous dup implementation with permissions
> checking for the ALSA's PCM interface in kernel to enable the restricted
> DMA sound buffer sharing for the restricted tasks.

Is there any news on merging this during the current development cycle,
or anything else needed to move this forwards?
Takashi Iwai March 26, 2019, 2:13 p.m. UTC | #5
On Tue, 26 Mar 2019 15:09:28 +0100,
Mark Brown wrote:
> 
> On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
> > This patchset contains the anonymous dup implementation with permissions
> > checking for the ALSA's PCM interface in kernel to enable the restricted
> > DMA sound buffer sharing for the restricted tasks.
> 
> Is there any news on merging this during the current development cycle,
> or anything else needed to move this forwards?

I've seen no reaction whether the patch really tested, worked or
helped in the real scenario...


Takashi
Phil Burk March 26, 2019, 2:27 p.m. UTC | #6
Hello,

Thanks for keeping this moving forward.

Any suggestions for testing this? We can try to test it here in Android. It
may be tricky because your patches may conflict with some proprietary
Qualcomm changes in the kernel we are currently using.

Can I get a summary list of the patches required for kernel and tinyalsa?

Thanks,
Phil Burk


On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de> wrote:

> On Tue, 26 Mar 2019 15:09:28 +0100,
> Mark Brown wrote:
> >
> > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
> > > This patchset contains the anonymous dup implementation with
> permissions
> > > checking for the ALSA's PCM interface in kernel to enable the
> restricted
> > > DMA sound buffer sharing for the restricted tasks.
> >
> > Is there any news on merging this during the current development cycle,
> > or anything else needed to move this forwards?
>
> I've seen no reaction whether the patch really tested, worked or
> helped in the real scenario...
>
>
> Takashi
>
Jaroslav Kysela March 26, 2019, 2:41 p.m. UTC | #7
Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
> Hello,
> 
> Thanks for keeping this moving forward.
> 
> Any suggestions for testing this? We can try to test it here in Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using.
> 
> Can I get a summary list of the patches required for kernel and tinyalsa?

The v6 is the last version, the full information is in the cover letter:

https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.html
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.html
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.html
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.html

						Jaroslav

> 
> Thanks,
> Phil Burk
> 
> 
> On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de <mailto:tiwai@suse.de>> wrote:
> 
>     On Tue, 26 Mar 2019 15:09:28 +0100,
>     Mark Brown wrote:
>     >
>     > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
>     > > This patchset contains the anonymous dup implementation with permissions
>     > > checking for the ALSA's PCM interface in kernel to enable the restricted
>     > > DMA sound buffer sharing for the restricted tasks.
>     >
>     > Is there any news on merging this during the current development cycle,
>     > or anything else needed to move this forwards?
> 
>     I've seen no reaction whether the patch really tested, worked or
>     helped in the real scenario...
> 
> 
>     Takashi
>
Takashi Iwai April 23, 2019, 6:08 p.m. UTC | #8
On Tue, 26 Mar 2019 15:41:11 +0100,
Jaroslav Kysela wrote:
> 
> Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
> > Hello,
> > 
> > Thanks for keeping this moving forward.
> > 
> > Any suggestions for testing this? We can try to test it here in Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using.
> > 
> > Can I get a summary list of the patches required for kernel and tinyalsa?
> 
> The v6 is the last version, the full information is in the cover letter:
> 
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.html
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.html
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.html
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.html

Is there any test result from the practical use cases?

Basically now is the last chance for merging such an intrusive change
for 5.2 kernel.  If we miss it, it'll be postponed at least for 5.3,
i.e. more three months.


thanks,

Takashi


> 
> 						Jaroslav
> 
> > 
> > Thanks,
> > Phil Burk
> > 
> > 
> > On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de <mailto:tiwai@suse.de>> wrote:
> > 
> >     On Tue, 26 Mar 2019 15:09:28 +0100,
> >     Mark Brown wrote:
> >     >
> >     > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
> >     > > This patchset contains the anonymous dup implementation with permissions
> >     > > checking for the ALSA's PCM interface in kernel to enable the restricted
> >     > > DMA sound buffer sharing for the restricted tasks.
> >     >
> >     > Is there any news on merging this during the current development cycle,
> >     > or anything else needed to move this forwards?
> > 
> >     I've seen no reaction whether the patch really tested, worked or
> >     helped in the real scenario...
> > 
> > 
> >     Takashi
> > 
> 
> 
> -- 
> Jaroslav Kysela <perex@perex.cz>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
>
Phil Burk April 23, 2019, 8:11 p.m. UTC | #9
Hello Takashi,

Sorry for the late reply. I got pulled off on some other projects.

We will try to test this in-house but we will need Qualcomm's help.
I will also try to get some of our SOC partners to help with testing.

> If we miss it, it'll be postponed at least for 5.3,
> i.e. more three months.

I think it will be difficult to complete the testing before that 5.2
deadline.
We should probably shoot for 5.3. If someone needs it sooner that they will
have the patches.

Thanks again for all this work.

I assume the four emails below have all the latest patches needed for rev 6.

Thanks,
Phil Burk


On Tue, Apr 23, 2019 at 11:08 AM Takashi Iwai <tiwai@suse.de> wrote:

> On Tue, 26 Mar 2019 15:41:11 +0100,
> Jaroslav Kysela wrote:
> >
> > Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
> > > Hello,
> > >
> > > Thanks for keeping this moving forward.
> > >
> > > Any suggestions for testing this? We can try to test it here in
> Android. It may be tricky because your patches may conflict with some
> proprietary Qualcomm changes in the kernel we are currently using.
> > >
> > > Can I get a summary list of the patches required for kernel and
> tinyalsa?
> >
> > The v6 is the last version, the full information is in the cover letter:
> >
> >
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.html
> >
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.html
> >
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.html
> >
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.html
>
> Is there any test result from the practical use cases?
>
> Basically now is the last chance for merging such an intrusive change
> for 5.2 kernel.  If we miss it, it'll be postponed at least for 5.3,
> i.e. more three months.
>
>
> thanks,
>
> Takashi
>
>
> >
> >                                               Jaroslav
> >
> > >
> > > Thanks,
> > > Phil Burk
> > >
> > >
> > > On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de <mailto:
> tiwai@suse.de>> wrote:
> > >
> > >     On Tue, 26 Mar 2019 15:09:28 +0100,
> > >     Mark Brown wrote:
> > >     >
> > >     > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
> > >     > > This patchset contains the anonymous dup implementation with
> permissions
> > >     > > checking for the ALSA's PCM interface in kernel to enable the
> restricted
> > >     > > DMA sound buffer sharing for the restricted tasks.
> > >     >
> > >     > Is there any news on merging this during the current development
> cycle,
> > >     > or anything else needed to move this forwards?
> > >
> > >     I've seen no reaction whether the patch really tested, worked or
> > >     helped in the real scenario...
> > >
> > >
> > >     Takashi
> > >
> >
> >
> > --
> > Jaroslav Kysela <perex@perex.cz>
> > Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
> >
>
Takashi Iwai April 23, 2019, 8:25 p.m. UTC | #10
On Tue, 23 Apr 2019 22:11:50 +0200,
Phil Burk wrote:
> 
> Hello Takashi,
> 
> Sorry for the late reply. I got pulled off on some other projects.
> 
> We will try to test this in-house but we will need Qualcomm's help.
> I will also try to get some of our SOC partners to help with testing.

OK.

> > If we miss it, it'll be postponed at least for 5.3,
> > i.e. more three months.
> 
> I think it will be difficult to complete the testing before that 5.2 deadline.
> We should probably shoot for 5.3. If someone needs it sooner that they will
> have the patches.

Fair enough, let's postpone the merge, then.

> Thanks again for all this work.
> 
> I assume the four emails below have all the latest patches needed for rev 6.

I believe yes.

Thanks for the quick update!


Takashi


> Thanks,
> Phil Burk
> 
> On Tue, Apr 23, 2019 at 11:08 AM Takashi Iwai <tiwai@suse.de> wrote:
> 
>     On Tue, 26 Mar 2019 15:41:11 +0100,
>     Jaroslav Kysela wrote:
>     >
>     > Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
>     > > Hello,
>     > >
>     > > Thanks for keeping this moving forward.
>     > >
>     > > Any suggestions for testing this? We can try to test it here in
>     Android. It may be tricky because your patches may conflict with some
>     proprietary Qualcomm changes in the kernel we are currently using.
>     > >
>     > > Can I get a summary list of the patches required for kernel and
>     tinyalsa?
>     >
>     > The v6 is the last version, the full information is in the cover letter:
>     >
>     > 
>     https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.html
>     > 
>     https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.html
>     > 
>     https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.html
>     > 
>     https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.html
>    
>     Is there any test result from the practical use cases?
>    
>     Basically now is the last chance for merging such an intrusive change
>     for 5.2 kernel.  If we miss it, it'll be postponed at least for 5.3,
>     i.e. more three months.
> 
>     thanks,
>    
>     Takashi
> 
>     >
>     >                                               Jaroslav
>     >
>     > >
>     > > Thanks,
>     > > Phil Burk
>     > >
>     > >
>     > > On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de <mailto:
>     tiwai@suse.de>> wrote:
>     > >
>     > >     On Tue, 26 Mar 2019 15:09:28 +0100,
>     > >     Mark Brown wrote:
>     > >     >
>     > >     > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
>     > >     > > This patchset contains the anonymous dup implementation with
>     permissions
>     > >     > > checking for the ALSA's PCM interface in kernel to enable the
>     restricted
>     > >     > > DMA sound buffer sharing for the restricted tasks.
>     > >     >
>     > >     > Is there any news on merging this during the current development
>     cycle,
>     > >     > or anything else needed to move this forwards?
>     > >
>     > >     I've seen no reaction whether the patch really tested, worked or
>     > >     helped in the real scenario...
>     > >
>     > >
>     > >     Takashi
>     > >
>     >
>     >
>     > --
>     > Jaroslav Kysela <perex@perex.cz>
>     > Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
>     >
> 
>
Mark Brown June 8, 2020, 1:45 p.m. UTC | #11
On Tue, Apr 23, 2019 at 01:11:50PM -0700, Phil Burk wrote:
> Hello Takashi,
> 
> Sorry for the late reply. I got pulled off on some other projects.
> 
> We will try to test this in-house but we will need Qualcomm's help.
> I will also try to get some of our SOC partners to help with testing.

Did anything ever happen with this testing?  These anonymous mmap
patches never got merged.
Phil Burk June 8, 2020, 5:49 p.m. UTC | #12
Hello Mark,

Thank you for keeping this moving forward.

We sent the patches out to several SOC vendors for Android last year.
They thanked us and said they would send feedback but never did.
I pinged them again.

If we cannot get the changes tested by partners then I will try to get them
tested internally.

For reference, this is being tracked internally at b/119712034

Thanks,
Phil Burk



On Mon, Jun 8, 2020 at 6:45 AM Mark Brown <broonie@kernel.org> wrote:

> On Tue, Apr 23, 2019 at 01:11:50PM -0700, Phil Burk wrote:
> > Hello Takashi,
> >
> > Sorry for the late reply. I got pulled off on some other projects.
> >
> > We will try to test this in-house but we will need Qualcomm's help.
> > I will also try to get some of our SOC partners to help with testing.
>
> Did anything ever happen with this testing?  These anonymous mmap
> patches never got merged.
>
Phil Burk June 10, 2020, 11:10 p.m. UTC | #13
Hello Mark,

I wrote to Peter Huang at UniSOC

>  Did you ever get a chance to try the Linaro patches? Did they work for
you?



Peter wrote:

>  we have tried these patches, and they works. [snip]

>  And Baolin also suggests that his patch is too complicated and difficult
to maintain, and a better solution is list as below, this new patch I have
not tried.



[1]:

https://lore.kernel.org/patchwork/patch/1033206/



[2]:

https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144925.html

https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144924.html


It is hard for me to extract the relevant patches from those emails.


I would love to finish this project but I am not sure how. It seems we need
to:


1) Evaluate the patches that Baolin suggests as a simpler alternative.

2) Test them in an Android kernel with AAudio MMAP.


If you can provide a clear description of the latest set of patches then
maybe I can work with someone in-house to test this.


I am open to suggestions.


Phil Burk



On Mon, Jun 8, 2020 at 10:49 AM Phil Burk <philburk@google.com> wrote:

> Hello Mark,
>
> Thank you for keeping this moving forward.
>
> We sent the patches out to several SOC vendors for Android last year.
> They thanked us and said they would send feedback but never did.
> I pinged them again.
>
> If we cannot get the changes tested by partners then I will try to get
> them tested internally.
>
> For reference, this is being tracked internally at b/119712034
> <https://buganizer.corp.google.com/119712034>
>
> Thanks,
> Phil Burk
>
>
>
> On Mon, Jun 8, 2020 at 6:45 AM Mark Brown <broonie@kernel.org> wrote:
>
>> On Tue, Apr 23, 2019 at 01:11:50PM -0700, Phil Burk wrote:
>> > Hello Takashi,
>> >
>> > Sorry for the late reply. I got pulled off on some other projects.
>> >
>> > We will try to test this in-house but we will need Qualcomm's help.
>> > I will also try to get some of our SOC partners to help with testing.
>>
>> Did anything ever happen with this testing?  These anonymous mmap
>> patches never got merged.
>>
>
Mark Brown June 12, 2020, 4:59 p.m. UTC | #14
On Wed, Jun 10, 2020 at 04:10:15PM -0700, Phil Burk wrote:

> https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144925.html
> https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144924.html

> It is hard for me to extract the relevant patches from those emails.

I've attached them in mbox format (b4 is a great tool for extracting
stuff from the archives on lore.kernel.org BTW:

   https://pypi.org/project/b4/

).  They seem to apply cleanly with git am -3 for me so should be easy
enough to apply to your tree hopefully.

> I would love to finish this project but I am not sure how. It seems we need
> to:

> 1) Evaluate the patches that Baolin suggests as a simpler alternative.

> 2) Test them in an Android kernel with AAudio MMAP.

> If you can provide a clear description of the latest set of patches then
> maybe I can work with someone in-house to test this.

I agree that that's the best approach - hopefully the mbox helps with
getting the patches onto a relevant tree, let me know if you still have
trouble.

For reference it's the patches from Jaroslav in this thread we're on
here, which can also be found here:

   https://lore.kernel.org/alsa-devel/20190204093910.23878-1-perex@perex.cz/
From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jaroslav Kysela <perex@perex.cz>
Subject: [PATCH 1/2] ALSA: pcm: implement the anonymous dup
	(inode file descriptor)
Date: Mon,  4 Feb 2019 10:39:09 +0100
Message-ID: <20190204093910.23878-2-perex@perex.cz>
References: <20190204093910.23878-1-perex@perex.cz>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <alsa-devel-bounces@alsa-project.org>
Received: from mail1.perex.cz (mail1.perex.cz [77.48.224.245])
 by alsa0.perex.cz (Postfix) with ESMTP id 2873D267549
 for <alsa-devel@alsa-project.org>; Mon,  4 Feb 2019 10:39:54 +0100 (CET)
In-Reply-To: <20190204093910.23878-1-perex@perex.cz>
List-Unsubscribe: <http://mailman.alsa-project.org/mailman/options/alsa-devel>,
 <mailto:alsa-devel-request@alsa-project.org?subject=unsubscribe>
List-Archive: <http://mailman.alsa-project.org/pipermail/alsa-devel/>
List-Post: <mailto:alsa-devel@alsa-project.org>
List-Help: <mailto:alsa-devel-request@alsa-project.org?subject=help>
List-Subscribe: <http://mailman.alsa-project.org/mailman/listinfo/alsa-devel>,
 <mailto:alsa-devel-request@alsa-project.org?subject=subscribe>
Errors-To: alsa-devel-bounces@alsa-project.org
Sender: alsa-devel-bounces@alsa-project.org
To: ALSA development <alsa-devel@alsa-project.org>
Cc: Baolin Wang <baolin.wang@linaro.org>, Takashi Iwai <tiwai@suse.de>, Phil Burk <philburk@google.com>, Zach Riggle <riggle@google.com>, Mark Brown <broonie@kernel.org>, Leo Yan <leo.yan@linaro.org>
List-Id: alsa-devel@alsa-project.org
Archived-At: <https://lore.kernel.org/alsa-devel/20190204093910.23878-2-perex@perex.cz/>
List-Archive: <https://lore.kernel.org/alsa-devel/>

This patch implements new SNDRV_PCM_IOCTL_ANONYMOUS_DUP ioctl which
returns the new duplicated anonymous inode file descriptor
(anon_inode:snd-pcm) which can be passed to the restricted clients.

This patch is meant to be the alternative for the dma-buf interface. Both
implementation have some pros and cons:

anon_inode:dmabuf

- a bit standard export API for the DMA buffers
- fencing for the concurrent access [1]
- driver/kernel interface for the DMA buffer [1]
- multiple attach/detach scheme [1]

[1] the real usage for the sound PCM is unknown at the moment for this feature

anon_inode:snd-pcm

- simple (no problem with ref-counting, non-standard mmap implementation etc.)
- allow to use more sound interfaces for the file descriptor like status ioctls
- more fine grained security policies (another anon_inode name unshared with
  other drivers)

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h         |  8 +++---
 include/uapi/sound/asound.h |  3 +-
 sound/core/oss/pcm_oss.c    |  2 +-
 sound/core/pcm.c            | 13 ++++-----
 sound/core/pcm_compat.c     |  1 +
 sound/core/pcm_native.c     | 67 +++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 75 insertions(+), 19 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index ca20f80f8976..b79ffaa0241d 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -579,11 +579,11 @@ static inline int snd_pcm_suspend_all(struct snd_pcm *pcm)
 }
 #endif
 int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg);
-int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, struct file *file,
-			   struct snd_pcm_substream **rsubstream);
+int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, int subdevice,
+                           struct file *file, struct snd_pcm_substream **rsubstream);
 void snd_pcm_release_substream(struct snd_pcm_substream *substream);
-int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, struct file *file,
-			     struct snd_pcm_substream **rsubstream);
+int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, int subdevice,
+			     struct file *file, struct snd_pcm_substream **rsubstream);
 void snd_pcm_detach_substream(struct snd_pcm_substream *substream);
 int snd_pcm_mmap_data(struct snd_pcm_substream *substream, struct file *file, struct vm_area_struct *area);
 
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 404d4b9ffe76..ebc17d5a3490 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -153,7 +153,7 @@ struct snd_hwdep_dsp_image {
  *                                                                           *
  *****************************************************************************/
 
-#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 14)
+#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 15)
 
 typedef unsigned long snd_pcm_uframes_t;
 typedef signed long snd_pcm_sframes_t;
@@ -576,6 +576,7 @@ enum {
 #define SNDRV_PCM_IOCTL_TSTAMP		_IOW('A', 0x02, int)
 #define SNDRV_PCM_IOCTL_TTSTAMP		_IOW('A', 0x03, int)
 #define SNDRV_PCM_IOCTL_USER_PVERSION	_IOW('A', 0x04, int)
+#define SNDRV_PCM_IOCTL_ANONYMOUS_DUP   _IOWR('A', 0x05, int)
 #define SNDRV_PCM_IOCTL_HW_REFINE	_IOWR('A', 0x10, struct snd_pcm_hw_params)
 #define SNDRV_PCM_IOCTL_HW_PARAMS	_IOWR('A', 0x11, struct snd_pcm_hw_params)
 #define SNDRV_PCM_IOCTL_HW_FREE		_IO('A', 0x12)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index d5b0d7ba83c4..2ed609b65c45 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2420,7 +2420,7 @@ static int snd_pcm_oss_open_file(struct file *file,
 			if (! (f_mode & FMODE_READ))
 				continue;
 		}
-		err = snd_pcm_open_substream(pcm, idx, file, &substream);
+		err = snd_pcm_open_substream(pcm, idx, -1, file, &substream);
 		if (err < 0) {
 			snd_pcm_oss_release_file(pcm_oss_file);
 			return err;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 4f45b3000347..af6f7fc3687b 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -918,15 +918,14 @@ static int snd_pcm_dev_free(struct snd_device *device)
 	return snd_pcm_free(pcm);
 }
 
-int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
-			     struct file *file,
+int snd_pcm_attach_substream(struct snd_pcm *pcm,
+			     int stream, int subdevice, struct file *file,
 			     struct snd_pcm_substream **rsubstream)
 {
 	struct snd_pcm_str * pstr;
 	struct snd_pcm_substream *substream;
 	struct snd_pcm_runtime *runtime;
 	struct snd_card *card;
-	int prefer_subdevice;
 	size_t size;
 
 	if (snd_BUG_ON(!pcm || !rsubstream))
@@ -940,7 +939,6 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 		return -ENODEV;
 
 	card = pcm->card;
-	prefer_subdevice = snd_ctl_get_preferred_subdevice(card, SND_CTL_SUBDEV_PCM);
 
 	if (pcm->info_flags & SNDRV_PCM_INFO_HALF_DUPLEX) {
 		int opposite = !stream;
@@ -953,14 +951,14 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 	}
 
 	if (file->f_flags & O_APPEND) {
-		if (prefer_subdevice < 0) {
+		if (subdevice < 0) {
 			if (pstr->substream_count > 1)
 				return -EINVAL; /* must be unique */
 			substream = pstr->substream;
 		} else {
 			for (substream = pstr->substream; substream;
 			     substream = substream->next)
-				if (substream->number == prefer_subdevice)
+				if (substream->number == subdevice)
 					break;
 		}
 		if (! substream)
@@ -974,8 +972,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 
 	for (substream = pstr->substream; substream; substream = substream->next) {
 		if (!SUBSTREAM_BUSY(substream) &&
-		    (prefer_subdevice == -1 ||
-		     substream->number == prefer_subdevice))
+		    (subdevice == -1 || substream->number == subdevice))
 			break;
 	}
 	if (substream == NULL)
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index 946ab080ac00..22446cd574ee 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -675,6 +675,7 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
 	case SNDRV_PCM_IOCTL_TSTAMP:
 	case SNDRV_PCM_IOCTL_TTSTAMP:
 	case SNDRV_PCM_IOCTL_USER_PVERSION:
+	case SNDRV_PCM_IOCTL_ANONYMOUS_DUP:
 	case SNDRV_PCM_IOCTL_HWSYNC:
 	case SNDRV_PCM_IOCTL_PREPARE:
 	case SNDRV_PCM_IOCTL_RESET:
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 0bc4aa0ac9cf..bb14658e4482 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -37,6 +37,8 @@
 #include <sound/minors.h>
 #include <linux/uio.h>
 #include <linux/delay.h>
+#include <linux/anon_inodes.h>
+#include <linux/syscalls.h>
 
 #include "pcm_local.h"
 
@@ -2437,14 +2439,17 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream)
 }
 EXPORT_SYMBOL(snd_pcm_release_substream);
 
-int snd_pcm_open_substream(struct snd_pcm *pcm, int stream,
+int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, int subdevice,
 			   struct file *file,
 			   struct snd_pcm_substream **rsubstream)
 {
 	struct snd_pcm_substream *substream;
 	int err;
 
-	err = snd_pcm_attach_substream(pcm, stream, file, &substream);
+	if (subdevice < 0 && pcm)
+		subdevice = snd_ctl_get_preferred_subdevice(pcm->card, SND_CTL_SUBDEV_PCM);
+
+	err = snd_pcm_attach_substream(pcm, stream, subdevice, file, &substream);
 	if (err < 0)
 		return err;
 	if (substream->ref_count > 1) {
@@ -2480,13 +2485,14 @@ EXPORT_SYMBOL(snd_pcm_open_substream);
 
 static int snd_pcm_open_file(struct file *file,
 			     struct snd_pcm *pcm,
-			     int stream)
+			     int stream,
+			     int subdevice)
 {
 	struct snd_pcm_file *pcm_file;
 	struct snd_pcm_substream *substream;
 	int err;
 
-	err = snd_pcm_open_substream(pcm, stream, file, &substream);
+	err = snd_pcm_open_substream(pcm, stream, subdevice, file, &substream);
 	if (err < 0)
 		return err;
 
@@ -2551,7 +2557,7 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
 	add_wait_queue(&pcm->open_wait, &wait);
 	mutex_lock(&pcm->open_mutex);
 	while (1) {
-		err = snd_pcm_open_file(file, pcm, stream);
+		err = snd_pcm_open_file(file, pcm, stream, -1);
 		if (err >= 0)
 			break;
 		if (err == -EAGAIN) {
@@ -2595,6 +2601,9 @@ static int snd_pcm_release(struct inode *inode, struct file *file)
 	struct snd_pcm_file *pcm_file;
 
 	pcm_file = file->private_data;
+	/* a problem in the anonymous dup can hit the NULL pcm_file */
+	if (pcm_file == NULL)
+		return 0;
 	substream = pcm_file->substream;
 	if (snd_BUG_ON(!substream))
 		return -ENXIO;
@@ -2878,6 +2887,52 @@ static int snd_pcm_forward_ioctl(struct snd_pcm_substream *substream,
 	return result < 0 ? result : 0;
 }
 
+static int snd_pcm_anonymous_dup(struct file *file,
+				 struct snd_pcm_substream *substream,
+				 int __user *arg)
+{
+	int fd, err, perm, flags;
+	struct file *nfile;
+	struct snd_pcm *pcm = substream->pcm;
+
+	if (get_user(perm, arg))
+		return -EFAULT;
+	if (perm < 0)
+		return -EPERM;
+	flags = file->f_flags & (O_ACCMODE | O_NONBLOCK);
+	flags |= O_APPEND | O_CLOEXEC;
+	fd = get_unused_fd_flags(flags);
+	if (fd < 0)
+		return fd;
+	nfile = anon_inode_getfile("snd-pcm", file->f_op, NULL, flags);
+	if (IS_ERR(nfile)) {
+		put_unused_fd(fd);
+		return PTR_ERR(nfile);
+	}
+	/* anon_inode_getfile() filters the O_APPEND flag out */
+	nfile->f_flags |= O_APPEND;
+	fd_install(fd, nfile);
+	if (!try_module_get(pcm->card->module)) {
+		err = -EFAULT;
+		goto __error1;
+	}
+	err = snd_card_file_add(pcm->card, nfile);
+	if (err < 0)
+		goto __error2;
+	err = snd_pcm_open_file(nfile, substream->pcm,
+				substream->stream, substream->number);
+	if (err >= 0) {
+		put_user(fd, arg);
+		return 0;
+	}
+	snd_card_file_remove(pcm->card, nfile);
+      __error2:
+	module_put(pcm->card->module);
+      __error1:
+	ksys_close(fd);
+	return err;
+}
+
 static int snd_pcm_common_ioctl(struct file *file,
 				 struct snd_pcm_substream *substream,
 				 unsigned int cmd, void __user *arg)
@@ -2906,6 +2961,8 @@ static int snd_pcm_common_ioctl(struct file *file,
 			     (unsigned int __user *)arg))
 			return -EFAULT;
 		return 0;
+	case SNDRV_PCM_IOCTL_ANONYMOUS_DUP:
+		return snd_pcm_anonymous_dup(file, substream, (int __user *)arg);
 	case SNDRV_PCM_IOCTL_HW_REFINE:
 		return snd_pcm_hw_refine_user(substream, arg);
 	case SNDRV_PCM_IOCTL_HW_PARAMS: