Re: [RFC][Patch 2/6] integrity: fs hook placement

["Serge E. Hallyn" <serue@us.ibm.com>]

Quoting Chris Wright (chrisw@sous-sol.org):
> * Mimi Zohar (zohar@linux.vnet.ibm.com) wrote:
> > +	integrity_d_instantiate(entry, inode);
> >  	security_d_instantiate(entry, inode);
> 
> > +		integrity_d_instantiate(entry, inode);
> >  		security_d_instantiate(entry, inode);
> 
> >  			spin_unlock(&dcache_lock);
> > +			integrity_d_instantiate(new, inode);
> >  			security_d_instantiate(new, inode);
> >  			d_rehash(dentry);
> 
> >  			d_move(new, dentry);
> > +			integrity_d_instantiate(dentry, inode);
> >  			security_d_instantiate(dentry, inode);
> >  			d_rehash(dentry);
> 
> >  	if (actual == dentry) {
> > +		integrity_d_instantiate(dentry, inode);
> >  		security_d_instantiate(dentry, inode);
> 
> 
> >  		file->f_op->release(inode, file);
> >  	security_file_free(file);
> > +	integrity_file_free(file);
> 
> 
> >  		security_file_free(file);
> > +		integrity_file_free(file);
> 
> 
> > 		if (security_inode_alloc(inode)) {
> ...
> > +		if (integrity_inode_alloc(inode)) {
> > +			if (inode->i_sb->s_op->destroy_inode)
> > +				inode->i_sb->s_op->destroy_inode(inode);
> 
> >  	security_inode_free(inode);
> > +	integrity_inode_free(inode);
> 
> >  	error = security_inode_setxattr(dentry, name, value, size, flags);
> >  	if (error)
> >  		goto out;
> > +
> > +	error = integrity_inode_setxattr(dentry, name, value, size, flags);
> 
> >  			fsnotify_xattr(dentry);
> > +			integrity_inode_post_setxattr(dentry, name);
> >  			security_inode_post_setxattr(dentry, name, value,
> 
> I know there's some slightly different goals, but this just doesn't make
> sense.  Need to get back to defining and expressing just the differences.

Are you objecting only to the duplication at the callsites, so that an
fsnotify-type of consolidation of security and integrity hooks would be
ok?  Or are you complaining that the security_inode_setxattr and
integrity_inode_setxattr hooks are too similar anyway, and integrity
modules should just use some lsm hooks for anything which will be
authoritative?

(I could see an argument that integirty subsystem should be purely for
measuring and hence its hooks should never return a value.  Only hitch
there is that if integrity subsystem hits ENOMEM it should be able to
refuse the action...)

-serge