Re: [fuse-devel] fuse+aufs
Miklos Szeredi: > Yes. And I think there's actually a bug in there. The comment says, > that the attributes are refreshed after a timeout, but that may not > always happen. > > So fuse_permission() should check the attribute validity timeout and > if it has expired, refresh the attributes before calling > generic_permission(). > > Thanks for spotting that :) You're welcome. > If you hold onto the dentry, then that can happen, yes. But holding > the dentry for an indefinite time may not be a good idea, because if > the rmdir() did succeed, that could be keeping some resources > (e.g. disk space) from being freed. I don't think that you are thinking that aufs could do such thing seriously. Bye Junjiro Okajima - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
Re: [fuse-devel] fuse+aufs
Miklos Szeredi: > I have not said that a filesystem must not refresh it's attributes in > ->permission(), but it does not _have_ to do that. It's the > filesystem own private choice, and none of the VFS's business :) I understand that you wrote about the plan to move ->permission. And it indicates what you think about the border of VFS and fs. But current fuse code, for example, fuse_permission() is enough to make me confused who is reading your opinion zillion times. You may say it is OK since fuse_permission() refreshes inode after generic_permission() returns EACCES. Why don't you refresh it before calling generic_permission()? If it returns success, it may be due to stale (in your word) inode, isn't it? If so, it can be a security hole. > OK, so there's no problem whatsoever. What are we arguing about then? I already recognize your opnion which I don't agree. But it is not a problem. I don't object you as I have already written. > What do you mean? If rmdir fails, then the directory won't be > removed. It is a matter of a test you suggested. After the failure of rmdir, dentry may be unhashed. So the test will not work well, I think. Junjiro Okajima - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
Re: [fuse-devel] fuse+aufs
Miklos Szeredi: > generic_permission() is only called, if the filesystem doesn't define > i_op->permission(). Fuse defines i_op->permission() and fuse_permission() calls generic_permission(). > I've fixed that, by setting n_link to 1 for the root. Is there some > other problem? Tomas wrote that he passed nlink test in aufs. > BTW, I think a better way to check if a directory (or file) has been > unliked is checking with d_unhashed(). Is this available too, in case of the dir which has experienced a rmdir failure? Or in case of root dir? Junjiro Okajima - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
Re: [fuse-devel] fuse+aufs
Miklos Szeredi: > Which one was that? The only two I see invloving i_uid, are the > sticky check and the O_NOATIME check. generic_permission() is referencing i_uid. According to your opinion, i_uid has to be updated by getattr, hasn't it? Or are you planning to refresh inode before calling generic_permission()? > Why is this causing problems in the fuse case? If you just copy the > attributes, that should be perfectly fine. Sorry. In Tomas's case, aufs lookup the specified dir, later aufs checks it was not removed by its i_nlink. Junjiro Okajima - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
Re: [fuse-devel] fuse+aufs
Miklos Szeredi: > And I've shown, that all the other cases are irrelevant. I didn't agree all of them, especially about permission bits and i_uid. > You seem to think, that we've already decided, that the right way to > deal with this is to refresh the attributes on lookup. Not *right* way. While I don't know the correct English word, I will call as 'a workaround for fuse.' > The fact is, file attributes are usually not needed during or after > lookup, so refreshing them unconditionally is probably the wrong thing > to do. It is entirely depending upon the filesystem. > I still don't know why aufs needs those attributes. Can you please > ellaborate? For example, aufs lookup is actually lookup for the lower (real) filesystem. After the real lookup, aufs copies the real inode attributes to the virtual inode. > A lookup on "/" won't revalidate the root dentry. What I wrote is stat(2) which includes getattr. > In this case, should the stat fail, because the foo is stale? I don't > think so. It shouln'd matter if foo was renamed, because it is not > being looked up, only it's attributes are being queried. > > Now NFS will probably fail in that case because of the REVAL_DOT > thingy, but that's not the right thing to do IMO. I, probably, understand what you are saying repeatedly. Also I guess the main difference between you and me is whether to rely on the inode attr in positive (lookup-ed) dentry or not. In other word, explicit getattr after lookup, before referencing inode attr, is required or not. Junjiro Okajima - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
Re: [fuse-devel] fuse+aufs
Miklos Szeredi: > You misunderstand. What I think is: > > To be able to correctly perform permission checks based on cached > inode attributes, those attributes may need to be refreshed before > making the permission checks. You seem to be replacing the problem. The problem is more generic, not only permission check. What you wrote is, - in order to refer any inode attributes except file type in i_mode, the caller have to update the inode. - the caller must not rely on the inode attributes in a positive dentry which is returned by path_lookup(). isn't it? The two cases you mentioned (sticky bit and may_open()) is a part of the list I wrote as example where VFS accesses inode attributes. And I don't have a paticular opinion about judging those are bugs. I already decided to add a special handling for fuse into aufs. Because you wrote, - you are the one who expected to fix VFS about this issue. - you have another thing more important. > Let's take a concrete example: ::: > Should that rmdir succeed? Surely not, the sticky check would fail. > But if the stale attributes are used, than it _will_ succeed. I thought we have already agreed about the staleness of cached nfs inode including REVAL_DOT. Junjiro Okajima - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
