In dr_frontend_unix.c, reimplement drfront_access using POSIX access().

Commit log for first patchset:
---------------
In dr_frontend_unix.c, reimplement drfront_access using POSIX access().

This is simpler, avoiding use of geteuid and drfront_dir_try_writable.
---------------

[bruening, 2016-12-09 15:11:59]

On 2016/12/09 15:01:58, Edmund.Grimley.Evans wrote:
> In dr_frontend_unix.c, reimplement drfront_access using POSIX access().
> This is simpler, avoiding use of geteuid and drfront_dir_try_writable.

Only checking permission bits is not good enough: we have to check for read-only
filesytems, especially on Android.

[Edmund.Grimley.Evans, 2016-12-09 15:14:32]

On 2016/12/09 15:11:59, bruening wrote:
> On 2016/12/09 15:01:58, Edmund.Grimley.Evans wrote:
> > In dr_frontend_unix.c, reimplement drfront_access using POSIX access().
> > This is simpler, avoiding use of geteuid and drfront_dir_try_writable.
> 
> Only checking permission bits is not good enough: we have to check for
read-only
> filesytems, especially on Android.

Is that an Android-specific bug? It works for me on Linux: access() returns:

EROFS  Write permission was requested for a file on a read-only filesystem.

[bruening, 2016-12-09 15:15:25]

On 2016/12/09 15:11:59, bruening wrote:
> On 2016/12/09 15:01:58, Edmund.Grimley.Evans wrote:
> > In dr_frontend_unix.c, reimplement drfront_access using POSIX access().
> > This is simpler, avoiding use of geteuid and drfront_dir_try_writable.
> 
> Only checking permission bits is not good enough: we have to check for
read-only
> filesytems, especially on Android.

I should add that we also have to deal with SELinux, which is enabled on
Android.  Trying to do something is often the simplest solution.

Corollary: any change to these routines needs to be thoroughly tested on Android
as that is where we have had the most trouble, particularly with Dr. Memory log
directories.

[Edmund.Grimley.Evans, 2016-12-09 15:24:25]

On 2016/12/09 15:15:25, bruening wrote:
> On 2016/12/09 15:11:59, bruening wrote:
> > On 2016/12/09 15:01:58, Edmund.Grimley.Evans wrote:
> > > In dr_frontend_unix.c, reimplement drfront_access using POSIX access().
> > > This is simpler, avoiding use of geteuid and drfront_dir_try_writable.
> > 
> > Only checking permission bits is not good enough: we have to check for
> read-only
> > filesytems, especially on Android.
> 
> I should add that we also have to deal with SELinux, which is enabled on
> Android.  Trying to do something is often the simplest solution.

There's SELinux on some of the systems I use, too.

Using "access" is certainly simpler than using drfront_dir_try_writable, so the
question is: does it work?

> Corollary: any change to these routines needs to be thoroughly tested on
Android
> as that is where we have had the most trouble, particularly with Dr. Memory
log
> directories.

Do you feel like testing it?

[bruening, 2016-12-09 16:31:16]

I was also going to say that if unix uses access() too maybe we can share the
Windows code but I don't think that's going to work out.

I'm not sure why access() was not considered before for unix -- is it available
on Mac?

[bruening, 2016-12-10 20:45:57]

On 2016/12/09 15:14:32, Edmund.Grimley.Evans wrote:
> On 2016/12/09 15:11:59, bruening wrote:
> > On 2016/12/09 15:01:58, Edmund.Grimley.Evans wrote:
> > > In dr_frontend_unix.c, reimplement drfront_access using POSIX access().
> > > This is simpler, avoiding use of geteuid and drfront_dir_try_writable.
> > 
> > Only checking permission bits is not good enough: we have to check for
> read-only
> > filesytems, especially on Android.
> 
> Is that an Android-specific bug? It works for me on Linux: access() returns:
> 
> EROFS  Write permission was requested for a file on a read-only filesystem.

Really?  Surprising: I would have thought it just checks the privilege bits.  Is
this behavior something that can be relied on in the POSIX spec or sthg?  I
really don't think the POSIX 1.x Windows version does this.  We would have to
worry about Mac and Android as well.

[bruening, 2016-12-10 20:48:25]

access() seems to check the real user id, not the effective, so it is a semantic
change in the API and would require changing the docs for this function.

[Edmund.Grimley.Evans, 2016-12-10 22:02:07]

On 2016/12/10 20:48:25, bruening wrote:
> access() seems to check the real user id, not the effective, so it is a
semantic
> change in the API and would require changing the docs for this function.

There's "real" uid, "effective" uid, and "filesystem" uid, if I remember
correctly, and likewise three gids. I would hope that access() does the right
thing on Linux because it's implemented with a system call and therefore under
the control of the same people who implement the real thing, "open". It would be
good to check, though, particularly on Mac. I can't see why Android should
differ from Linux, since it's a kernel thing, but you never know...

[Edmund.Grimley.Evans, 2016-12-10 22:15:24]

By the way, the comment in drfront_dir_try_writable says not to worry about
races, but what about several DynamoRIO processes running as different users,
and what about lunatic processes with umask 0222?

$ umask 0222
$ echo > x
$ echo > x
bash: x: Permission denied
$

Probably not worth worrying about, though, really.

[bruening, 2016-12-11 02:33:59]

On 2016/12/10 22:02:07, Edmund.Grimley.Evans wrote:
> On 2016/12/10 20:48:25, bruening wrote:
> > access() seems to check the real user id, not the effective, so it is a
> semantic
> > change in the API and would require changing the docs for this function.
> 
> There's "real" uid, "effective" uid, and "filesystem" uid, if I remember
> correctly, and likewise three gids. I would hope that access() does the right
> thing on Linux because it's implemented with a system call and therefore under
> the control of the same people who implement the real thing, "open". It would
be
> good to check, though, particularly on Mac. I can't see why Android should
> differ from Linux, since it's a kernel thing, but you never know...

"man access" says "The check is done using the calling process's real UID and
GID, rather than the effective IDs".

[Edmund.Grimley.Evans, 2016-12-11 07:58:30]

On 2016/12/11 02:33:59, bruening wrote:
> "man access" says "The check is done using the calling process's real UID and
> GID, rather than the effective IDs".

Yes, indeed. Do we change the docs, or try to get the old behaviour somehow?

[bruening, 2016-12-11 21:58:16]

On 2016/12/11 07:58:30, Edmund.Grimley.Evans wrote:
> On 2016/12/11 02:33:59, bruening wrote:
> > "man access" says "The check is done using the calling process's real UID
and
> > GID, rather than the effective IDs".
> 
> Yes, indeed. Do we change the docs, or try to get the old behaviour somehow?

I guess the main question is how valuable is it to switch to access().  If using
access() on UNIX really does cover mounting and SELinux on Linux, Android, and
Mac, it seems like it'd be worth putting it in even if it uses real ID's, so
long as we document it (no known current uses with setuid binaries).  We could
also try faccessat with AT_EACCESS but I'm not sure if that's available
everywhere.

Please see the history of changes for Android where several changes were put in
to handle various problems there (things like root being able to write places
marked unwritable): we'd have to make sure access() covered those.

For further history, xref i#1730 Windows problems, where access() does *not*
check anything beyond local file permission bits and extra try-write checks are
required, and I believe we just ported that same approach to UNIX assuming
access() behaved the same.