lkml.org 
[lkml]   [2003]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: permission() bug?
Date
Andreas Gruenbacher wrote:

> I think there is a bug in fs/namei.c:vfs_permission(). The function
> contains:
>
>
> int vfs_permission(struct inode * inode, int mask)
> {
> [...]
> /*
> * Read/write DACs are always overridable.
> * Executable DACs are overridable if at least one exec
> * bit is set.
> */
> if ((mask & (MAY_READ|MAY_WRITE)) || (inode->i_mode & S_IXUGO))
> if (capable(CAP_DAC_OVERRIDE))
> return 0;
> [...]
> return -EACCES;
> }
>
>
> The comment makes sense; the code doesn't quite implement what the
> comment says. Consider the case of an inode with "--" permissions. We
> get the following results:
>
> permission(inode, MAY_READ) = 0
> permission(inode, MAY_EXEC) = -EACCESS
> permission(inode, MAY_READ|MAY_EXEC = 0
>
> The last result seems wrong; I would expect -EACCESS instead.

Some pieces from SUSv3 that look relevant to me:

[[
<unistd.h>

The constants F_OK, R_OK, W_OK, and X_OK and the expressions
R_OK|W_OK, R_OK|X_OK, and R_OK|W_OK|X_OK shall all have
distinct values.

access()
If any access permissions are checked, each shall be checked
individually, as described in the Base Definitions volume of
IEEE Std 1003.1-2001, Chapter 3, Definitions. If the process
has appropriate privileges, an implementation may indicate
success for X_OK even if none of the execute file permission
bits are set.

ERRORS
[EACCES] Permission bits of the file mode do not
permit the requested access, or search permission is
denied on a component of the path prefix.
]]

The implication of all this is that it is bits/masks that are
relevant for the check. In this interpretation it is nonsensical
that access() on a file with no perms should return:

0 if mask is R_OK | X_OK

but

-1/EACCES if mask is just X_OK.

I'd say that in this case, both of these calls should fail
(-1/EACCES), though in my reading of the following,
SUSv3 allows (but discourages) the possibility that both
would succeed. (What is bizarre is the current scenario
where one of the above calls succeeds and the other fails...)

[[
SUSv3 rationale to access():
In early proposals, some inadequacies in the access()
function led to the creation of an eaccess() function
because:

1. Historical implementations of access() do not test
file access correctly when the process' real user
ID is superuser. In particular, they always return
zero when testing execute permissions without regard
to whether the file is executable.

2. The superuser has complete access to all files on a
system. As a consequence, programs started by the
superuser and switched to the effective user ID with
lesser privileges cannot use access() to test their
file access permissions.

However, the historical model of eaccess() does not
resolve problem (1), so this volume of IEEE Std
1003.1-2001 now allows access() to behave in the desired
way because several implementations have corrected the
problem. It was also argued that problem (2) is more
easily solved by using open(), chdir(), or one of the
exec functions as appropriate and responding to the
error, rather than creating a new function that would
not be as reliable. Therefore, eaccess() is not included
in this volume of IEEE Std 1003.1-2001.

The sentence concerning appropriate privileges and
execute permission bits reflects the two possibilities
implemented by historical implementations when checking
superuser access for X_OK.

New implementations are discouraged from returning X_OK
unless at least one execution permission bit is set.
]]

> So IMHO
> the code in permission (and in the file system specific copies) should
> read:
>
> if (!(mask & MAY_EXEC) || (inode->i_mode & S_IXUGO))
> if (capable(CAP_DAC_OVERRIDE))
> return 0;

The above is consistent with how I interpret SUSv3.

Cheers,

Michael
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:58    [W:0.070 / U:0.612 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site