Re: [PATCH] - avoid permission checks on EXCLUSIVE_CREATE replay

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

On Thu, Apr 22, 2010 at 10:10:42AM +1000, Neil Brown wrote:
> 
> With NFSv4, if we create a file then open it we explicit avoid checking the
> permissions on the file during the open because the fact that we created it
> ensures we should be allow to open it (the create and the open should appear
> to be a single operation).
> 
> However if the reply to an EXCLUSIVE create gets lots and the client resends
> the create, the current code will perform the permission check - because it
> doesn't realise that it did the open already..
> 
> This patch should fix this.

Thanks, but: hm, does this leave a loophole for a clever attacker?
They'll still have to get past the initial

	fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE)

but that just checks the parent directory; if the existing file is
actually owned by someone else, do we allow an open that we shouldn't?

Maybe when "created" is set we should keep the permission check but add
NFSD_ALLOW_OWNER_OVERRIDE?

--b.

> 
> Note that I haven't actually seen this cause a problem.  I was just looking
> at the code trying to figure out a different EXCLUSIVE open related issue,
> and this looked wrong.
> 
> NeilBrown
> 
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6dd5f19..ec40b36 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1451,8 +1451,11 @@ nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		case NFS3_CREATE_EXCLUSIVE:
>  			if (   dchild->d_inode->i_mtime.tv_sec == v_mtime
>  			    && dchild->d_inode->i_atime.tv_sec == v_atime
> -			    && dchild->d_inode->i_size  == 0 )
> +			    && dchild->d_inode->i_size  == 0 ) {
> +				if (created)
> +					*created = 1;
>  				break;
> +			}
>  			 /* fallthru */
>  		case NFS3_CREATE_GUARDED:
>  			err = nfserr_exist;