linux-kernel - Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OF0AEBDCA2.1E27B823-ON852574A1.000FEE48-852574A1.004C3185@us.ibm.com>
Date:	Sun, 10 Aug 2008 09:52:13 -0400
From:	Mimi Zohar <zohar@...ibm.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	Al Viro <viro@...IV.linux.org.uk>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	James Morris <jmorris@...ei.org>,
	Randy Dunlap <randy.dunlap@...cle.com>, safford@...son.ibm.com,
	serue@...ux.vnet.ibm.com, sailer@...son.ibm.com,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

Christoph Hellwig <hch@...radead.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/