From owner-freebsd-bugs Fri Dec 26 04:20:11 1997 Return-Path: Received: (from root@localhost) by hub.freebsd.org (8.8.7/8.8.7) id EAA09133 for bugs-outgoing; Fri, 26 Dec 1997 04:20:11 -0800 (PST) (envelope-from owner-freebsd-bugs) Received: (from gnats@localhost) by hub.freebsd.org (8.8.7/8.8.7) id EAA09122; Fri, 26 Dec 1997 04:20:02 -0800 (PST) (envelope-from gnats) Date: Fri, 26 Dec 1997 04:20:02 -0800 (PST) Message-Id: <199712261220.EAA09122@hub.freebsd.org> To: freebsd-bugs Cc: From: MIHIRA "Sanpei" Yoshiro Subject: Re: bin/5148: mode of file and access on NFS mounted partitions Reply-To: MIHIRA "Sanpei" Yoshiro Sender: owner-freebsd-bugs@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk The following reply was made to PR bin/5148; it has been noted by GNATS. From: MIHIRA "Sanpei" Yoshiro To: freebsd-gnats-submit@freebsd.org, Joel.Faedi@esial.u-nancy.fr, dfr@freebsd.org Cc: Subject: Re: bin/5148: mode of file and access on NFS mounted partitions Date: Fri, 26 Dec 1997 21:10:12 +0900 I reviewd related sources on NetBSD/OpenBSD. In OpenBSD CVS[1], sys/nfs/nfs_serv.c rev.1.8 and rev.1.9. and nfs_serv.c in NetBSD-current[2], is for this problem. I made patch for FreeBSD-current, FreeBSD-stable source from these changes in Net/OpenBSD. By the way, nfs_serv.c is different between NetBSD and OpenBSD. NetBSD check only NACCES, but OpenBSD check NACCES and EPERM. I think VOP_ACCESS function is only returned EACCESS(sys/ufs/ufs/ufs_vnops.c:ufs_access function.) In my patch, I only check NACCESS. I hope to merge these. Thank you. --- Yoshiro MIHIRA Dr. Candidate, Yamamoto Lab. Department of Computer Science Keio University. Yokohama, Japan [1] OpenBSD CVS repository http://www.openbsd.org/cgi-bin/cvsweb/src/sys/nfs/nfs_serv.c [2] NetBSD source tree ftp://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/nfs for FreeBSD-current --- sys/nfs/nfs_serv.c.orig Wed Dec 17 18:11:16 1997 +++ sys/nfs/nfs_serv.c Mon Dec 22 19:53:28 1997 @@ -101,7 +101,7 @@ SYSCTL_INT(_vfs_nfs, OID_AUTO, async, CTLFLAG_RW, &nfs_async, 0, ""); static int nfsrv_access __P((struct vnode *,int,struct ucred *,int, - struct proc *)); + struct proc *, int)); static void nfsrvw_coalesce __P((struct nfsrv_descript *, struct nfsrv_descript *)); @@ -146,7 +146,7 @@ } nfsmode = fxdr_unsigned(u_long, *tl); if ((nfsmode & NFSV3ACCESS_READ) && - nfsrv_access(vp, VREAD, cred, rdonly, procp)) + nfsrv_access(vp, VREAD, cred, rdonly, procp, 0)) nfsmode &= ~NFSV3ACCESS_READ; if (vp->v_type == VDIR) testmode = (NFSV3ACCESS_MODIFY | NFSV3ACCESS_EXTEND | @@ -154,14 +154,14 @@ else testmode = (NFSV3ACCESS_MODIFY | NFSV3ACCESS_EXTEND); if ((nfsmode & testmode) && - nfsrv_access(vp, VWRITE, cred, rdonly, procp)) + nfsrv_access(vp, VWRITE, cred, rdonly, procp, 0)) nfsmode &= ~testmode; if (vp->v_type == VDIR) testmode = NFSV3ACCESS_LOOKUP; else testmode = NFSV3ACCESS_EXECUTE; if ((nfsmode & testmode) && - nfsrv_access(vp, VEXEC, cred, rdonly, procp)) + nfsrv_access(vp, VEXEC, cred, rdonly, procp, 0)) nfsmode &= ~testmode; getret = VOP_GETATTR(vp, vap, cred, procp); vput(vp); @@ -329,7 +329,7 @@ error = EISDIR; goto out; } else if (error = nfsrv_access(vp, VWRITE, cred, rdonly, - procp)) + procp, 0)) goto out; } error = VOP_SETATTR(vp, vap, cred, procp); @@ -632,8 +632,8 @@ } if (!error) { nqsrv_getl(vp, ND_READ); - if (error = nfsrv_access(vp, VREAD, cred, rdonly, procp)) - error = nfsrv_access(vp, VEXEC, cred, rdonly, procp); + if (error = nfsrv_access(vp, VREAD, cred, rdonly, procp, 1)) + error = nfsrv_access(vp, VEXEC, cred, rdonly, procp, 1); } getret = VOP_GETATTR(vp, vap, cred, procp); if (!error) @@ -848,7 +848,7 @@ } if (!error) { nqsrv_getl(vp, ND_WRITE); - error = nfsrv_access(vp, VWRITE, cred, rdonly, procp); + error = nfsrv_access(vp, VWRITE, cred, rdonly, procp, 1); } if (error) { vput(vp); @@ -1125,7 +1125,7 @@ vp = NULL; if (!error) { nqsrv_getl(vp, ND_WRITE); - error = nfsrv_access(vp, VWRITE, cred, rdonly, procp); + error = nfsrv_access(vp, VWRITE, cred, rdonly, procp, 1); } if (nfsd->nd_stable == NFSV3WRITE_UNSTABLE) @@ -1523,7 +1523,7 @@ VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd); if (vap->va_size != -1) { error = nfsrv_access(vp, VWRITE, cred, - (nd.ni_cnd.cn_flags & RDONLY), procp); + (nd.ni_cnd.cn_flags & RDONLY), procp, 0); if (!error) { nqsrv_getl(vp, ND_WRITE); tempsize = vap->va_size; @@ -2579,7 +2579,7 @@ error = NFSERR_BAD_COOKIE; } if (!error) - error = nfsrv_access(vp, VEXEC, cred, rdonly, procp); + error = nfsrv_access(vp, VEXEC, cred, rdonly, procp, 0); if (error) { vput(vp); nfsm_reply(NFSX_POSTOPATTR(v3)); @@ -2832,7 +2832,7 @@ error = NFSERR_BAD_COOKIE; if (!error) { nqsrv_getl(vp, ND_READ); - error = nfsrv_access(vp, VEXEC, cred, rdonly, procp); + error = nfsrv_access(vp, VEXEC, cred, rdonly, procp, 0); } if (error) { vput(vp); @@ -3412,18 +3412,20 @@ * refer to files already opened by a Unix client. You cannot just use * vn_writechk() and VOP_ACCESS() for two reasons. * 1 - You must check for exported rdonly as well as MNT_RDONLY for the write case - * 2 - The owner is to be given access irrespective of mode bits so that - * processes that chmod after opening a file don't break. I don't like - * this because it opens a security hole, but since the nfs server opens - * a security hole the size of a barn door anyhow, what the heck. + * 2 - The owner is to be given access irrespective of mode bits for some + * operations, so that processes that chmod after opening a file don't + * break. I don't like this because it opens a security hole, but since + * the nfs server opens a security hole the size of a barn door anyhow, + * what the heck. */ static int -nfsrv_access(vp, flags, cred, rdonly, p) +nfsrv_access(vp, flags, cred, rdonly, p, override) register struct vnode *vp; int flags; register struct ucred *cred; int rdonly; struct proc *p; + int override; { struct vattr vattr; int error; @@ -3449,10 +3451,14 @@ } if (error = VOP_GETATTR(vp, &vattr, cred, p)) return (error); - if ((error = VOP_ACCESS(vp, flags, cred, p)) && - cred->cr_uid != vattr.va_uid) - return (error); - return (0); + error = VOP_ACCESS(vp, flags, cred, p); + /* + * Allow certain operations for the owner (reads and writes + * on files that are already open). + */ + if (override && error == EACCES && cred->cr_uid == vattr.va_uid) + error = 0; + return error; } #endif /* NFS_NOSERVER */ ---------- for FreeBSD-stable --- sys/nfs/nfs_serv.c.orig Tue Dec 16 18:04:11 1997 +++ sys/nfs/nfs_serv.c Mon Dec 22 19:54:13 1997 @@ -103,7 +103,7 @@ SYSCTL_INT(_vfs_nfs, OID_AUTO, async, CTLFLAG_RW, &nfs_async, 0, ""); static int nfsrv_access __P((struct vnode *,int,struct ucred *,int, - struct proc *)); + struct proc *, int)); static void nfsrvw_coalesce __P((struct nfsrv_descript *, struct nfsrv_descript *)); @@ -148,7 +148,7 @@ } nfsmode = fxdr_unsigned(u_long, *tl); if ((nfsmode & NFSV3ACCESS_READ) && - nfsrv_access(vp, VREAD, cred, rdonly, procp)) + nfsrv_access(vp, VREAD, cred, rdonly, procp, 0)) nfsmode &= ~NFSV3ACCESS_READ; if (vp->v_type == VDIR) testmode = (NFSV3ACCESS_MODIFY | NFSV3ACCESS_EXTEND | @@ -156,14 +156,14 @@ else testmode = (NFSV3ACCESS_MODIFY | NFSV3ACCESS_EXTEND); if ((nfsmode & testmode) && - nfsrv_access(vp, VWRITE, cred, rdonly, procp)) + nfsrv_access(vp, VWRITE, cred, rdonly, procp, 0)) nfsmode &= ~testmode; if (vp->v_type == VDIR) testmode = NFSV3ACCESS_LOOKUP; else testmode = NFSV3ACCESS_EXECUTE; if ((nfsmode & testmode) && - nfsrv_access(vp, VEXEC, cred, rdonly, procp)) + nfsrv_access(vp, VEXEC, cred, rdonly, procp, 0)) nfsmode &= ~testmode; getret = VOP_GETATTR(vp, vap, cred, procp); vput(vp); @@ -331,7 +331,7 @@ error = EISDIR; goto out; } else if (error = nfsrv_access(vp, VWRITE, cred, rdonly, - procp)) + procp, 0)) goto out; } error = VOP_SETATTR(vp, vap, cred, procp); @@ -588,8 +588,8 @@ } if (!error) { nqsrv_getl(vp, ND_READ); - if (error = nfsrv_access(vp, VREAD, cred, rdonly, procp)) - error = nfsrv_access(vp, VEXEC, cred, rdonly, procp); + if (error = nfsrv_access(vp, VREAD, cred, rdonly, procp, 1)) + error = nfsrv_access(vp, VEXEC, cred, rdonly, procp, 1); } getret = VOP_GETATTR(vp, vap, cred, procp); if (!error) @@ -804,7 +804,7 @@ } if (!error) { nqsrv_getl(vp, ND_WRITE); - error = nfsrv_access(vp, VWRITE, cred, rdonly, procp); + error = nfsrv_access(vp, VWRITE, cred, rdonly, procp, 1); } if (error) { vput(vp); @@ -1078,7 +1078,7 @@ vp = NULL; if (!error) { nqsrv_getl(vp, ND_WRITE); - error = nfsrv_access(vp, VWRITE, cred, rdonly, procp); + error = nfsrv_access(vp, VWRITE, cred, rdonly, procp, 1); } if (nfsd->nd_stable == NFSV3WRITE_UNSTABLE) @@ -1476,7 +1476,7 @@ VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd); if (vap->va_size != -1) { error = nfsrv_access(vp, VWRITE, cred, - (nd.ni_cnd.cn_flags & RDONLY), procp); + (nd.ni_cnd.cn_flags & RDONLY), procp, 0); if (!error) { nqsrv_getl(vp, ND_WRITE); tempsize = vap->va_size; @@ -2533,7 +2533,7 @@ error = NFSERR_BAD_COOKIE; } if (!error) - error = nfsrv_access(vp, VEXEC, cred, rdonly, procp); + error = nfsrv_access(vp, VEXEC, cred, rdonly, procp, 0); if (error) { vput(vp); nfsm_reply(NFSX_POSTOPATTR(v3)); @@ -2799,7 +2799,7 @@ error = NFSERR_BAD_COOKIE; if (!error) { nqsrv_getl(vp, ND_READ); - error = nfsrv_access(vp, VEXEC, cred, rdonly, procp); + error = nfsrv_access(vp, VEXEC, cred, rdonly, procp, 0); } if (error) { vput(vp); @@ -3392,18 +3392,20 @@ * refer to files already opened by a Unix client. You cannot just use * vn_writechk() and VOP_ACCESS() for two reasons. * 1 - You must check for exported rdonly as well as MNT_RDONLY for the write case - * 2 - The owner is to be given access irrespective of mode bits so that - * processes that chmod after opening a file don't break. I don't like - * this because it opens a security hole, but since the nfs server opens - * a security hole the size of a barn door anyhow, what the heck. + * 2 - The owner is to be given access irrespective of mode bits for some + * operations, so that processes that chmod after opening a file don't + * break. I don't like this because it opens a security hole, but since + * the nfs server opens a security hole the size of a barn door anyhow, + * what the heck. */ static int -nfsrv_access(vp, flags, cred, rdonly, p) +nfsrv_access(vp, flags, cred, rdonly, p, override) register struct vnode *vp; int flags; register struct ucred *cred; int rdonly; struct proc *p; + int override; { struct vattr vattr; int error; @@ -3429,10 +3431,14 @@ } if (error = VOP_GETATTR(vp, &vattr, cred, p)) return (error); - if ((error = VOP_ACCESS(vp, flags, cred, p)) && - cred->cr_uid != vattr.va_uid) - return (error); - return (0); + error = VOP_ACCESS(vp, flags, cred, p); + /* + * Allow certain operations for the owner (reads and writes + * on files that are already open). + */ + if (override && error == EACCES && cred->cr_uid == vattr.va_uid) + error = 0; + return error; } #endif /* NFS_NOSERVER */