Re: [RFC] vfs: cleanup of permission()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux