Date: Mon, 22 Jun 2009 13:22:32 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r194586 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs kern sys Message-ID: <20090622102232.GM2884@deviant.kiev.zoral.com.ua> In-Reply-To: <20090622133231.W31111@delplex.bde.org> References: <200906211341.n5LDfWEx008281@svn.freebsd.org> <20090622133231.W31111@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--37nyS7qXrnu4wN2o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Jun 22, 2009 at 02:12:57PM +1000, Bruce Evans wrote:
> On Sun, 21 Jun 2009, Konstantin Belousov wrote:
>=20
> >Log:
> > Add another flags argument to vn_open_cred. Use it to specify that some
> > vn_open_cred invocations shall not audit namei path.
>=20
> >Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c
> >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> >--- head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c Sun Jun 21=
=20
> >13:15:56 2009 (r194585)
> >+++ head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c Sun Jun 21=
=20
> >13:41:32 2009 (r194586)
> >@@ -85,7 +85,8 @@ kobj_open_file_vnode(const char *file)
> >
> > flags =3D FREAD;
> > NDINIT(&nd, LOOKUP, MPSAFE, UIO_SYSSPACE, file, td);
> >- error =3D vn_open_cred(&nd, &flags, O_NOFOLLOW, curthread->td_ucred,=
=20
> >NULL);
> >+ error =3D vn_open_cred(&nd, &flags, O_NOFOLLOW, 0, curthread->td_ucred,
> >+ NULL);
>=20
> 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.
>=20
> 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.
>=20
> >Modified: head/sys/cddl/compat/opensolaris/sys/vnode.h
> >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> >--- head/sys/cddl/compat/opensolaris/sys/vnode.h Sun Jun 21 13:15:56=20
> >2009 (r194585)
> >+++ head/sys/cddl/compat/opensolaris/sys/vnode.h Sun Jun 21 13:41:32=20
> >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,=20
> > td);
> > filemode |=3D O_NOFOLLOW;
> >- error =3D vn_open_cred(&nd, &filemode, createmode, td->td_ucred, NULL);
> >+ error =3D vn_open_cred(&nd, &filemode, createmode, 0, td->td_ucred,=20
> >NULL);
>=20
> Here it does put O_NOFOLLOW in the existing pointer-to-flags arg. It
> obfuscates the open-flags variable by naming it filemode.
>=20
> >Modified: head/sys/kern/vfs_vnops.c
> >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> >--- 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=20
> >vn_open_flags,
> >+ struct ucred *cred, struct file *fp)
> >{
> > struct vnode *vp;
> > struct mount *mp;
> >@@ -124,9 +121,11 @@ restart:
> > if (fmode & O_CREAT) {
>=20
> Internally, flags are obfuscated by copying *flagp to the misnamed local
> variable fmode.
>=20
> 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.
--37nyS7qXrnu4wN2o
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)
iEYEARECAAYFAko/W2cACgkQC3+MBN1Mb4j/7gCeI0aqQi0175YQN7oXtEY04XM8
XWAAn2dGN1NK7VbblBKRpsaXXCZkpd2S
=MhGT
-----END PGP SIGNATURE-----
--37nyS7qXrnu4wN2o--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090622102232.GM2884>