lkml.org 
[lkml]   [2008]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 3/4] integrity: Linux Integrity Module(LIM)
    From
    Date
    Christoph Hellwig <hch@infradead.org> wrote on 08/09/2008 02:53:40 PM:

    > > + if (integrity_inode_alloc(inode)) {
    > > + if (inode->i_sb->s_op->destroy_inode)
    > > + inode->i_sb->s_op->destroy_inode(inode);
    > > + else
    > > + kmem_cache_free(inode_cachep, (inode));
    > > + return NULL;
    > > + }
    > > +
    >
    > Please factor this and the lsm failure case out into a single
    > out_free_inode goto label.

    ok

    > > int vfs_permission(struct nameidata *nd, int mask)
    > > {
    > > - return inode_permission(nd->path.dentry->d_inode, mask);
    > > + int retval;
    > > +
    > > + retval = inode_permission(nd->path.dentry->d_inode, mask);
    > > + if (retval)
    > > + return retval;
    > > + return integrity_inode_permission(NULL, &nd->path,
    > > + mask & (MAY_READ | MAY_WRITE |
    > > + MAY_EXEC));
    > > }
    > >
    > > /**
    > > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
    > > */
    > > int file_permission(struct file *file, int mask)
    > > {
    > > - return inode_permission(file->f_path.dentry->d_inode, mask);
    > > + int retval;
    > > +
    > > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
    > > + if (retval)
    > > + return retval;
    > > + return integrity_inode_permission(file, NULL,
    > > + mask & (MAY_READ | MAY_WRITE |
    > > + MAY_EXEC));
    >
    > Please put your hook into inode_permission. Note that in inode
    > permission and lots of callers there is no path available so don't pass
    > it. Please pass the full MAY_FOO mask for new interfaces and do
    > filtering that won't break if new ones are introduced.

    We started out with the integrity_inode_permission() hook call in
    inode_permission(), but because of the removal of the nameidata
    parameter in the last merge, based on discussions
    http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
    the call to integrity_inode_permission() was moved up to the caller,
    where either a file or path are available. Any suggestions?

    > > +#ifdef CONFIG_INTEGRITY
    > > + void *i_integrity;
    > > +#endif
    >
    > Sorry, but I don't think we can bloat the inode even further for this.

    The original version of IMA was LSM based, using i_security. Based
    on discussions on the LSM mailing list, it was decided that the LSM hooks
    were meant only for access control. During the same time frame, there
    was a lot of work done in stacking LSM modules and i_security, but that
    approach was dropped. It was suggested that we define a separate set of
    hooks for integrity, which this patch set provides. Caching integrity
    results is an important aspect. Any suggestions in lieu of defining
    i_integrity?

    > > +/*
    > > + * integrity.h
    >
    > don't bother to mention the filename in the top of file comment.

    ok

    Mimi


    \
     
     \ /
      Last update: 2008-08-10 15:55    [W:0.026 / U:0.252 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site