lkml.org 
[lkml]   [2009]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/8] Simplify exec_permission_lite() further


On Tue, 8 Sep 2009, Jamie Lokier wrote:
>
> Linus Torvalds wrote:
> > This function is only called for path components that are already known
> > to be directories (they have a '->lookup' method). So don't bother
> > doing that whole S_ISDIR() testing,
>
> A few years ago you briefly discussed diddling the VFS to accept
> objects which are both files and directories. That is, have
> ->lookup() and also can be read as regular files.
>
> I don't know if your thinking has changed, and I don't particularly
> care, only thought I'd remind in case it's something you'd like to
> support at some point.

No, I explicitly thought about that case too, and the thing is, that even
_if_ we do that some day, the change is actually the RightThing(tm) to do.

In fact, it's even _more_ true in that case. The old code was broken for
that case.

The thing is, if we ever implement the concept of using a regular file as
a path component (say, we look up some attributes or we teach the kernel
to look into tar-files or whatever), then the permission check for using
it as a path component would be _different_ from the permission check for
"executing" the file as a file. When used as a path component, we'd want
to use CAP_DAC_SEARCH, for example, not some "is it ok to execute this
file" permission.

So "permission" for a path component really is fundamentally different
from the same thing as an actual end-result file open/use. They do share
some logic, of course, like just the whole owner/group/other and ACL
logic. But at the same time they have conceptual - and actual -
differences too.

Just think about what "execute" permission means: it's not just the 'x'
bit, there are things like per-filesystem "NOEXEC" bits. We check that at
a completely different level already - exactly because the "execute"
permission for a pathname component is something fundamentally _different_
from executing the result of an actual final lookup.

The "exec_permission_lite()" function makes some of that difference more
explicit - and also makes it explicit what is shared. We do check the 'x'
bit, and we do honor ACL bits, but the whole Integrity check part was very
obviously meant to be done at the "actual final lookup result" level
rather than at the path component level.

In fact, as it was, "ima_path_check()" wouldn't know whether MAY_EXEC
meant that we were going to actually run a program, or look up a pathname.
The way it "solved" that was to just ignore non-regular files. And I'm
convinced that the real solution is to just say that "ima_path_check()" is
always checking the end result of a path, not the components, and then
MAY_EXEC actually has a much more unambiguous meaning.

(Admittedly I think the IMA code could also have disambiguated the cases
by looking at whether MAY_OPEN is set ot not - looking at whether the
inode in question had S_ISDIR set probably had other reasons too).

Anyway, I do suspect that if we ever do the whole "able to treat regular
files as a lookup target" we _will_ hit other issues, and it's going to
require a lot of thought - but I suspect this series is actually going to
help it by separating out the permission handling more clearly.

Linus


\
 
 \ /
  Last update: 2009-10-18 23:28    [W:1.061 / U:0.568 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site