Re: svn commit: r194586 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs kern sys

2009-06-22 Thread Kostik Belousov
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

2009-06-22 Thread Bruce Evans

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

2009-06-21 Thread Konstantin Belousov
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

2009-06-21 Thread Bruce Evans

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