2004-04-30 08:49:27

by Greg Banks

[permalink] [raw]
Subject: Re: Marking inodes as stale can be wrong

Olaf Kirch wrote:
>
> Here's a minimalistic fix. It may be better to change nfsd_acceptable
> to return 0 on success and negative error code on error, but I'm not
> sure this is really needed as long as the only acceptance test
> in nfsd_acceptable is the subtree check.

I think the patch makes the opposite mistake to the current code.
RFC1813 says:

NFS3ERR_ACCES
Permission denied. The caller does not have the correct
permission to perform the requested operation.[...]

NFS3ERR_STALE
Invalid file handle. The file handle given in the
arguments was invalid. The file referred to by that file
handle no longer exists or access to it has been
revoked.

So in the case where "subtree_check" is on and the client sends a
file handle to a file in a tree which has been unexported, a server
with the patch will incorrectly send ACCES instead of STALE.

I think the real fix is your other suggestion: changing the calling
convention of nfsd_acceptable to allow it to choose the error.

Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.


-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2004-04-30 09:00:46

by Olaf Kirch

[permalink] [raw]
Subject: Re: Marking inodes as stale can be wrong

On Fri, Apr 30, 2004 at 06:47:24PM +1000, Greg Banks wrote:
> So in the case where "subtree_check" is on and the client sends a
> file handle to a file in a tree which has been unexported, a server
> with the patch will incorrectly send ACCES instead of STALE.

When we get there, we have already found a connected, valid dentry. We
also have an export entry, which was specified by the client. If we don't
have either, we'll error mout with ESTALE long before.

All that's left to do at this point is make sure the dentry

a) actually resides below the exported inode
b) all intermediate directories have +x for the caller

So the only case where this patch incorrectly(?) returns EACCESS is if
the specified dentry does not reside below the exported dentry.

So yes, the cleaner approach is probably to change nfsd_acceptable.

Olaf
--
Olaf Kirch | The Hardware Gods hate me.
[email protected] |
---------------+


-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs