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);--
Jaroslav-- Jaroslav Kysela perex@perex.cz Linux Sound Maintainer; ALSA Project; Red Hat, Inc.