2008-05-08 03:03:18

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] exportfs: fix incorrect EACCES in reconnect_path()

On Tuesday May 6, [email protected] wrote:
> On Tue, May 06, 2008 at 10:35:46AM +1000, Neil Brown wrote:
> >
> > To fix the current bug properly, reconnect_path still needs to bypass
> > normal permission checks even when subtree_check is in effect, so it
> > can be sure of getting read permission on the parent directory.
>
> OK, but why not just forget the subtree_check case? It would be just
> another item on the "reasons not to use subtree_check" list.

I guess so.

>
> If a fix for the subtree checking case were easy (or if someone else had
> the time to do a very careful job of it), then fine, but maybe we should
> just fix the easy case and leave the subtree checking as is for now.

So is this the proposed fix? A bit ugly, but I guess it's OK.

NeilBrown

Signed-off-by: Neil Brown <[email protected]>

### Diffstat output
./fs/nfsd/nfsfh.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)

diff .prev/fs/nfsd/nfsfh.c ./fs/nfsd/nfsfh.c
--- .prev/fs/nfsd/nfsfh.c 2008-05-06 10:06:59.000000000 +1000
+++ ./fs/nfsd/nfsfh.c 2008-05-08 13:01:06.000000000 +1000
@@ -176,9 +176,24 @@ static __be32 nfsd_set_fh_dentry(struct
if (IS_ERR(exp))
return nfserrno(PTR_ERR(exp));

- error = nfsd_setuser_and_check_port(rqstp, exp);
- if (error)
- goto out;
+ if (exp->ex_flags & NFSEXP_NOSUBTREECHECK) {
+ /* Elevate privileges so that the lack of 'r' or 'x'
+ * permission on some parent directory will
+ * not stop exportfs_decode_fh from being able
+ * to reconnect a directory into the dentry cache.
+ * The same problem can affect "SUBTREECHECK" exports,
+ * but as nfsd_acceptable depends on correct
+ * access control settings being in effect, we cannot
+ * fix that case easily - so though.
+ */
+ current->cap_effective =
+ cap_raise_nfsd_set(current->cap_effective,
+ current->cap_permitted);
+ } else {
+ error = nfsd_setuser_and_check_port(rqstp, exp);
+ if (error)
+ goto out;
+ }

/*
* Look up the dentry using the NFS file handle.
@@ -215,6 +230,14 @@ static __be32 nfsd_set_fh_dentry(struct
goto out;
}

+ if (exp->ex_flags & NFSEXP_NOSUBTREECHECK) {
+ error = nfsd_setuser_and_check_port(rqstp, exp);
+ if (error) {
+ dput(dentry);
+ goto out;
+ }
+ }
+
if (S_ISDIR(dentry->d_inode->i_mode) &&
(dentry->d_flags & DCACHE_DISCONNECTED)) {
printk("nfsd: find_fh_dentry returned a DISCONNECTED directory: %s/%s\n",