[alsa-devel] [PATCH 0/2] ALSA: pcm: implement the anonymous dup v3

[Zach Riggle 🖖]

The big concerns on our end are:

(1) Can the buffer be mremap()ed with a different *offset* into the
buffer?  This was a concern in the past and the reason for the anon_inode
stuff at all.  I believe that as long as the *size* of the mapping doesn't
change, Linux mm will gladly permit mremap() without informing the driver.
(2) Can we ensure that permissions can only ever be dropped? (new_perms =
old_perms & requested_perms) . It's probably useful to throw an error code
if new permissions are requested.
(3) It looks like the code for snd_pcm_anonymous_dup in the patchset takes
a *perm* argument from user-space and discards it.  Is this intended?
(4) I'm not familiar with the lifecycle of all of the objects, and
introducing a custom *dup* routine might cause unexpected problems (e.g.
use-after-free, double-free).  I'm not well-versed-enough in how the
driver-specific stuff is handled underneath e.g. snd_card_file_add /
snd_card_file_remove
to be sure about it.

Zach Riggle |  Android Security |  riggle at google.com |  Austin, TX 🇨🇱


On Thu, Jan 31, 2019 at 1:36 PM Phil Burk <philburk at google.com> wrote:

> Mark and Zach and I talked.
>
> Zach said that "dmabuf" is not a hard requirement. Another "anon_inode"
> would probably be OK as long as the app cannot turn on any permissions
> besides PCM access. Our security team will just need to review the changes.
>
> So I think you should proceed with the "anon_inode:snd-pcm" if you think
> that will be more secure. Thanks for proposing this.
>
> Zach has some notes in his initial review of Jaroslav's code. Zach?
>
> One thing Zach mentioned is that the API should only allow *removing*
> permissions and not adding permissions.
>
> What permissions would be set on the FD given to the app?
>
> Also Mark mentioned that the FD app would have PCM access and "close"
> permission. What flag allows close? What else is permitted under that flag?
> Or is close permission  just a generic "FD" permission unrelated to ALSA?
>
> Thanks for all your work on this. Sorry if I caused alarm. I just wanted
> to make sure we could use the solution you provide.
>
> Phil Burk
>
>
>
>
>