> On Nov 9, 2018, at 2:42 PM, Daniel Colascione <dancol@xxxxxxxxxx> wrote: > > On Fri, Nov 9, 2018 at 2:37 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> 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.
