Re: svn commit: r194586 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs kern sys
On Mon, Jun 22, 2009 at 02:12:57PM +1000, Bruce Evans wrote: On Sun, 21 Jun 2009, Konstantin Belousov wrote: Log: Add another flags argument to vn_open_cred. Use it to specify that some vn_open_cred invocations shall not audit namei path. Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c == --- head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c Sun Jun 21 13:15:56 2009(r194585) +++ head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c Sun Jun 21 13:41:32 2009(r194586) @@ -85,7 +85,8 @@ kobj_open_file_vnode(const char *file) flags = FREAD; NDINIT(nd, LOOKUP, MPSAFE, UIO_SYSSPACE, file, td); -error = vn_open_cred(nd, flags, O_NOFOLLOW, curthread-td_ucred, NULL); +error = vn_open_cred(nd, flags, O_NOFOLLOW, 0, curthread-td_ucred, +NULL); I was going to ask why not put the flag in the existing flags arg, like O_NOFOLLOW here?, but it seems that there is no existing flags arg and the above O_NOFOLLOW is garbage. O_NOFOLLOW happens to be 0x100, so I think the above asks for mode S_IRUSR. I fixed this, O_NOFOLLOW is set in flags. Now I will ask why not put O_NOFOLLOW here and the new flag in the existing pointer-to-flags arg?. I do not quite understand what is named by here. O_NOFOLLOW is defined as a user-supplied flag for the mode argument of the open(2), that determines that it must go in the flags. I do not want to put kernel-only flags into the mode argument, at least because it shrinks the space available for further additions of the open(2) mode flags, that is periodically done by the standards freebsd tries to follow. Modified: head/sys/cddl/compat/opensolaris/sys/vnode.h == --- head/sys/cddl/compat/opensolaris/sys/vnode.h Sun Jun 21 13:15:56 2009 (r194585) +++ head/sys/cddl/compat/opensolaris/sys/vnode.h Sun Jun 21 13:41:32 2009 (r194586) @@ -182,7 +182,7 @@ vn_openat(char *pnamep, enum uio_seg seg vref(startvp); NDINIT_ATVP(nd, operation, MPSAFE, UIO_SYSSPACE, pnamep, startvp, td); filemode |= O_NOFOLLOW; -error = vn_open_cred(nd, filemode, createmode, td-td_ucred, NULL); +error = vn_open_cred(nd, filemode, createmode, 0, td-td_ucred, NULL); Here it does put O_NOFOLLOW in the existing pointer-to-flags arg. It obfuscates the open-flags variable by naming it filemode. Modified: head/sys/kern/vfs_vnops.c == --- head/sys/kern/vfs_vnops.cSun Jun 21 13:15:56 2009 (r194585) +++ head/sys/kern/vfs_vnops.cSun Jun 21 13:41:32 2009 (r194586) @@ -102,11 +102,8 @@ vn_open(ndp, flagp, cmode, fp) * due to the NDINIT being done elsewhere. */ int -vn_open_cred(ndp, flagp, cmode, cred, fp) -struct nameidata *ndp; -int *flagp, cmode; -struct ucred *cred; -struct file *fp; +vn_open_cred(struct nameidata *ndp, int *flagp, int cmode, u_int vn_open_flags, +struct ucred *cred, struct file *fp) { struct vnode *vp; struct mount *mp; @@ -124,9 +121,11 @@ restart: if (fmode O_CREAT) { Internally, flags are obfuscated by copying *flagp to the misnamed local variable fmode. The pointer-to-flags variable has about 12 spare bits in it. It already has just 1 kernel-only flag (O_HASLOCK, misnamed FHASLOCK and misassigned in the middle of the user flags). fcntl.h's list of open flags has been obfuscated by putting AT_ flags in the middle of the list. I moved the AT_* definitions in sys/fcntl.h after the list. pgpT9WKm1FonF.pgp Description: PGP signature
Re: svn commit: r194586 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs kern sys
On Mon, 22 Jun 2009, Kostik Belousov wrote: On Mon, Jun 22, 2009 at 02:12:57PM +1000, Bruce Evans wrote: On Sun, 21 Jun 2009, Konstantin Belousov wrote: Log: Add another flags argument to vn_open_cred. Use it to specify that some vn_open_cred invocations shall not audit namei path. Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c == --- head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.cSun Jun 21 13:15:56 2009 (r194585) +++ head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.cSun Jun 21 13:41:32 2009 (r194586) @@ -85,7 +85,8 @@ kobj_open_file_vnode(const char *file) flags = FREAD; NDINIT(nd, LOOKUP, MPSAFE, UIO_SYSSPACE, file, td); - error = vn_open_cred(nd, flags, O_NOFOLLOW, curthread-td_ucred, NULL); + error = vn_open_cred(nd, flags, O_NOFOLLOW, 0, curthread-td_ucred, + NULL); I was going to ask why not put the flag in the existing flags arg, like O_NOFOLLOW here?, but it seems that there is no existing flags arg and the above O_NOFOLLOW is garbage. O_NOFOLLOW happens to be 0x100, so I think the above asks for mode S_IRUSR. I fixed this, O_NOFOLLOW is set in flags. Now I will ask why not put O_NOFOLLOW here and the new flag in the existing pointer-to-flags arg?. I do not quite understand what is named by here. O_NOFOLLOW is defined as a user-supplied flag for the mode argument of the open(2), that determines that it must go in the flags. here is above. I do not want to put kernel-only flags into the mode argument, at least because it shrinks the space available for further additions of the open(2) mode flags, that is periodically done by the standards freebsd tries to follow. Not a problem, since the kernel flags can easily be moved later if necessary. Just don't repeat the mistake for F_HASLOCK, of putting them in the middle of user flags. Bruce ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
svn commit: r194586 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs kern sys
Author: kib Date: Sun Jun 21 13:41:32 2009 New Revision: 194586 URL: http://svn.freebsd.org/changeset/base/194586 Log: Add another flags argument to vn_open_cred. Use it to specify that some vn_open_cred invocations shall not audit namei path. In particular, specify VN_OPEN_NOAUDIT for dotdot lookup performed by default implementation of vop_vptocnp, and for the open done for core file. vn_fullpath is called from the audit code, and vn_open there need to disable audit to avoid infinite recursion. Core file is created on return to user mode, that, in particular, happens during syscall return. The creation of the core file is audited by direct calls, and we do not want to overwrite audit information for syscall. Reported, reviewed and tested by: rwatson Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c head/sys/cddl/compat/opensolaris/sys/vnode.h head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c head/sys/kern/kern_alq.c head/sys/kern/kern_sig.c head/sys/kern/vfs_default.c head/sys/kern/vfs_vnops.c head/sys/sys/vnode.h Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c == --- head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.cSun Jun 21 13:15:56 2009(r194585) +++ head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.cSun Jun 21 13:41:32 2009(r194586) @@ -85,7 +85,8 @@ kobj_open_file_vnode(const char *file) flags = FREAD; NDINIT(nd, LOOKUP, MPSAFE, UIO_SYSSPACE, file, td); - error = vn_open_cred(nd, flags, O_NOFOLLOW, curthread-td_ucred, NULL); + error = vn_open_cred(nd, flags, O_NOFOLLOW, 0, curthread-td_ucred, + NULL); NDFREE(nd, NDF_ONLY_PNBUF); if (error != 0) return (NULL); Modified: head/sys/cddl/compat/opensolaris/sys/vnode.h == --- head/sys/cddl/compat/opensolaris/sys/vnode.hSun Jun 21 13:15:56 2009(r194585) +++ head/sys/cddl/compat/opensolaris/sys/vnode.hSun Jun 21 13:41:32 2009(r194586) @@ -182,7 +182,7 @@ vn_openat(char *pnamep, enum uio_seg seg vref(startvp); NDINIT_ATVP(nd, operation, MPSAFE, UIO_SYSSPACE, pnamep, startvp, td); filemode |= O_NOFOLLOW; - error = vn_open_cred(nd, filemode, createmode, td-td_ucred, NULL); + error = vn_open_cred(nd, filemode, createmode, 0, td-td_ucred, NULL); NDFREE(nd, NDF_ONLY_PNBUF); if (error == 0) { /* We just unlock so we hold a reference. */ Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c == --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Sun Jun 21 13:15:56 2009(r194585) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Sun Jun 21 13:41:32 2009(r194586) @@ -4519,7 +4519,7 @@ vop_getextattr { flags = FREAD; NDINIT_ATVP(nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, attrname, xvp, td); - error = vn_open_cred(nd, flags, 0, ap-a_cred, NULL); + error = vn_open_cred(nd, flags, 0, 0, ap-a_cred, NULL); vp = nd.ni_vp; NDFREE(nd, NDF_ONLY_PNBUF); if (error != 0) { @@ -4640,7 +4640,7 @@ vop_setextattr { flags = FFLAGS(O_WRONLY | O_CREAT); NDINIT_ATVP(nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, attrname, xvp, td); - error = vn_open_cred(nd, flags, 0600, ap-a_cred, NULL); + error = vn_open_cred(nd, flags, 0600, 0, ap-a_cred, NULL); vp = nd.ni_vp; NDFREE(nd, NDF_ONLY_PNBUF); if (error != 0) { Modified: head/sys/kern/kern_alq.c == --- head/sys/kern/kern_alq.cSun Jun 21 13:15:56 2009(r194585) +++ head/sys/kern/kern_alq.cSun Jun 21 13:41:32 2009(r194586) @@ -351,7 +351,7 @@ alq_open(struct alq **alqp, const char * NDINIT(nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, file, td); flags = FWRITE | O_NOFOLLOW | O_CREAT; - error = vn_open_cred(nd, flags, cmode, cred, NULL); + error = vn_open_cred(nd, flags, cmode, 0, cred, NULL); if (error) return (error); Modified: head/sys/kern/kern_sig.c == --- head/sys/kern/kern_sig.cSun Jun 21 13:15:56 2009(r194585) +++ head/sys/kern/kern_sig.cSun Jun 21 13:41:32 2009(r194586) @@ -2940,7 +2940,8 @@ coredump(struct thread *td) restart: NDINIT(nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, name, td); flags = O_CREAT | FWRITE | O_NOFOLLOW; - error = vn_open(nd, flags, S_IRUSR | S_IWUSR, NULL); + error = vn_open_cred(nd, flags,
Re: svn commit: r194586 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs kern sys
On Sun, 21 Jun 2009, Konstantin Belousov wrote: Log: Add another flags argument to vn_open_cred. Use it to specify that some vn_open_cred invocations shall not audit namei path. Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c == --- head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.cSun Jun 21 13:15:56 2009(r194585) +++ head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.cSun Jun 21 13:41:32 2009(r194586) @@ -85,7 +85,8 @@ kobj_open_file_vnode(const char *file) flags = FREAD; NDINIT(nd, LOOKUP, MPSAFE, UIO_SYSSPACE, file, td); - error = vn_open_cred(nd, flags, O_NOFOLLOW, curthread-td_ucred, NULL); + error = vn_open_cred(nd, flags, O_NOFOLLOW, 0, curthread-td_ucred, + NULL); I was going to ask why not put the flag in the existing flags arg, like O_NOFOLLOW here?, but it seems that there is no existing flags arg and the above O_NOFOLLOW is garbage. O_NOFOLLOW happens to be 0x100, so I think the above asks for mode S_IRUSR. Now I will ask why not put O_NOFOLLOW here and the new flag in the existing pointer-to-flags arg?. Modified: head/sys/cddl/compat/opensolaris/sys/vnode.h == --- head/sys/cddl/compat/opensolaris/sys/vnode.hSun Jun 21 13:15:56 2009(r194585) +++ head/sys/cddl/compat/opensolaris/sys/vnode.hSun Jun 21 13:41:32 2009(r194586) @@ -182,7 +182,7 @@ vn_openat(char *pnamep, enum uio_seg seg vref(startvp); NDINIT_ATVP(nd, operation, MPSAFE, UIO_SYSSPACE, pnamep, startvp, td); filemode |= O_NOFOLLOW; - error = vn_open_cred(nd, filemode, createmode, td-td_ucred, NULL); + error = vn_open_cred(nd, filemode, createmode, 0, td-td_ucred, NULL); Here it does put O_NOFOLLOW in the existing pointer-to-flags arg. It obfuscates the open-flags variable by naming it filemode. Modified: head/sys/kern/vfs_vnops.c == --- head/sys/kern/vfs_vnops.c Sun Jun 21 13:15:56 2009(r194585) +++ head/sys/kern/vfs_vnops.c Sun Jun 21 13:41:32 2009(r194586) @@ -102,11 +102,8 @@ vn_open(ndp, flagp, cmode, fp) * due to the NDINIT being done elsewhere. */ int -vn_open_cred(ndp, flagp, cmode, cred, fp) - struct nameidata *ndp; - int *flagp, cmode; - struct ucred *cred; - struct file *fp; +vn_open_cred(struct nameidata *ndp, int *flagp, int cmode, u_int vn_open_flags, +struct ucred *cred, struct file *fp) { struct vnode *vp; struct mount *mp; @@ -124,9 +121,11 @@ restart: if (fmode O_CREAT) { Internally, flags are obfuscated by copying *flagp to the misnamed local variable fmode. The pointer-to-flags variable has about 12 spare bits in it. It already has just 1 kernel-only flag (O_HASLOCK, misnamed FHASLOCK and misassigned in the middle of the user flags). fcntl.h's list of open flags has been obfuscated by putting AT_ flags in the middle of the list. Bruce ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org