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

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, May 08, 2008 at 01:03:09PM +1000, Neil Brown wrote:
> On Tuesday May 6, bfields@fieldses.org 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.

Something like that, yep.  (Frank, can you confirm that this does the
job for you?)

It'd be nice if we could find a way to incorporate a little cleanup at
the same time, but I'm not sure exactly what to suggest.

--b.

> 
> NeilBrown
> 
> Signed-off-by: Neil Brown <neilb@suse.de>
> 
> ### 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",