Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 864f0f61 authored by J. Bruce Fields's avatar J. Bruce Fields
Browse files

nfsd: simplify fh_verify access checks 



All nfsd security depends on the security checks in fh_verify, and
especially on nfsd_setuser().

It therefore bothers me that the nfsd_setuser call may be made from
three different places, depending on whether the filehandle has already
been mapped to a dentry, and on whether subtreechecking is in force.

Instead, make an unconditional call in fh_verify(), so it's trivial to
verify that the call always occurs.

That leaves us with a redundant nfsd_setuser() call in the subtreecheck
case--it needs the correct user set earlier in order to check execute
permissions on the path to this filehandle--but I'm willing to accept
that minor inefficiency in the subtreecheck case in return for more
straightforward permission checking.

Signed-off-by: default avatarJ. Bruce Fields <bfields@citi.umich.edu>
parent 9b8b317d

fs/nfsd/nfsfh.c

+21 −29
Original line number Diff line number Diff line
@@ -233,14 +233,6 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp) 
		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",
@@ -295,28 +287,28 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, int access) 
		error = nfsd_set_fh_dentry(rqstp, fhp);

		if (error)

			goto out;

		dentry = fhp->fh_dentry;

		exp = fhp->fh_export;

	} else {

		/*

		 * just rechecking permissions

		 * (e.g. nfsproc_create calls fh_verify, then nfsd_create

		 * does as well)

		 */

		dprintk("nfsd: fh_verify - just checking\n");

	}

	dentry = fhp->fh_dentry;

	exp = fhp->fh_export;

	/*

		 * Set user creds for this exportpoint; necessary even

		 * in the "just checking" case because this may be a

		 * filehandle that was created by fh_compose, and that

		 * is about to be used in another nfsv4 compound

		 * operation.

	 * We still have to do all these permission checks, even when

	 * fh_dentry is already set:

	 * 	- fh_verify may be called multiple times with different

	 * 	  "access" arguments (e.g. nfsd_proc_create calls

	 * 	  fh_verify(...,NFSD_MAY_EXEC) first, then later (in

	 * 	  nfsd_create) calls fh_verify(...,NFSD_MAY_CREATE).

	 *	- in the NFSv4 case, the filehandle may have been filled

	 *	  in by fh_compose, and given a dentry, but further

	 *	  compound operations performed with that filehandle

	 *	  still need permissions checks.  In the worst case, a

	 *	  mountpoint crossing may have changed the export

	 *	  options, and we may now need to use a different uid

	 *	  (for example, if different id-squashing options are in

	 *	  effect on the new filesystem).

	 */

	error = nfsd_setuser_and_check_port(rqstp, exp);

	if (error)

		goto out;

	}



	error = nfsd_mode_check(rqstp, dentry->d_inode->i_mode, type);

	if (error)