Re: [alsa-devel] [RFC PATCH] ALSA: core: Add DMA share buffer support

[Takashi Iwai]

On Mon, 28 Jan 2019 14:31:23 +0100,
Jaroslav Kysela wrote:
> 
> Dne 25.1.2019 v 19:25 Mark Brown napsal(a):
> > On Fri, Jan 25, 2019 at 02:19:22PM +0100, Takashi Iwai wrote:
> >> Leo Yan wrote:
> > 
> >>> If we directly use the device node /dev/snd/ as file descriptor, even
> >>> though we specify flag O_EXCL when open it, but it still is not an
> >>> anon inode file descriptor.  Thus this is not safe enough and will be
> >>> blocked by SELinux.  On the other hand, this patch wants to use
> >>> dma-buf framework to provide file descriptor for the audio buffer, and
> >>> this audio buffer can be one of mutiple audio buffers in the system
> >>> and it can be shared to any audio client program.
> > 
> >> Hrm, it sounds like a workaround just to bypass SELinux check...
> > 
> >> The sound server can open another PCM stream with O_APPEND, and pass
> >> that fd to the client, too?
> > 
> > So long as we can teach SELinux that they're safe to export, yeah.
> 
> It seems that SELinux works with the file, so the SELinux will block the
> fd pass, because the file descriptor (through standard dup()) continues
> to use the /dev/snd inode.
>
> I would propose to implement a dup ioctl to return a new
> anon_inode:snd-pcm file descriptor (see bellow).

I like the idea.  This would work around the messy issues gracefully,
and more importantly, it's easier to maintain for us.

And the restriction of ioctls for anon dup should be fairly easy to
implement on top of this.


Thanks!

Takashi

> If we agree on this, I can propose the full solution.
> 
> --
> Subject: [PATCH] ALSA: pcm: implement the anonymous dup (inode file
>  descriptor)
> 
> 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 implementation is just a concept for comments - it does not contain
> the additional restriction control.
> 
> TODO: The clients might be restricted to disallow a set of
> controls (ioctls) for the audio stream.
> 
> 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)
> ---
>  include/uapi/sound/asound.h |  1 +
>  sound/core/pcm_compat.c     |  1 +
>  sound/core/pcm_native.c     | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 404d4b9ffe76..ad821a52f970 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -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   _IOW('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/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 26afb6b0889a..a21bb482b4b0 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"
> 
> @@ -2836,6 +2838,42 @@ 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;
> +	int res;
> +	int dup_mode;
> +	int flags;
> +	struct file *nfile;
> +	struct snd_pcm_substream *rsubstream;
> +
> +	if (get_user(dup_mode, (int __user *)arg))
> +		return -EFAULT;
> +	if (dup_mode != 0)
> +		return -ENOSYS;
> +	flags = file->f_flags & (O_RDWR|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);
> +	}
> +	fd_install(fd, nfile);
> +	res = snd_pcm_open_substream(substream->pcm, substream->number,
> +				     nfile, &rsubstream);
> +	if (res < 0) {
> +		ksys_close(fd);
> +		return res;
> +	}
> +	put_user(fd, (int __user *)arg);
> +	return 0;
> +}
> +
>  static int snd_pcm_common_ioctl(struct file *file,
>  				 struct snd_pcm_substream *substream,
>  				 unsigned int cmd, void __user *arg)
> @@ -2864,6 +2902,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:
> -- 
> 
> 						Jaroslav
> 
> -- 
> Jaroslav Kysela <perex@xxxxxxxx>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
>