Re: [PATCH v2] ima: initialize the real inode attributes of the iint cache

Re: [PATCH v2] ima: initialize the real inode attributes of the iint cache

[sashiko-bot]

> ima_iint_cache objects are allocated with kmem_cache_alloc() from a
> dedicated slab whose constructor only runs once, when the slab is first
> created; the constructor does not run again when a freed object is
> reallocated.  ima_iint_init_always() resets all iint fields except
> real_inode.ino and real_inode.dev, which therefore keep the values of
> the previously freed iint.
> 
> integrity_inode_attrs_changed() reads those fields in
> process_measurement() to detect changes to the backing file on stacked
> filesystems.  The fields are normally written by
> integrity_inode_attrs_store() from ima_collect_measurement(), but the
> IMA_HASH path sets IMA_HASHED without calling it, so on the next file
> open the stale, uninitialized ino and dev are read.  Depending on the
> residual values, IMA may skip re-evaluating a file whose backing data
> or metadata changed, or needlessly re-evaluate it.
> 
> At allocation time there are no valid real-inode attributes yet, so
> clear the entire real_inode structure.  On stacked filesystems the
> first comparison against the underlying real inode will then fail until
> ima_collect_measurement() stores the real inode's attributes, which is
> the safe, conservative direction.
> 
> Fixes: b836c4d29f27 ("ima: detect changes to the backing overlay file")
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103034320.1756673-1-caixinchen1@huawei.com?part=1

[PATCH v2] ima: initialize the real inode attributes of the iint cache

[Cai Xinchen]

ima_iint_cache objects are allocated with kmem_cache_alloc() from a
dedicated slab whose constructor only runs once, when the slab is first
created; the constructor does not run again when a freed object is
reallocated.  ima_iint_init_always() resets all iint fields except
real_inode.ino and real_inode.dev, which therefore keep the values of
the previously freed iint.

integrity_inode_attrs_changed() reads those fields in
process_measurement() to detect changes to the backing file on stacked
filesystems.  The fields are normally written by
integrity_inode_attrs_store() from ima_collect_measurement(), but the
IMA_HASH path sets IMA_HASHED without calling it, so on the next file
open the stale, uninitialized ino and dev are read.  Depending on the
residual values, IMA may skip re-evaluating a file whose backing data
or metadata changed, or needlessly re-evaluate it.

At allocation time there are no valid real-inode attributes yet, so
clear the entire real_inode structure.  On stacked filesystems the
first comparison against the underlying real inode will then fail until
ima_collect_measurement() stores the real inode's attributes, which is
the safe, conservative direction.

Fixes: b836c4d29f27 ("ima: detect changes to the backing overlay file")
Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/integrity/ima/ima_iint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index 00b249101f98..c25f41a4dede 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -59,7 +59,7 @@ static void ima_iint_init_always(struct ima_iint_cache *iint,
 				 struct inode *inode)
 {
 	iint->ima_hash = NULL;
-	iint->real_inode.version = 0;
+	memset(&iint->real_inode, 0, sizeof(iint->real_inode));
 	iint->flags = 0UL;
 	iint->atomic_flags = 0UL;
 	iint->ima_file_status = INTEGRITY_UNKNOWN;
-- 
2.18.0.huawei.25