[PATCH] VFS: document what MAY_ACCESS means

[PATCH] VFS: document what MAY_ACCESS means

[Eric Paris]

The vfs MAY_ACCESS flag really means that we might not use the object
immediately (consider chdir which might not actually use the new dir).
Thus permissions must be checked rather than relying on checkes during
later access of the object in question.  This patch just adds some
documentation so the meaning of the flag is clear.  I would rename the flag,
but it's already visable (although useless) to userspace.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 include/linux/fs.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 215b708..f683b29 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -51,6 +51,13 @@ struct inodes_stat_t {
 #define MAY_WRITE 2
 #define MAY_READ 4
 #define MAY_APPEND 8
+/*
+ * The vfs MAY_ACCESS flag really means that we might not use the object
+ * immediately (consider chdir which might not actually use the new dir).
+ * Thus permissions must be checked mmediately rather than relying on later
+ * checks during the actual user of the object in question.  This is an
+ * internal flag and should not come from userspace.
+ */
 #define MAY_ACCESS 16
 #define MAY_OPEN 32
 

Re: [PATCH] VFS: document what MAY_ACCESS means

[Nicholas Miell]

Since this is a documentation patch, here are some nits.

> +/*
> + * The vfs MAY_ACCESS flag really means that we might not use the object
> + * immediately (consider chdir which might not actually use the new dir).
> + * Thus permissions must be checked mmediately rather than relying on later
                                       ^ immediately
> + * checks during the actual user of the object in question.  This is an
                               ^ use
> + * internal flag and should not come from userspace.
> + */
>  #define MAY_ACCESS 16
>  #define MAY_OPEN 32

-- 
Nicholas Miell <nmiell@comcast.net>

Re: [PATCH] VFS: document what MAY_ACCESS means

[Jamie Lokier]

Eric Paris wrote:
> The vfs MAY_ACCESS flag really means that we might not use the object
> immediately (consider chdir which might not actually use the new dir).
> Thus permissions must be checked rather than relying on checkes during
> later access of the object in question.  This patch just adds some
> documentation so the meaning of the flag is clear.  I would rename the flag,
> but it's already visable (although useless) to userspace.

As it's intended to clarify the meaning, I must admit that I didn't
find the comment clear at all!  I had to grep the code for MAY_ACCESS
to understand what your comment meant.

Especially what was meant by "chdir which might not actually use the
new dir".

Suggest: MAY_ACCESS means we are calling from access() or chdir() and
won't do the actual read/write/exec/appene/open, so ->permission()
must fully check the permission and not assume it can optimise away checks.

(Btw, side issue: I was very surprised to find fchdir() to an open
directory can fail on NFS due to change of permissions, so the pattern
dir = open("."); chdir("foo"); fchdir(dir) can fail to restore the
current directory).

-- Jamie


> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  include/linux/fs.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 215b708..f683b29 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -51,6 +51,13 @@ struct inodes_stat_t {
>  #define MAY_WRITE 2
>  #define MAY_READ 4
>  #define MAY_APPEND 8
> +/*
> + * The vfs MAY_ACCESS flag really means that we might not use the object
> + * immediately (consider chdir which might not actually use the new dir).
> + * Thus permissions must be checked mmediately rather than relying on later
> + * checks during the actual user of the object in question.  This is an
> + * internal flag and should not come from userspace.
> + */
>  #define MAY_ACCESS 16
>  #define MAY_OPEN 32
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] VFS: document what MAY_ACCESS means

[Trond Myklebust]

On Mon, 2009-09-21 at 09:10 +0100, Jamie Lokier wrote
> (Btw, side issue: I was very surprised to find fchdir() to an open
> directory can fail on NFS due to change of permissions, so the pattern
> dir = open("."); chdir("foo"); fchdir(dir) can fail to restore the
> current directory).

Welcome to the world of stateless server-enforced security. Unlike the
POSIX model, a NFS server doesn't have the ability to track what
permissions have already been checked using a file descriptor. It
therefore needs to check permissions on each RPC operation you perform
using the credential you present then and there.

Cheers
  Trond

Re: [PATCH] VFS: document what MAY_ACCESS means

[Jamie Lokier]

Trond Myklebust wrote:
> On Mon, 2009-09-21 at 09:10 +0100, Jamie Lokier wrote
> > (Btw, side issue: I was very surprised to find fchdir() to an open
> > directory can fail on NFS due to change of permissions, so the pattern
> > dir = open("."); chdir("foo"); fchdir(dir) can fail to restore the
> > current directory).
> 
> Welcome to the world of stateless server-enforced security. Unlike the
> POSIX model, a NFS server doesn't have the ability to track what
> permissions have already been checked using a file descriptor. It
> therefore needs to check permissions on each RPC operation you perform
> using the credential you present then and there.

No, no, it's not that.

It's allowed to have the current directory be a directory you can't
access any more.  You find out you've lost permission, as you say,
later when you _do_ something with the directory.  In other words when
you do a lookup or readdir from the directory.

Putting it another way, this will _never_ error due to another process
messing with the permissions of the original directory after subdir is
opened:

    dir=open(".");
    dir2=open("/elsewhere");
    fstatat(dir2, "something_elsewhere");

But this can fail, leaving you in a different directory:

    dir=open(".");
    dir2=open("/elsewhere");
    fchdir(dir2);
    stat("something_elsewhere");
    fchdir(dir);

I find that surprising.  Imho, both codes are intended to have the
same behaviour.

Is there something in POSIX which says fchdir() must verify you still
have execute permission in the directory you are switching to at the
time you call fchdir()?

I suspect having fchdir() fail in this admittedly obscure case is more
likely to cause problems than the RPC permission check, due to
surprise and no obvious error recovery strategy in an application
where fchdir is used in some subroutine to temporarily switch
directory and then return to the caller, which doesn't expect the
current directory might be changed by the call.  I suspect when that
happens, most applications will either terminate abruptly or behave
wrongly later.  It's just a guess though....

-- Jamie

Re: [PATCH] VFS: document what MAY_ACCESS means

[Trond Myklebust]

On Mon, 2009-09-21 at 19:53 +0100, Jamie Lokier wrote:
> Trond Myklebust wrote:
> > On Mon, 2009-09-21 at 09:10 +0100, Jamie Lokier wrote
> > > (Btw, side issue: I was very surprised to find fchdir() to an open
> > > directory can fail on NFS due to change of permissions, so the pattern
> > > dir = open("."); chdir("foo"); fchdir(dir) can fail to restore the
> > > current directory).
> > 
> > Welcome to the world of stateless server-enforced security. Unlike the
> > POSIX model, a NFS server doesn't have the ability to track what
> > permissions have already been checked using a file descriptor. It
> > therefore needs to check permissions on each RPC operation you perform
> > using the credential you present then and there.
> 
> No, no, it's not that.
> 
> It's allowed to have the current directory be a directory you can't
> access any more.  You find out you've lost permission, as you say,
> later when you _do_ something with the directory.  In other words when
> you do a lookup or readdir from the directory.
> 
> Putting it another way, this will _never_ error due to another process
> messing with the permissions of the original directory after subdir is
> opened:
> 
>     dir=open(".");
>     dir2=open("/elsewhere");
>     fstatat(dir2, "something_elsewhere");
> 
> But this can fail, leaving you in a different directory:
> 
>     dir=open(".");
>     dir2=open("/elsewhere");
>     fchdir(dir2);
>     stat("something_elsewhere");
>     fchdir(dir);
> 
> I find that surprising.  Imho, both codes are intended to have the
> same behaviour.
> 
> Is there something in POSIX which says fchdir() must verify you still
> have execute permission in the directory you are switching to at the
> time you call fchdir()?
> 
> I suspect having fchdir() fail in this admittedly obscure case is more
> likely to cause problems than the RPC permission check, due to
> surprise and no obvious error recovery strategy in an application
> where fchdir is used in some subroutine to temporarily switch
> directory and then return to the caller, which doesn't expect the
> current directory might be changed by the call.  I suspect when that
> happens, most applications will either terminate abruptly or behave
> wrongly later.  It's just a guess though....

Oh, I see what you're getting at.

So, looking at the code for fchdir(), it would appear that the
permission check there is being made by the VFS, not NFS. I suspect that
is because it is mandated by POSIX.
Indeed, looking at the spec for fchdir(), it would appear that you _do_
need valid permissions. See

   http://www.opengroup.org/onlinepubs/009695399/functions/fchdir.html

which states that the function returns EACCES if the process doesn't
have search permissions.

Cheers
  Trond

fchdir, EACCESS and when to check (was: [PATCH] VFS: document what MAY_ACCESS means)

[Jamie Lokier]

Trond Myklebust wrote:
> > I suspect having fchdir() fail in this admittedly obscure case is more
> > likely to cause problems than the RPC permission check, due to
> > surprise and no obvious error recovery strategy in an application
> > where fchdir is used in some subroutine to temporarily switch
> > directory and then return to the caller, which doesn't expect the
> > current directory might be changed by the call.  I suspect when that
> > happens, most applications will either terminate abruptly or behave
> > wrongly later.  It's just a guess though....
> 
> Oh, I see what you're getting at.
> 
> So, looking at the code for fchdir(), it would appear that the
> permission check there is being made by the VFS, not NFS. I suspect that
> is because it is mandated by POSIX.
> Indeed, looking at the spec for fchdir(), it would appear that you _do_
> need valid permissions. See
> 
>    http://www.opengroup.org/onlinepubs/009695399/functions/fchdir.html
> 
> which states that the function returns EACCES if the process doesn't
> have search permissions.

Sadly I think you're right.  The check is performed locally, too.
It's not an NFS quirk (unlike, say, read permissions), and local
concurrency can trip it up.

Well well well.

http://www.mail-archive.com/bug-gnulib@gnu.org/msg12513.html

    "using only fchdir does have the advantage that we know that
    restoring the current directory can't fail.  (It can fail IIRC on
    SunOS, but I think we don't support the affected versions any
    more)."

That doesn't look convincing any more.

[Have added James Youngman who wrote that, to Cc]

Let's look around:

http://www.opengroup.org/onlinepubs/9699919799/functions/openat.html

    "the file to be opened is determined relative to the directory
    associated with the file descriptor fd instead of the current
    working directory. If the file descriptor was opened without
    O_SEARCH, the function shall check whether directory searches are
    permitted using the current permissions of the directory
    underlying the file descriptor. If the file descriptor was opened
    with O_SEARCH, the function shall not perform the check."

Also,
http://www.opengroup.org/austin/docs/austin_383.txt

That's not about fchdir(), but it's a general property of directories
opened with O_SEARCH when used with *at() functions.

fchdir() doesn't apply the "shall not perform the check" rule.  It's
only used for *at() lookups.  Given the existence of any rule which
allows search permission to be checked at open() time and not checked
later, it looks like it might be quite useful for fchdir() to have the
same property, so you'd know you can always return to a directory if you
successfully opened it before.

I've added Ulrich Drepper to the Cc list in case he cares to say
something about fchdir(), since he seems to have introduced O_SEARCH
to SUS.

Ulrich, do you think fchdir should fail even if you successfully
opened a directory with O_SEARCH (if that is eventually implemented in
Linux ;-), when the permissions have been changed after the descriptor
is opened, even though all the *at() functions can successfully search
using the descriptor?

-- Jamie