[fmac-discuss] [PATCH] Move create hooks into ZFS
Stephen Smalley
sds at tycho.nsa.gov
Tue Sep 30 07:07:49 PDT 2008
On Tue, 2008-09-30 at 07:58 -0600, Mark Shellenbaum wrote:
> Stephen Smalley wrote:
> > Inserting the calls to the fmac_vnode_create() and
> > fmac_vnode_post_create() hooks into fop_create() causes the computation
> > of a new file security context and create permission checking to occur
> > on both new file creation and opening an existing file since the
> > underlying ->vop_create() method handles both cases. However, when
> > opening an existing file, we want to instead apply an access check
> > against the existing file security context. Thus, this patch moves
> > those calls into zfs_create(), and does likewise for the mkdir
> > processing for consistency. Setting of the incore vnode secid by
> > fmac_vnode_post_create() is pushed down the call chain through
> > zfs_mknode() down to zfs_perm_init() and the hook is renamed for
> > consistency/clarity. When opening an existing file, fmac_vnode_access()
> > is called instead of fmac_vnode_create().
> >
>
> Can you create a webrev of these changes?
Yes, will do. Just ran into a bug though in the code itself, so it
might be a little while before I have a new diff + webrev to post.
> Thanks,
>
> -Mark
>
> > diff --git a/usr/src/uts/common/fmac/fmac.c b/usr/src/uts/common/fmac/fmac.c
> > --- a/usr/src/uts/common/fmac/fmac.c
> > +++ b/usr/src/uts/common/fmac/fmac.c
> > @@ -337,7 +337,7 @@
> > }
> >
> > void
> > -fmac_vnode_post_create(vnode_t *vp, security_id_t secid)
> > +fmac_vnode_secid_init(vnode_t *vp, security_id_t secid)
> > {
> > if (!fmac_enabled)
> > return;
> > diff --git a/usr/src/uts/common/fs/vnode.c b/usr/src/uts/common/fs/vnode.c
> > --- a/usr/src/uts/common/fs/vnode.c
> > +++ b/usr/src/uts/common/fs/vnode.c
> > @@ -3367,8 +3367,6 @@
> > vsecattr_t *vsecp) /* ACL to set during create */
> > {
> > int ret;
> > - xvattr_t xvattr;
> > - security_id_t secid;
> >
> > if (vsecp != NULL &&
> > vfs_has_feature(dvp->v_vfsp, VFSFT_ACLONCREATE) == 0) {
> > @@ -3382,17 +3380,12 @@
> > (vfs_has_feature(dvp->v_vfsp, VFSFT_CASEINSENSITIVE) == 0 &&
> > vfs_has_feature(dvp->v_vfsp, VFSFT_NOCASESENSITIVE) == 0))
> > return (EINVAL);
> > -
> > - ret = fmac_vnode_create(dvp, name, &xvattr, &vap, cr, &secid);
> > - if (ret)
> > - return (ret);
> >
> > VOPXID_MAP_CR(dvp, cr);
> >
> > ret = (*(dvp)->v_op->vop_create)
> > (dvp, name, vap, excl, mode, vpp, cr, flags, ct, vsecp);
> > if (ret == 0 && *vpp) {
> > - fmac_vnode_post_create(*vpp, secid);
> > VOPSTATS_UPDATE(*vpp, create);
> > if ((*vpp)->v_path == NULL) {
> > vn_setpath(rootdir, dvp, *vpp, name, strlen(name));
> > @@ -3499,8 +3492,6 @@
> > int flags,
> > vsecattr_t *vsecp) /* ACL to set during create */
> > {
> > - xvattr_t xvattr;
> > - security_id_t secid;
> > int ret;
> >
> > if (vsecp != NULL &&
> > @@ -3515,17 +3506,12 @@
> > (vfs_has_feature(dvp->v_vfsp, VFSFT_CASEINSENSITIVE) == 0 &&
> > vfs_has_feature(dvp->v_vfsp, VFSFT_NOCASESENSITIVE) == 0))
> > return (EINVAL);
> > -
> > - ret = fmac_vnode_create(dvp, dirname, &xvattr, &vap, cr, &secid);
> > - if (ret)
> > - return (ret);
> >
> > VOPXID_MAP_CR(dvp, cr);
> >
> > ret = (*(dvp)->v_op->vop_mkdir)
> > (dvp, dirname, vap, vpp, cr, ct, flags, vsecp);
> > if (ret == 0 && *vpp) {
> > - fmac_vnode_post_create(*vpp, secid);
> > VOPSTATS_UPDATE(*vpp, mkdir);
> > if ((*vpp)->v_path == NULL) {
> > vn_setpath(rootdir, dvp, *vpp, dirname,
> > diff --git a/usr/src/uts/common/fs/zfs/sys/zfs_acl.h b/usr/src/uts/common/fs/zfs/sys/zfs_acl.h
> > --- a/usr/src/uts/common/fs/zfs/sys/zfs_acl.h
> > +++ b/usr/src/uts/common/fs/zfs/sys/zfs_acl.h
> > @@ -34,6 +34,7 @@
> > #endif
> > #include <sys/acl.h>
> > #include <sys/dmu.h>
> > +#include <sys/fmac/flask_types.h>
> > #include <sys/zfs_fuid.h>
> >
> > #ifdef __cplusplus
> > @@ -188,7 +189,7 @@
> >
> > #ifdef _KERNEL
> > void zfs_perm_init(struct znode *, struct znode *, int, vattr_t *,
> > - dmu_tx_t *, cred_t *, zfs_acl_t *, zfs_fuid_info_t **);
> > + dmu_tx_t *, cred_t *, zfs_acl_t *, zfs_fuid_info_t **, security_id_t);
> > int zfs_getacl(struct znode *, vsecattr_t *, boolean_t, cred_t *);
> > int zfs_setacl(struct znode *, vsecattr_t *, boolean_t, cred_t *);
> > void zfs_acl_rele(void *);
> > diff --git a/usr/src/uts/common/fs/zfs/sys/zfs_dir.h b/usr/src/uts/common/fs/zfs/sys/zfs_dir.h
> > --- a/usr/src/uts/common/fs/zfs/sys/zfs_dir.h
> > +++ b/usr/src/uts/common/fs/zfs/sys/zfs_dir.h
> > @@ -30,6 +30,8 @@
> >
> > #include <sys/pathname.h>
> > #include <sys/dmu.h>
> > +#include <sys/fmac/flask_types.h>
> > +#include <sys/fmac/flask.h>
> > #include <sys/zfs_znode.h>
> >
> > #ifdef __cplusplus
> > @@ -59,7 +61,7 @@
> > extern int zfs_dirlook(znode_t *, char *, vnode_t **, int, int *,
> > pathname_t *);
> > extern void zfs_mknode(znode_t *, vattr_t *, dmu_tx_t *, cred_t *,
> > - uint_t, znode_t **, int, zfs_acl_t *, zfs_fuid_info_t **);
> > + uint_t, znode_t **, int, zfs_acl_t *, zfs_fuid_info_t **, security_id_t);
> > extern void zfs_rmnode(znode_t *);
> > extern void zfs_dl_name_switch(zfs_dirlock_t *dl, char *new, char **old);
> > extern boolean_t zfs_dirempty(znode_t *);
> > diff --git a/usr/src/uts/common/fs/zfs/zfs_acl.c b/usr/src/uts/common/fs/zfs/zfs_acl.c
> > --- a/usr/src/uts/common/fs/zfs/zfs_acl.c
> > +++ b/usr/src/uts/common/fs/zfs/zfs_acl.c
> > @@ -50,6 +50,7 @@
> > #include <sys/dmu.h>
> > #include <sys/dnode.h>
> > #include <sys/zap.h>
> > +#include <sys/fmac/fmac.h>
> > #include "fs/fs_subr.h"
> > #include <acl/acl_common.h>
> >
> > @@ -1767,7 +1768,8 @@
> > void
> > zfs_perm_init(znode_t *zp, znode_t *parent, int flag,
> > vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
> > - zfs_acl_t *setaclp, zfs_fuid_info_t **fuidp)
> > + zfs_acl_t *setaclp, zfs_fuid_info_t **fuidp,
> > + security_id_t secid)
> > {
> > uint64_t mode, fuid, fgid;
> > int error;
> > @@ -1876,6 +1878,8 @@
> >
> > if (aclp != setaclp)
> > zfs_acl_free(aclp);
> > +
> > + fmac_vnode_secid_init(ZTOV(zp), secid);
> > }
> >
> > /*
> > diff --git a/usr/src/uts/common/fs/zfs/zfs_dir.c b/usr/src/uts/common/fs/zfs/zfs_dir.c
> > --- a/usr/src/uts/common/fs/zfs/zfs_dir.c
> > +++ b/usr/src/uts/common/fs/zfs/zfs_dir.c
> > @@ -836,7 +836,8 @@
> > dmu_tx_abort(tx);
> > return (error);
> > }
> > - zfs_mknode(zp, vap, tx, cr, IS_XATTR, &xzp, 0, NULL, &fuidp);
> > + zfs_mknode(zp, vap, tx, cr, IS_XATTR, &xzp, 0, NULL, &fuidp,
> > + SECINITSID_FILE);
> > ASSERT(xzp->z_phys->zp_parent == zp->z_id);
> > dmu_buf_will_dirty(zp->z_dbuf, tx);
> > zp->z_phys->zp_xattr = xzp->z_id;
> > diff --git a/usr/src/uts/common/fs/zfs/zfs_vnops.c b/usr/src/uts/common/fs/zfs/zfs_vnops.c
> > --- a/usr/src/uts/common/fs/zfs/zfs_vnops.c
> > +++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c
> > @@ -1232,6 +1232,8 @@
> >
> > if (zp == NULL) {
> > uint64_t txtype;
> > + xvattr_t xvattr;
> > + security_id_t secid;
> >
> > /*
> > * Create a new file object and update the directory
> > @@ -1240,6 +1242,11 @@
> > if (error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, B_FALSE, cr)) {
> > goto out;
> > }
> > +
> > + error = fmac_vnode_create(dvp, name, &xvattr, &vap, cr,
> > + &secid);
> > + if (error)
> > + goto out;
> >
> > /*
> > * We only support the creation of regular files in
> > @@ -1288,7 +1295,7 @@
> > zfs_acl_free(aclp);
> > return (error);
> > }
> > - zfs_mknode(dzp, vap, tx, cr, 0, &zp, 0, aclp, &fuidp);
> > + zfs_mknode(dzp, vap, tx, cr, 0, &zp, 0, aclp, &fuidp, secid);
> > (void) zfs_link_create(dl, zp, tx, ZNEW);
> > txtype = zfs_log_create_txtype(Z_FILE, vsecp, vap);
> > if (flag & FIGNORECASE)
> > @@ -1324,6 +1331,10 @@
> > if (mode && (error = zfs_zaccess_rwx(zp, mode, aflags, cr))) {
> > goto out;
> > }
> > +
> > + error = fmac_vnode_access(ZTOV(zp), mode, aflags, cr, B_TRUE);
> > + if (error)
> > + goto out;
> >
> > mutex_enter(&dzp->z_lock);
> > dzp->z_seq++;
> > @@ -1610,6 +1621,8 @@
> > zfs_acl_t *aclp = NULL;
> > zfs_fuid_info_t *fuidp = NULL;
> > int zf = ZNEW;
> > + xvattr_t xvattr;
> > + security_id_t secid;
> >
> > ASSERT(vap->va_type == VDIR);
> >
> > @@ -1660,6 +1673,13 @@
> > }
> >
> > if (error = zfs_zaccess(dzp, ACE_ADD_SUBDIRECTORY, 0, B_FALSE, cr)) {
> > + zfs_dirent_unlock(dl);
> > + ZFS_EXIT(zfsvfs);
> > + return (error);
> > + }
> > +
> > + error = fmac_vnode_create(dvp, dirname, &xvattr, &vap, cr, &secid);
> > + if (error) {
> > zfs_dirent_unlock(dl);
> > ZFS_EXIT(zfsvfs);
> > return (error);
> > @@ -1713,7 +1733,7 @@
> > /*
> > * Create new node.
> > */
> > - zfs_mknode(dzp, vap, tx, cr, 0, &zp, 0, aclp, &fuidp);
> > + zfs_mknode(dzp, vap, tx, cr, 0, &zp, 0, aclp, &fuidp, secid);
> >
> > if (aclp)
> > zfs_acl_free(aclp);
> > @@ -3244,6 +3264,8 @@
> > int error;
> > int zflg = ZNEW;
> > zfs_fuid_info_t *fuidp = NULL;
> > + xvattr_t xvattr;
> > + security_id_t secid;
> >
> > ASSERT(vap->va_type == VLNK);
> >
> > @@ -3260,6 +3282,12 @@
> > zflg |= ZCILOOK;
> > top:
> > if (error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, B_FALSE, cr)) {
> > + ZFS_EXIT(zfsvfs);
> > + return (error);
> > + }
> > +
> > + error = fmac_vnode_create(dvp, name, &xvattr, &vap, cr, &secid);
> > + if (error) {
> > ZFS_EXIT(zfsvfs);
> > return (error);
> > }
> > @@ -3317,13 +3345,13 @@
> > * otherwise, store it just like any other file data.
> > */
> > if (sizeof (znode_phys_t) + len <= dmu_bonus_max()) {
> > - zfs_mknode(dzp, vap, tx, cr, 0, &zp, len, NULL, &fuidp);
> > + zfs_mknode(dzp, vap, tx, cr, 0, &zp, len, NULL, &fuidp, secid);
> > if (len != 0)
> > bcopy(link, zp->z_phys + 1, len);
> > } else {
> > dmu_buf_t *dbp;
> >
> > - zfs_mknode(dzp, vap, tx, cr, 0, &zp, 0, NULL, &fuidp);
> > + zfs_mknode(dzp, vap, tx, cr, 0, &zp, 0, NULL, &fuidp, secid);
> > /*
> > * Nothing can access the znode yet so no locking needed
> > * for growing the znode's blocksize.
> > diff --git a/usr/src/uts/common/fs/zfs/zfs_znode.c b/usr/src/uts/common/fs/zfs/zfs_znode.c
> > --- a/usr/src/uts/common/fs/zfs/zfs_znode.c
> > +++ b/usr/src/uts/common/fs/zfs/zfs_znode.c
> > @@ -715,6 +715,7 @@
> > * bonuslen - length of bonus buffer
> > * setaclp - File/Dir initial ACL
> > * fuidp - Tracks fuid allocation.
> > + * secid - FMAC security identifier
> > *
> > * OUT: zpp - allocated znode
> > *
> > @@ -722,7 +723,7 @@
> > void
> > zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
> > uint_t flag, znode_t **zpp, int bonuslen, zfs_acl_t *setaclp,
> > - zfs_fuid_info_t **fuidp)
> > + zfs_fuid_info_t **fuidp, security_id_t secid)
> > {
> > dmu_buf_t *db;
> > znode_phys_t *pzp;
> > @@ -847,7 +848,7 @@
> > */
> > *zpp = dzp;
> > }
> > - zfs_perm_init(*zpp, dzp, flag, vap, tx, cr, setaclp, fuidp);
> > + zfs_perm_init(*zpp, dzp, flag, vap, tx, cr, setaclp, fuidp, secid);
> > }
> >
> > void
> > @@ -1626,7 +1627,8 @@
> >
> > ASSERT(!POINTER_IS_VALID(rootzp->z_zfsvfs));
> > rootzp->z_zfsvfs = &zfsvfs;
> > - zfs_mknode(rootzp, &vattr, tx, cr, IS_ROOT_NODE, &zp, 0, NULL, NULL);
> > + zfs_mknode(rootzp, &vattr, tx, cr, IS_ROOT_NODE, &zp, 0, NULL, NULL,
> > + SECINITSID_FILE);
> > ASSERT3P(zp, ==, rootzp);
> > ASSERT(!vn_in_dnlc(ZTOV(rootzp))); /* not valid to move */
> > error = zap_add(os, moid, ZFS_ROOT_OBJ, 8, 1, &rootzp->z_id, tx);
> > diff --git a/usr/src/uts/common/sys/fmac/fmac.h b/usr/src/uts/common/sys/fmac/fmac.h
> > --- a/usr/src/uts/common/sys/fmac/fmac.h
> > +++ b/usr/src/uts/common/sys/fmac/fmac.h
> > @@ -90,7 +90,7 @@
> > int fmac_vnode_set_secctx(char *, cred_t *, vtype_t, vnode_t *);
> > int fmac_vnode_create(vnode_t *, char *, xvattr_t *, vattr_t **, cred_t *,
> > security_id_t *);
> > -void fmac_vnode_post_create(vnode_t *, security_id_t);
> > +void fmac_vnode_secid_init(vnode_t *, security_id_t);
> > int fmac_vnode_link(vnode_t *tdvp, vnode_t *svp, char *name, cred_t *cr,
> > caller_context_t *ct);
> > int fmac_vnode_remove(vnode_t *dvp, vnode_t *vp, char *name, cred_t *cr);
> >
> >
> >
>
> _______________________________________________
> fmac-discuss mailing list
> fmac-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/fmac-discuss
--
Stephen Smalley
National Security Agency
More information about the fmac-discuss
mailing list