2004-04-29 14:43:37

by Olaf Kirch

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

Hi,

I've been debugging a strange problem with kmail on an NFS mounted
home directory. Attaching files to an outgoing message would cause
the mail to fail. For instance, after opening the file selection
dialog, kmail was no longer able to read the outbox.
This was 100 % reproduceable

The problem was caused by the following:

- a KDE component would invoke some silly setuid
helper program prior to displaying the file
selection dialog. This helper would receive all
open file descriptors, including those of the
open mail boxes.

- the helper program itself did not access any of
these files, but for some reason, we still ended
up calling revalidate_inode on the open files.
I haven't quite found out why.

- nfs_revalidate_inode would do a GETATTR call to
the server (running the 2.4.22 kernel), using
uid 0.

The file system was exported with root_squash
and subtree_check.

fh_verify did the subtree check, and found that
~/Mail was mode 700, and hence user nobody didn't
have x permissions on the directory. So it would
return ESTALE to the client.

- The client marks the file handle as STALE

- The next attempt to read from the file fails because
of this.

How should we fix this? I asked the KDE folks to mark their mail box
file descriptor FD_CLOEXEC, but that is just a workaround, as other
application may exhibit similar problems.

I think it's wrong to mark the file handle STALE for everyone; whether
GETATTR reports NFSERR_STALE can depend on the process doing the call.

What is the rationale behind making the stale-ness information sticky?
Can we safely get rid of the NFS_INO_STALE flag?

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


2004-04-29 17:09:34

by Trond Myklebust

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

On Thu, 2004-04-29 at 12:58, Olaf Kirch wrote:
> On Thu, Apr 29, 2004 at 12:44:56PM -0400, Trond Myklebust wrote:
> > On Thu, 2004-04-29 at 10:40, Olaf Kirch wrote:
> > > fh_verify did the subtree check, and found that
> > > ~/Mail was mode 700, and hence user nobody didn't
> > > have x permissions on the directory. So it would
> > > return ESTALE to the client.
> >
> > This is a bug!
>
> Yes, probably. Even though I'm not entirely sure NFSERR_ACCES is a valid
> return for all NFS operations.

The problem is that Neil is using NFSERR_STALE as a standin for
NFSERR_ACCES precisely because the latter is not on the list of accepted
return value for GETATTR in RFC1813.
IMO this is *worse* behaviour than just returning the non-rfc compatible
error.

> However, machines with 2.4 will be out there for a while, so the question
> I'm asking myself is how to cope with those in the client.

I am totally uninterested in fixing up a broken server option (even if
it is a default option) inside the client. These "fixes" have a tendency
to persist for way too long in the kernel code, and end up causing way
more problems in the long run.

"subtree_check" has a number of other RFC-violating bugs (most notably
with cross-directory renames) that mean we should *never* have made it a
default option in the first place.
The correct workaround here is to advise people to *always* use
"no_subtree_check" and then to look into fixing these problems in future
revisions of the server, not the client.

Cheers,
Trond


-------------------------------------------------------
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-29 17:21:51

by Trond Myklebust

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

On Thu, 2004-04-29 at 10:40, Olaf Kirch wrote:
> Hi,

> - nfs_revalidate_inode would do a GETATTR call to
> the server (running the 2.4.22 kernel), using
> uid 0.

Note: several of the nfs_file_*() functions call nfs_revalidate_inode(),
and they do so with task creds instead of using the actual credentials
in the struct file.

While I disagree that the client's interpretation of NFSERR_STALE is
incorrect, I do agree that using the wrong creds when revalidating an
open file is indeed a client bug, and should be fixed.

Cheers,
Trond


-------------------------------------------------------
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-29 21:28:40

by Olaf Kirch

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

On Thu, Apr 29, 2004 at 01:09:30PM -0400, Trond Myklebust wrote:
> The correct workaround here is to advise people to *always* use
> "no_subtree_check" and then to look into fixing these problems in future
> revisions of the server, not the client.

I agree with that.

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

2004-04-29 23:51:39

by Greg Banks

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

On Thu, Apr 29, 2004 at 01:09:30PM -0400, Trond Myklebust wrote:
> On Thu, 2004-04-29 at 12:58, Olaf Kirch wrote:
> > On Thu, Apr 29, 2004 at 12:44:56PM -0400, Trond Myklebust wrote:
> > > On Thu, 2004-04-29 at 10:40, Olaf Kirch wrote:
> > > > fh_verify did the subtree check, and found that
> > > > ~/Mail was mode 700, and hence user nobody didn't
> > > > have x permissions on the directory. So it would
> > > > return ESTALE to the client.
> > >
> > > This is a bug!
> >
> > Yes, probably. Even though I'm not entirely sure NFSERR_ACCES is a valid
> > return for all NFS operations.
>
> The problem is that Neil is using NFSERR_STALE as a standin for
> NFSERR_ACCES precisely because the latter is not on the list of accepted
> return value for GETATTR in RFC1813.
> IMO this is *worse* behaviour than just returning the non-rfc compatible
> error.

FWIW there's a little known sentence in RFC1813 Section 3 which says

> The ERRORS section lists the errors returned for specific types of
> failures. These lists are not intended to be the definitive statement
> of all of the errors which can be returned by any specific procedure,
> but as a guide for the more common errors which may be returned.

This has been interpreted in discussions here as a loophole that allows
the server to return any of the defined NFS errors for any of the calls
at its discretion.

I don't know why the server returns NFSERR_STALE instead of NFSERR_ACCES,
but RFC compliance is not a good reason.

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 00:43:47

by Trond Myklebust

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

On Thu, 2004-04-29 at 19:50, Greg Banks wrote:
> > The ERRORS section lists the errors returned for specific types of
> > failures. These lists are not intended to be the definitive statement
> > of all of the errors which can be returned by any specific procedure,
> > but as a guide for the more common errors which may be returned.
>
> This has been interpreted in discussions here as a loophole that allows
> the server to return any of the defined NFS errors for any of the calls
> at its discretion.

True, and indeed I should have known that.

Note for instance that the error NFS3ERR_JUKEBOX is not listed as a
return value for any of the procedures: I'm sure that it would come as a
surprise to most server implementors if you were to disallow its use
solely on those grounds.

> I don't know why the server returns NFSERR_STALE instead of NFSERR_ACCES,
> but RFC compliance is not a good reason.

Right. It is quite simply a bug...

NFS3ERR_STALE can be returned if the server unexports the filesystem
("revokes access") but here we are talking about a pure credential-based
access check, and so NFS3ERR_ACCES is the correct return value.

Cheers,
Trond


-------------------------------------------------------
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-29 16:44:59

by Trond Myklebust

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

On Thu, 2004-04-29 at 10:40, Olaf Kirch wrote:
> fh_verify did the subtree check, and found that
> ~/Mail was mode 700, and hence user nobody didn't
> have x permissions on the directory. So it would
> return ESTALE to the client.

This is a bug!

Cheers,
Trond


-------------------------------------------------------
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-29 16:58:06

by Olaf Kirch

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

On Thu, Apr 29, 2004 at 12:44:56PM -0400, Trond Myklebust wrote:
> On Thu, 2004-04-29 at 10:40, Olaf Kirch wrote:
> > fh_verify did the subtree check, and found that
> > ~/Mail was mode 700, and hence user nobody didn't
> > have x permissions on the directory. So it would
> > return ESTALE to the client.
>
> This is a bug!

Yes, probably. Even though I'm not entirely sure NFSERR_ACCES is a valid
return for all NFS operations.

However, machines with 2.4 will be out there for a while, so the question
I'm asking myself is how to cope with those in the client.

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