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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
