lkml.org 
[lkml]   [2006]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] vfs: cleanup of permission()
Trond Myklebust wrote:
>>after thinking some time about the oracle words
>>(sent in reply to previous BME submissions) we
>>(Sam and I) came to the conclusion that it would
>>be a good idea to remove the nameidata introduced
>>in September 2003 from the inode permission()
>>checks, so that vfs_permission() can take care
>>of them ...
> Why? There may be perfectly legitimate reasons for the filesystem to
> request information about the path.

Part of what we're trying to achieve is clearly seperating VFS from FS
operations. A lot of the problems with supporting the per-vfsmnt
options stemmed from this confusion.

In fact this was largely developed from your feedback on the prior
thread¹ :) we audited the uses, and you were right - some places really
couldn't set that field properly. After some playing and discussion, we
came to guess that this kind of cleanup was a good way to satisfy your
concerns as well as Christoph's².

¹ http://xrl.us/j9et
² http://xrl.us/j9eo

The only place that the nameidata structure is currently used is in
those two places, and given that it *does* appear to represent FS vs VFS
brain damage to some extent, it's quite important we get this right
before umpteen security modules spring up using this bad hook ... the
API is broken (doesn't cover (struct file*)-based operations, only
lookup (struct nameidata*), and parameter is optional).

> I can think of server failover
> situations in NFSv4 where the client may need to look up the filehandle
> for the file on the new server before it can service the ACCESS call.

The current code doesn't seem to use that right now, at least not via
permission(). Either way, the information should be available - it just
needs to hook in somewhere else.

> Firstly, the fact that the lookup intent flags happen not to collide
> with MAY_* is a complete fluke, not a design. The numerical values of
> either set of flags could change tomorrow for all you know.
> Secondly, an intent is _not_ a permissions mask by any stretch of the
> imagination.

Yeah, I had this in there at one point as a safety check;

#if (_LOOKUP_MASK ^ _ACCESS_MASK) != (_LOOKUP_MASK | _ACCESS_MASK)
#error splode
#endif

You are right, it should either be converted to two parameters or the
combination made more formal.

> IOW: at the very least make that intent flag a separate parameter.

Yes, or rename it mask_and_intent perhaps if they were to be combined in
the same word (which makes sense, with a grand total of 7 bits being
used, and gcc not being in a position to spot that and optimise for us).

>
> Cheers,
> Trond

Thanks for your feedback!

Sam.



-
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: 2006-03-01 23:14    [W:0.096 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site