From cb66845a55fd2b4f122f77e6dc648b44de8f4d74 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Wed, 26 Aug 2009 10:40:41 -0700 Subject: [PATCH] Kernel - fix access checks * VOP_ACCESS() is used for more then just access(). UFS and other filesystems (but not HAMMER) were calling it in the open/create/rename/ unlink paths. The uid/gid must be used in those cases, not the ruid/rgid. Add a VOP_EACCESS() macro which passes the appropriate flag to use the uid/gid instead of the ruid/rgid, and adjust the filesystems to use this macro. Reported-by: Stathis Kamperis --- sys/sys/vfsops.h | 5 +++++ sys/vfs/gnu/ext2fs/ext2_lookup.c | 6 +++--- sys/vfs/gnu/ext2fs/ext2_vfsops.c | 4 ++-- sys/vfs/gnu/ext2fs/ext2_vnops.c | 4 ++-- sys/vfs/hpfs/hpfs_vnops.c | 6 +++--- sys/vfs/isofs/cd9660/cd9660_vfsops.c | 2 +- sys/vfs/msdosfs/msdosfs_lookup.c | 6 +++--- sys/vfs/msdosfs/msdosfs_vfsops.c | 4 ++-- sys/vfs/msdosfs/msdosfs_vnops.c | 4 ++-- sys/vfs/nfs/nfs_serv.c | 2 +- sys/vfs/nwfs/nwfs_ioctl.c | 4 ++-- sys/vfs/nwfs/nwfs_vnops.c | 6 +++--- sys/vfs/smbfs/smbfs_vnops.c | 12 ++++++------ sys/vfs/udf/udf_vfsops.c | 2 +- sys/vfs/ufs/ffs_vfsops.c | 4 ++-- sys/vfs/ufs/ufs_lookup.c | 6 +++--- sys/vfs/ufs/ufs_vnops.c | 4 ++-- sys/vfs/union/union_subr.c | 2 +- 18 files changed, 44 insertions(+), 39 deletions(-) diff --git a/sys/sys/vfsops.h b/sys/sys/vfsops.h index 3717d0db6a..94d2b6f22c 100644 --- a/sys/sys/vfsops.h +++ b/sys/sys/vfsops.h @@ -70,6 +70,9 @@ #ifndef _SYS_BUF_H_ #include /* buf_cmd_t */ #endif +#ifndef _SYS_FCNTL_H_ +#include /* AT_EACCESS */ +#endif struct syslink_desc; struct vnode; @@ -976,6 +979,8 @@ extern struct syslink_desc vop_nrename_desc; vop_close(*(vp)->v_ops, vp, fflag) #define VOP_ACCESS(vp, mode, cred) \ vop_access(*(vp)->v_ops, vp, mode, 0, cred) +#define VOP_EACCESS(vp, mode, cred) \ + vop_access(*(vp)->v_ops, vp, mode, AT_EACCESS, cred) #define VOP_ACCESS_FLAGS(vp, mode, flags, cred) \ vop_access(*(vp)->v_ops, vp, mode, flags, cred) #define VOP_GETATTR(vp, vap) \ diff --git a/sys/vfs/gnu/ext2fs/ext2_lookup.c b/sys/vfs/gnu/ext2fs/ext2_lookup.c index e1b72d16aa..48fa130960 100644 --- a/sys/vfs/gnu/ext2fs/ext2_lookup.c +++ b/sys/vfs/gnu/ext2fs/ext2_lookup.c @@ -490,7 +490,7 @@ searchloop: * Access for write is interpreted as allowing * creation of files in the directory. */ - if ((error = VOP_ACCESS(vdp, VWRITE, cred)) != 0) + if ((error = VOP_EACCESS(vdp, VWRITE, cred)) != 0) return (error); /* * Return an indication of where the new directory @@ -566,7 +566,7 @@ found: /* * Write access to directory required to delete files. */ - if ((error = VOP_ACCESS(vdp, VWRITE, cred)) != 0) + if ((error = VOP_EACCESS(vdp, VWRITE, cred)) != 0) return (error); /* * Return pointer to current entry in dp->i_offset, @@ -611,7 +611,7 @@ found: * regular file, or empty directory. */ if (nameiop == NAMEI_RENAME && wantparent) { - if ((error = VOP_ACCESS(vdp, VWRITE, cred)) != 0) + if ((error = VOP_EACCESS(vdp, VWRITE, cred)) != 0) return (error); /* * Careful about locking second inode. diff --git a/sys/vfs/gnu/ext2fs/ext2_vfsops.c b/sys/vfs/gnu/ext2fs/ext2_vfsops.c index f2ece19103..b48a9fa131 100644 --- a/sys/vfs/gnu/ext2fs/ext2_vfsops.c +++ b/sys/vfs/gnu/ext2fs/ext2_vfsops.c @@ -325,7 +325,7 @@ ext2_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) */ if (cred->cr_uid != 0) { vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - error = VOP_ACCESS(devvp, VREAD | VWRITE, cred); + error = VOP_EACCESS(devvp, VREAD | VWRITE, cred); if (error) { vn_unlock(devvp); return (error); @@ -389,7 +389,7 @@ ext2_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) if ((mp->mnt_flag & MNT_RDONLY) == 0) accessmode |= VWRITE; vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - if ((error = VOP_ACCESS(devvp, accessmode, cred)) != 0) { + if ((error = VOP_EACCESS(devvp, accessmode, cred)) != 0) { vput(devvp); return (error); } diff --git a/sys/vfs/gnu/ext2fs/ext2_vnops.c b/sys/vfs/gnu/ext2fs/ext2_vnops.c index 0e71be7145..1b7fa73e03 100644 --- a/sys/vfs/gnu/ext2fs/ext2_vnops.c +++ b/sys/vfs/gnu/ext2fs/ext2_vnops.c @@ -517,7 +517,7 @@ abortit: * to namei, as the parent directory is unlocked by the * call to checkpath(). */ - error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred); + error = VOP_EACCESS(fvp, VWRITE, tcnp->cn_cred); vn_unlock(fvp); /* @@ -1488,7 +1488,7 @@ ext2_setattr(struct vop_setattr_args *ap) if (cred->cr_uid != ip->i_uid && (error = priv_check_cred(cred, PRIV_VFS_SETATTR, 0)) && ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || - (error = VOP_ACCESS(vp, VWRITE, cred)))) + (error = VOP_EACCESS(vp, VWRITE, cred)))) return (error); if (vap->va_atime.tv_sec != VNOVAL) ip->i_flag |= IN_ACCESS; diff --git a/sys/vfs/hpfs/hpfs_vnops.c b/sys/vfs/hpfs/hpfs_vnops.c index 8ca0b41d15..9dbcb3e490 100644 --- a/sys/vfs/hpfs/hpfs_vnops.c +++ b/sys/vfs/hpfs/hpfs_vnops.c @@ -533,7 +533,7 @@ hpfs_setattr(struct vop_setattr_args *ap) if (cred->cr_uid != hp->h_uid && (error = priv_check_cred(cred, PRIV_VFS_SETATTR, 0)) && ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || - (error = VOP_ACCESS(vp, VWRITE, cred)))) + (error = VOP_EACCESS(vp, VWRITE, cred)))) return (error); if (vap->va_atime.tv_sec != VNOVAL) hp->h_atime = vap->va_atime.tv_sec; @@ -982,7 +982,7 @@ hpfs_lookup(struct vop_old_lookup_args *ap) return (EOPNOTSUPP); } - error = VOP_ACCESS(dvp, VEXEC, cred); + error = VOP_EACCESS(dvp, VEXEC, cred); if(error) return (error); @@ -1035,7 +1035,7 @@ hpfs_lookup(struct vop_old_lookup_args *ap) dep->de_fnode, dep->de_cpid)); if (nameiop == NAMEI_DELETE) { - error = VOP_ACCESS(dvp, VWRITE, cred); + error = VOP_EACCESS(dvp, VWRITE, cred); if (error) { brelse(bp); return (error); diff --git a/sys/vfs/isofs/cd9660/cd9660_vfsops.c b/sys/vfs/isofs/cd9660/cd9660_vfsops.c index bfa2967c7f..cfab5def8a 100644 --- a/sys/vfs/isofs/cd9660/cd9660_vfsops.c +++ b/sys/vfs/isofs/cd9660/cd9660_vfsops.c @@ -232,7 +232,7 @@ cd9660_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) */ accessmode = VREAD; vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - error = VOP_ACCESS(devvp, accessmode, cred); + error = VOP_EACCESS(devvp, accessmode, cred); if (error) error = priv_check_cred(cred, PRIV_ROOT, 0); if (error) { diff --git a/sys/vfs/msdosfs/msdosfs_lookup.c b/sys/vfs/msdosfs/msdosfs_lookup.c index 729dcdd8fc..01cb43d7e3 100644 --- a/sys/vfs/msdosfs/msdosfs_lookup.c +++ b/sys/vfs/msdosfs/msdosfs_lookup.c @@ -337,7 +337,7 @@ notfound: * Access for write is interpreted as allowing * creation of files in the directory. */ - error = VOP_ACCESS(vdp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(vdp, VWRITE, cnp->cn_cred); if (error) return (error); /* @@ -429,7 +429,7 @@ foundroot: /* * Write access to directory required to delete files. */ - error = VOP_ACCESS(vdp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(vdp, VWRITE, cnp->cn_cred); if (error) return (error); @@ -463,7 +463,7 @@ foundroot: if (blkoff == MSDOSFSROOT_OFS) return EROFS; /* really? XXX */ - error = VOP_ACCESS(vdp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(vdp, VWRITE, cnp->cn_cred); if (error) return (error); diff --git a/sys/vfs/msdosfs/msdosfs_vfsops.c b/sys/vfs/msdosfs/msdosfs_vfsops.c index 8a39bb2bee..7cdc8cec12 100644 --- a/sys/vfs/msdosfs/msdosfs_vfsops.c +++ b/sys/vfs/msdosfs/msdosfs_vfsops.c @@ -201,7 +201,7 @@ msdosfs_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) devvp = pmp->pm_devvp; vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); if (cred->cr_uid != 0) { - error = VOP_ACCESS(devvp, VREAD | VWRITE, cred); + error = VOP_EACCESS(devvp, VREAD | VWRITE, cred); if (error) { vn_unlock(devvp); return (error); @@ -254,7 +254,7 @@ msdosfs_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) if ((mp->mnt_flag & MNT_RDONLY) == 0) accessmode |= VWRITE; vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - error = VOP_ACCESS(devvp, accessmode, cred); + error = VOP_EACCESS(devvp, accessmode, cred); if (error) { vput(devvp); return (error); diff --git a/sys/vfs/msdosfs/msdosfs_vnops.c b/sys/vfs/msdosfs/msdosfs_vnops.c index 7b7f76c6de..fdb549c69a 100644 --- a/sys/vfs/msdosfs/msdosfs_vnops.c +++ b/sys/vfs/msdosfs/msdosfs_vnops.c @@ -434,7 +434,7 @@ msdosfs_setattr(struct vop_setattr_args *ap) if (cred->cr_uid != pmp->pm_uid && (error = priv_check_cred(cred, PRIV_VFS_SETATTR, 0)) && ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || - (error = VOP_ACCESS(ap->a_vp, VWRITE, cred)))) + (error = VOP_EACCESS(ap->a_vp, VWRITE, cred)))) return (error); if (vp->v_type != VDIR) { if ((pmp->pm_flags & MSDOSFSMNT_NOWIN95) == 0 && @@ -1023,7 +1023,7 @@ abortit: * to namei, as the parent directory is unlocked by the * call to doscheckpath(). */ - error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred); + error = VOP_EACCESS(fvp, VWRITE, tcnp->cn_cred); vn_unlock(fvp); if (VTODE(fdvp)->de_StartCluster != VTODE(tdvp)->de_StartCluster) newparent = 1; diff --git a/sys/vfs/nfs/nfs_serv.c b/sys/vfs/nfs/nfs_serv.c index 6d2104251d..86a4b0c3d1 100644 --- a/sys/vfs/nfs/nfs_serv.c +++ b/sys/vfs/nfs/nfs_serv.c @@ -3984,7 +3984,7 @@ nfsrv_access(struct mount *mp, struct vnode *vp, int flags, struct ucred *cred, error = VOP_GETATTR(vp, &vattr); if (error) return (error); - error = VOP_ACCESS(vp, flags, cred); + error = VOP_ACCESS(vp, flags, cred); /* XXX ruid/rgid vs uid/gid */ /* * Allow certain operations for the owner (reads and writes * on files that are already open). diff --git a/sys/vfs/nwfs/nwfs_ioctl.c b/sys/vfs/nwfs/nwfs_ioctl.c index 5410ce5d04..5dd542ceb7 100644 --- a/sys/vfs/nwfs/nwfs_ioctl.c +++ b/sys/vfs/nwfs/nwfs_ioctl.c @@ -78,7 +78,7 @@ nwfs_ioctl(struct vop_ioctl_args *ap) *(int*)data = hp->nh_id; break; case NWFSIOC_GETEINFO: - if ((error = VOP_ACCESS(vp, VEXEC, cred))) break; + if ((error = VOP_EACCESS(vp, VEXEC, cred))) break; fap = data; error = ncp_obtain_info(nmp, np->n_fid.f_id, 0, NULL, fap, td, ap->a_cred); @@ -86,7 +86,7 @@ nwfs_ioctl(struct vop_ioctl_args *ap) fap->nameLen = np->n_nmlen; break; case NWFSIOC_GETNS: - if ((error = VOP_ACCESS(vp, VEXEC, cred))) break; + if ((error = VOP_EACCESS(vp, VEXEC, cred))) break; *(int*)data = nmp->name_space; break; default: diff --git a/sys/vfs/nwfs/nwfs_vnops.c b/sys/vfs/nwfs/nwfs_vnops.c index 03ffb83314..dc068b6ffe 100644 --- a/sys/vfs/nwfs/nwfs_vnops.c +++ b/sys/vfs/nwfs/nwfs_vnops.c @@ -832,7 +832,7 @@ nwfs_lookup(struct vop_old_lookup_args *ap) if ((mp->mnt_flag & MNT_RDONLY) && nameiop != NAMEI_LOOKUP) return (EROFS); - if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred))) + if ((error = VOP_EACCESS(dvp, VEXEC, cnp->cn_cred))) return (error); lockparent = flags & CNP_LOCKPARENT; wantparent = flags & (CNP_LOCKPARENT | CNP_WANTPARENT); @@ -891,7 +891,7 @@ kprintf("dvp %d:%d:%d\n", (int)mp, (int)dvp->v_flag & VROOT, (int)flags & CNP_IS }*/ /* handle DELETE case ... */ if (nameiop == NAMEI_DELETE) { /* delete last component */ - error = VOP_ACCESS(dvp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(dvp, VWRITE, cnp->cn_cred); if (error) return (error); if (NWCMPF(&dnp->n_fid, &fid)) { /* we found ourselfs */ vref(dvp); @@ -905,7 +905,7 @@ kprintf("dvp %d:%d:%d\n", (int)mp, (int)dvp->v_flag & VROOT, (int)flags & CNP_IS return (0); } if (nameiop == NAMEI_RENAME && wantparent) { - error = VOP_ACCESS(dvp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(dvp, VWRITE, cnp->cn_cred); if (error) return (error); if (NWCMPF(&dnp->n_fid, &fid)) return EISDIR; error = nwfs_nget(mp, fid, fap, dvp, &vp); diff --git a/sys/vfs/smbfs/smbfs_vnops.c b/sys/vfs/smbfs/smbfs_vnops.c index 6695918727..d0f7075b6c 100644 --- a/sys/vfs/smbfs/smbfs_vnops.c +++ b/sys/vfs/smbfs/smbfs_vnops.c @@ -355,7 +355,7 @@ smbfs_setattr(struct vop_setattr_args *ap) if (ap->a_cred->cr_uid != VTOSMBFS(vp)->sm_args.uid && (error = priv_check_cred(ap->a_cred, PRIV_VFS_SETATTR, 0)) && ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || - (error = VOP_ACCESS(vp, VWRITE, ap->a_cred)))) + (error = VOP_EACCESS(vp, VWRITE, ap->a_cred)))) return (error); #if 0 if (mtime == NULL) @@ -841,7 +841,7 @@ smbfs_getextattr(struct vop_getextattr_args *ap) char buf[10]; int i, attr, error; - error = VOP_ACCESS(vp, VREAD, cred); + error = VOP_EACCESS(vp, VREAD, cred); if (error) return error; error = VOP_GETATTR(vp, &vattr); @@ -1039,7 +1039,7 @@ smbfs_lookup(struct vop_old_lookup_args *ap) #endif if ((mp->mnt_flag & MNT_RDONLY) && nameiop != NAMEI_LOOKUP) return EROFS; - if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred)) != 0) + if ((error = VOP_EACCESS(dvp, VEXEC, cnp->cn_cred)) != 0) return error; lockparent = flags & CNP_LOCKPARENT; wantparent = flags & (CNP_LOCKPARENT | CNP_WANTPARENT); @@ -1072,7 +1072,7 @@ smbfs_lookup(struct vop_old_lookup_args *ap) * Handle RENAME or CREATE case... */ if ((nameiop == NAMEI_CREATE || nameiop == NAMEI_RENAME) && wantparent) { - error = VOP_ACCESS(dvp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(dvp, VWRITE, cnp->cn_cred); if (error) return error; if (!lockparent) { @@ -1089,7 +1089,7 @@ smbfs_lookup(struct vop_old_lookup_args *ap) * handle DELETE case ... */ if (nameiop == NAMEI_DELETE) { /* delete last component */ - error = VOP_ACCESS(dvp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(dvp, VWRITE, cnp->cn_cred); if (error) return error; if (isdot) { @@ -1108,7 +1108,7 @@ smbfs_lookup(struct vop_old_lookup_args *ap) return 0; } if (nameiop == NAMEI_RENAME && wantparent) { - error = VOP_ACCESS(dvp, VWRITE, cnp->cn_cred); + error = VOP_EACCESS(dvp, VWRITE, cnp->cn_cred); if (error) return error; if (isdot) diff --git a/sys/vfs/udf/udf_vfsops.c b/sys/vfs/udf/udf_vfsops.c index ebe6b2bd37..50db59c017 100644 --- a/sys/vfs/udf/udf_vfsops.c +++ b/sys/vfs/udf/udf_vfsops.c @@ -173,7 +173,7 @@ udf_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) /* Check the access rights on the mount device */ vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - error = VOP_ACCESS(devvp, VREAD, cred); + error = VOP_EACCESS(devvp, VREAD, cred); if (error) error = priv_check_cred(cred, PRIV_ROOT, 0); if (error) { diff --git a/sys/vfs/ufs/ffs_vfsops.c b/sys/vfs/ufs/ffs_vfsops.c index 13f5f6d29e..45820fa0ce 100644 --- a/sys/vfs/ufs/ffs_vfsops.c +++ b/sys/vfs/ufs/ffs_vfsops.c @@ -223,7 +223,7 @@ ffs_mount(struct mount *mp, /* mount struct pointer */ */ if (cred->cr_uid != 0) { vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - if ((error = VOP_ACCESS(devvp, VREAD | VWRITE, + if ((error = VOP_EACCESS(devvp, VREAD | VWRITE, cred)) != 0) { vn_unlock(devvp); return (error); @@ -302,7 +302,7 @@ ffs_mount(struct mount *mp, /* mount struct pointer */ if ((mp->mnt_flag & MNT_RDONLY) == 0) accessmode |= VWRITE; vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - if ((error = VOP_ACCESS(devvp, accessmode, cred)) != 0) { + if ((error = VOP_EACCESS(devvp, accessmode, cred)) != 0) { vput(devvp); return (error); } diff --git a/sys/vfs/ufs/ufs_lookup.c b/sys/vfs/ufs/ufs_lookup.c index a6bbafea0d..df7af7fe9c 100644 --- a/sys/vfs/ufs/ufs_lookup.c +++ b/sys/vfs/ufs/ufs_lookup.c @@ -381,7 +381,7 @@ notfound: * Access for write is interpreted as allowing * creation of files in the directory. */ - error = VOP_ACCESS(vdp, VWRITE, cred); + error = VOP_EACCESS(vdp, VWRITE, cred); if (error) return (error); /* @@ -465,7 +465,7 @@ found: /* * Write access to directory required to delete files. */ - error = VOP_ACCESS(vdp, VWRITE, cred); + error = VOP_EACCESS(vdp, VWRITE, cred); if (error) return (error); /* @@ -520,7 +520,7 @@ found: * regular file, or empty directory. */ if (nameiop == NAMEI_RENAME && wantparent) { - if ((error = VOP_ACCESS(vdp, VWRITE, cred)) != 0) + if ((error = VOP_EACCESS(vdp, VWRITE, cred)) != 0) return (error); /* * Careful about locking second inode. diff --git a/sys/vfs/ufs/ufs_vnops.c b/sys/vfs/ufs/ufs_vnops.c index 179b603329..39cfa639fa 100644 --- a/sys/vfs/ufs/ufs_vnops.c +++ b/sys/vfs/ufs/ufs_vnops.c @@ -472,7 +472,7 @@ ufs_setattr(struct vop_setattr_args *ap) if (cred->cr_uid != ip->i_uid && (error = priv_check_cred(cred, PRIV_VFS_SETATTR, 0)) && ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || - (error = VOP_ACCESS(vp, VWRITE, cred)))) + (error = VOP_EACCESS(vp, VWRITE, cred)))) return (error); if (vap->va_atime.tv_sec != VNOVAL) ip->i_flag |= IN_ACCESS; @@ -952,7 +952,7 @@ abortit: * to namei, as the parent directory is unlocked by the * call to checkpath(). */ - error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred); + error = VOP_EACCESS(fvp, VWRITE, tcnp->cn_cred); vn_unlock(fvp); /* diff --git a/sys/vfs/union/union_subr.c b/sys/vfs/union/union_subr.c index 5833a14588..155206514e 100644 --- a/sys/vfs/union/union_subr.c +++ b/sys/vfs/union/union_subr.c @@ -725,7 +725,7 @@ union_copyup(struct union_node *un, int docopy, struct ucred *cred, * be copied to upper layer. */ vn_lock(un->un_lowervp, LK_EXCLUSIVE | LK_RETRY); - error = VOP_ACCESS(un->un_lowervp, VREAD, cred); + error = VOP_EACCESS(un->un_lowervp, VREAD, cred); vn_unlock(un->un_lowervp); if (error) return (error); -- 2.11.4.GIT