From dae8d54f0708cf191cbb06ef6aa43cd570ceea90 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 2 May 2009 10:49:58 -0700 Subject: [PATCH] Fix sticky bit directory handling for deletions. Properly check the sticky bit and disallow deletions if set on a directory and the user doing the deletion does not own the directory or the file. This check is now being done by the kernel layer, VFSs do not need to do this check any more. Reported-by: YONETANI Tomokazu --- sys/kern/vfs_nlookup.c | 63 ++++++++++++++++++++++++++++++++++++++++++++------ sys/sys/nlookup.h | 3 +-- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/sys/kern/vfs_nlookup.c b/sys/kern/vfs_nlookup.c index 967ad655ad..886109f793 100644 --- a/sys/kern/vfs_nlookup.c +++ b/sys/kern/vfs_nlookup.c @@ -70,6 +70,8 @@ #include #endif +static int naccess_va(struct vattr *va, int vmode, struct ucred *cred); + /* * Initialize a nlookup() structure, early error return for copyin faults * or a degenerate empty string (which is not allowed). @@ -362,7 +364,7 @@ nlookup(struct nlookupdata *nd) /* * Check directory search permissions. */ - if ((error = naccess(&nd->nl_nch, VEXEC, nd->nl_cred)) != 0) + if ((error = naccess(&nd->nl_nch, VEXEC, nd->nl_cred, NULL)) != 0) break; /* @@ -459,7 +461,7 @@ nlookup(struct nlookupdata *nd) if (nd->nl_flags & NLC_NFS_RDONLY) error = EROFS; else - error = naccess(&nch, VCREATE, nd->nl_cred); + error = naccess(&nch, VCREATE, nd->nl_cred, NULL); } if (error == 0 && wasdotordotdot && (nd->nl_flags & NLC_DELETE)) error = EINVAL; @@ -590,7 +592,7 @@ nlookup(struct nlookupdata *nd) * Check directory permissions if a deletion is specified. */ if (*ptr == 0 && (nd->nl_flags & NLC_DELETE)) { - if ((error = naccess(&nch, VDELETE, nd->nl_cred)) != 0) { + if ((error = naccess(&nch, VDELETE, nd->nl_cred, NULL)) != 0) { cache_put(&nch); break; } @@ -720,17 +722,21 @@ fail: * ENOENT is returned. Note that nlookup() does not (and should not) return * ENOENT for non-existant leafs. * + * If VDELETE is specified we also do sticky-bit tests on the parent + * directory. + * * The passed ncp may or may not be locked. The caller should use a * locked ncp on leaf lookups, especially for VCREATE, VDELETE, and VEXCL * checks. */ int -naccess(struct nchandle *nch, int vmode, struct ucred *cred) +naccess(struct nchandle *nch, int vmode, struct ucred *cred, int *stickyp) { struct nchandle par; struct vnode *vp; struct vattr va; int error; + int sticky; if (nch->ncp->nc_flag & NCF_UNRESOLVED) { cache_lock(nch); @@ -738,6 +744,8 @@ naccess(struct nchandle *nch, int vmode, struct ucred *cred) cache_unlock(nch); } error = nch->ncp->nc_error; + sticky = 0; + if (vmode & (VDELETE|VCREATE|VEXCL)) { if (((vmode & VCREATE) && nch->ncp->nc_vp == NULL) || ((vmode & VDELETE) && nch->ncp->nc_vp != NULL) @@ -748,7 +756,9 @@ naccess(struct nchandle *nch, int vmode, struct ucred *cred) } else { par.mount = nch->mount; cache_hold(&par); - error = naccess(&par, VWRITE, cred); + error = naccess(&par, VWRITE, cred, &sticky); + if ((vmode & VDELETE) && sticky) + vmode |= VSVTX; cache_drop(&par); } } @@ -769,8 +779,23 @@ naccess(struct nchandle *nch, int vmode, struct ucred *cred) } } vput(vp); - if (error == 0) + + if (error == 0) { + /* + * Set the returned (*stickyp) if VSVTX is set and the uid + * is not the owner of the directory. The caller uses this + * disallow deletions of files not owned by the user if the + * user also does not own the directory and the sticky bit + * is set on the directory. Weird, I know. + */ + if (stickyp && va.va_uid != cred->cr_uid) + *stickyp = (va.va_mode & VSVTX); + + /* + * Process general access. + */ error = naccess_va(&va, vmode, cred); + } } } return(error); @@ -779,6 +804,7 @@ naccess(struct nchandle *nch, int vmode, struct ucred *cred) /* * Check the requested access against the given vattr using cred. */ +static int naccess_va(struct vattr *va, int vmode, struct ucred *cred) { @@ -801,7 +827,10 @@ naccess_va(struct vattr *va, int vmode, struct ucred *cred) return(0); /* - * Check owner perms, group perms, and world perms + * Check owner perms. + * + * If VOWN is set the owner of the file is allowed no matter when + * the owner-mode bits say (utimes). */ if (cred->cr_uid == va->va_uid) { if ((vmode & VOWN) == 0) { @@ -811,6 +840,23 @@ naccess_va(struct vattr *va, int vmode, struct ucred *cred) } return(0); } + + /* + * If VSVTX is set only the owner may access the file. This bit is + * typically set for VDELETE checks on files in sticky directories + * when the user does not own the directory. Note that other V bits + * are not typically set (for deletions we usually just care about + * the directory permissions, not the file permissions). + * + * The effect on VDELETE checks is thus to not allow the deletion + * of the file, and otherwise allow it. + */ + if (vmode & VSVTX) + return(EACCES); + + /* + * Check group perms + */ vmode &= S_IRWXU; vmode >>= 3; for (i = 0; i < cred->cr_ngroups; ++i) { @@ -821,6 +867,9 @@ naccess_va(struct vattr *va, int vmode, struct ucred *cred) } } + /* + * Check world perms + */ vmode >>= 3; if ((vmode & va->va_mode) != vmode) return(EACCES); diff --git a/sys/sys/nlookup.h b/sys/sys/nlookup.h index 9def8fb56e..085d15c57f 100644 --- a/sys/sys/nlookup.h +++ b/sys/sys/nlookup.h @@ -120,8 +120,7 @@ int nlookup_mp(struct mount *mp, struct nchandle *nch); int nlookup(struct nlookupdata *); int nreadsymlink(struct nlookupdata *nd, struct nchandle *nch, struct nlcomponent *nlc); -int naccess(struct nchandle *nch, int vmode, struct ucred *cred); -int naccess_va(struct vattr *va, int vmode, struct ucred *cred); +int naccess(struct nchandle *nch, int vmode, struct ucred *cred, int *stickyp); #endif -- 2.11.4.GIT