Re: [PATCH v3 25/25] integrity: Switch from rbtree to LSM-managed blob for integrity_iint_cache

Stefan Berger
2023-09-12T16:20:17Z

On 9/4/23 09:40, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Before the security field of kernel objects could be shared among LSMs with
> the LSM stacking feature, IMA and EVM had to rely on an alternative storage
> of inode metadata. The association between inode metadata and inode is
> maintained through an rbtree.
>
> With the reservation mechanism offered by the LSM infrastructure, the
> rbtree is no longer necessary, as each LSM could reserve a space in the
> security blob for each inode. Thus, request from the 'integrity' LSM a
> space in the security blob for the pointer of inode metadata
> (integrity_iint_cache structure).
>
> Prefer this to allocating the integrity_iint_cache structure directly, as
> IMA would require it only for a subset of inodes. Always allocating it
> would cause a waste of memory.
>
> Introduce two primitives for getting and setting the pointer of
> integrity_iint_cache in the security blob, respectively
> integrity_inode_get_iint() and integrity_inode_set_iint(). This would make
> the code more understandable, as they directly replace rbtree operations.
>
> Locking is not needed, as access to inode metadata is not shared, it is per
> inode.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>
> @@ -145,10 +91,8 @@ static void integrity_inode_free(struct inode *inode)
> if (!IS_IMA(inode))
> return;

I think you can remove this check !IS_IMA()  as well since the next
function called here integrity_iint_find() already has this check:

struct integrity_iint_cache *integrity_iint_find(struct inode *inode)
{
if (!IS_IMA(inode))
return NULL;

return integrity_inode_get_iint(inode);
}

>
> - write_lock(&integrity_iint_lock);
> - iint = __integrity_iint_find(inode);
> - rb_erase(&iint->rb_node, &integrity_iint_tree);
> - write_unlock(&integrity_iint_lock);
> + iint = integrity_iint_find(inode); <--------------
> + integrity_inode_set_iint(inode, NULL);
>
> iint_free(iint);
> }