+Zach Riggle riggle@google.com
Hello,
Eventually, your security expert can freely join to our conversation here.
Thanks. The biggest unanswered question is whether Android security will allow the file descriptor to be passed to an app. So I have added our security person, Zach Riggle, who originally requested the anon_inode:dmabuf FD. If Zach is happy then I am happy.
We will need two file descriptors, one with full permissions for the HAL, and one with only PCM access for the app to use. It seems we are considering two options for the app's FD: 1) provide an anon_inode:dmabuf that never has CONTROL permissions, which seems safe, but requires more changes to the driver and is a bit of a hack 2) provide an anon_inode:snd-pcm that has CONTROL permissions turned off, which seems seems less safe, but requires fewer changes and fits with the design
Which one is actually better for security?
Here is an earlier argument for snd-pcm from Jaroslav:
My point is that the dma-buf -> sound pcm buffer maping interface is
more complex, error prone and the code review/audit expensive than reusing the current code without any functionality or security benefits.
We can nicely restrict the file operations to allow to mmap only the pcm
sound buffer and eventually, if we are too much paranoid (to bypass the the bitmap like permission checking as I suggested), we can create a special case for the Android usage to return the file descriptor with very restricted 'struct file_operations' with just the mmap and release callbacks. We can also change the name for this file descriptor to distinguish it from the "anon_inode:snd-pcm" (for example "anon_inode:snd-pcm-paranoid") to let SELinux do it's work properly.
The mmap implementation for the sound driver is few lines of the code
(for the standard devices - very easy to review), so we cannot speak about security holes at all. If there is a problem with the kernel page allocation/management in the sound driver, there will be problem with dmabuf -> sound pcm buffer mapping, too (plus other problems caused by the concurrent access to the buffer which is managed /alloc/free/ by the sound driver - not dma-buf).
Also note that my emails bounce off the alsa-project mail list.
Phil Burk
On Thu, Jan 31, 2019 at 5:30 AM Jaroslav Kysela perex@perex.cz wrote:
Dne 31.1.2019 v 13:26 Mark Brown napsal(a):
On Thu, Jan 31, 2019 at 09:08:04AM +0100, Takashi Iwai wrote:
Mark Brown wrote:
anything O_APPEND based. My understanding is that this is
fundamentally
a risk mitigation thing - by not having any of the sound kernel interfaces available to the applications affected there's no
possibility
that any problems in the sound code can cause security issues.
The patch 2 implements exactly that kind of access restriction, so that the passed fd won't do anything else than wished.
Yeah.
If we want to be super-conservative, the implementation could be even simpler -- instead of filtering, we may pass a minimum fd ops that contains only mmap and release for the anon-dup fd...
I think that'd definitely help address the concerns.
A possible implementation:
http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=ca15bc69a984cc...
Jaroslav-- Jaroslav Kysela perex@perex.cz Linux Sound Maintainer; ALSA Project; Red Hat, Inc.