On Nov 9, 2018, at 2:42 PM, Daniel Colascione dancol@google.com wrote:
On Fri, Nov 9, 2018 at 2:37 PM, Andy Lutomirski luto@amacapital.net wrote:
Another, more general fix might be to prevent /proc/pid/fd/N opens from "upgrading" access modes. But that'd be a bigger ABI break.
I think we should fix that, too. I consider it a bug fix, not an ABI break, personally.
Someone, somewhere is probably relying on it though, and that means that we probably can't change it unless it's actually causing problems.
<mumble>spacebar heating</mumble>
I think it has caused problems in the past. It’s certainly extremely surprising behavior. I’d say it should be fixed and, if needed, a sysctl to unfix it might be okay.
That aside: I wonder whether a better API would be something that allows you to create a new readonly file descriptor, instead of fiddling with the writability of an existing fd.
That doesn't work, unfortunately. The ashmem API we're replacing with memfd requires file descriptor continuity. I also looked into opening a new FD and dup2(2)ing atop the old one, but this approach doesn't work in the case that the old FD has already leaked to some other context (e.g., another dup, SCM_RIGHTS). See https://developer.android.com/ndk/reference/group/memory. We can't break ASharedMemory_setProt.
Hmm. If we fix the general reopen bug, a way to drop write access from an existing struct file would do what Android needs, right? I don’t know if there are general VFS issues with that.
I also proposed that. :-) Maybe it'd work best as a special case of the perennial revoke(2) that people keep proposing. You'd be able to selectively revoke all access or just write access.
Sounds good to me, modulo possible races, but that shouldn’t be too hard to deal with.