Re: [PATCH] exportfs: be careful to only return expected errors.

On Thu, Aug 04 2016, Christoph Hellwig wrote:

On Thu, Aug 04, 2016 at 10:19:06AM +1000, NeilBrown wrote:

quoted

When nfsd calls fh_to_dentry, it expect ESTALE or ENOMEM as errors.
In particular it can be tempting to return ENOENT, but this is not
handled well by nfsd.

Rather than requiring strict adherence to error code code filesystems,
treat all unexpected error codes the same as ESTALE.  This is safest.

Signed-off-by: NeilBrown <redacted>
---

I didn't add a dprintk for unexpected error messages, partly
because dprintk isn't usable in exportfs.  I could have used pr_debug()
but I really didn't see much value.

This has been tested together with the btrfs change, and it restores
correct functionality.

I don't really like all this magic which is partially historic.  I think
we should instead allow the fs to return any error from the export
operations, and forbid returning NULL entirely.  Then the actualy caller
(nfsd) can sort out which errors it wants to send over the wire.

I'm certainly open to that possibility.
But is the "actual caller":
  nfsd_set_fh_dentry(), or
  fh_verify() or
  the various callers of fh_verify() which might have different rules
  about which error codess are acceptable?

I could probably make an argument for having fh_verify() be careful
about error codes, but as exportfs_decode_fh() is a more public
interface, I think it is more important that it have well defined error
options.

Are there *any* errors that could sensibly be returned from
exportfs_decode_fh() other than
  -ESTALE (there is no such file), or
  -ENOMEM (there probably is a file, but I cannot allocate a dentry for
           it) or
  -EACCES (there is such a file, but it isn't "acceptable")

???

If there aren't, why should we let them through?

NeilBrown