summaryrefslogtreecommitdiffstats
path: root/fs/nfsd/nfsfh.c
diff options
context:
space:
mode:
authorJ. Bruce Fields <bfields@citi.umich.edu>2009-11-25 17:42:05 -0500
committerJ. Bruce Fields <bfields@citi.umich.edu>2009-11-25 17:55:46 -0500
commit864f0f61f829bac5f150a903aad9619322a25424 (patch)
tree77a864ab5538255dfba454d13f67de60807f2973 /fs/nfsd/nfsfh.c
parent9b8b317d58084b9a44f6f33b355c4278d9f841fb (diff)
downloadop-kernel-dev-864f0f61f829bac5f150a903aad9619322a25424.zip
op-kernel-dev-864f0f61f829bac5f150a903aad9619322a25424.tar.gz
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: J. Bruce Fields <bfields@citi.umich.edu>
Diffstat (limited to 'fs/nfsd/nfsfh.c')
-rw-r--r--fs/nfsd/nfsfh.c50
1 files changed, 21 insertions, 29 deletions
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index d0d8a21..a77efb8 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -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.
- */
- error = nfsd_setuser_and_check_port(rqstp, exp);
- if (error)
- goto out;
}
+ dentry = fhp->fh_dentry;
+ exp = fhp->fh_export;
+ /*
+ * 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)
OpenPOWER on IntegriCloud