Re: [PATCH v5 01/13] ima: Remove ima_h_table structure

[Mimi Zohar <zohar@linux.ibm.com>]

On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 

Instead of jumping straight to "With the upcoming change ...", some context is
needed.  Perhaps something like:

The ima_h_table structure is a collection of IMA measurement list metadata -
number of records in the IMA measurement list, number of integrity violations,
and a hash table containing the IMA template data hash, needed to prevent
measurement list record duplication.

Removing records from the measurement list needs to be reflected in the hash
table.  As a pre-req to removing records from the measurement list, separate ...

> With the upcoming change of dynamically allocating and replacing the hash
> table, the ima_h_table structure would have been replaced with a new one.
> 
> However, since the ima_h_table structure contains also the counters for
> number of measurements entries and violations, we would have needed to
> preserve their values in the new ima_h_table structure.
> 
> Instead, separate those counters from the hash table, remove the
> ima_h_table structure, and just replace the hash table pointer.
> 
> Finally, rename ima_show_htable_value(), ima_show_htable_violations()
> and ima_htable_violations_ops respectively to ima_show_counter(),
> ima_show_num_violations() and ima_num_violations_ops.
> 
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>


Other than referring to "entries" in the measurement list, the patch looks good.
I prefer referring to them as "records".

> ---
>  security/integrity/ima/ima.h       |  9 +++------
>  security/integrity/ima/ima_api.c   |  2 +-
>  security/integrity/ima/ima_fs.c    | 19 +++++++++----------
>  security/integrity/ima/ima_kexec.c |  2 +-
>  security/integrity/ima/ima_queue.c | 17 ++++++++++-------
>  5 files changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 69e9bf0b82c6..51a8a582df56 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -324,12 +324,9 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
>   */
>  extern spinlock_t ima_queue_lock;
>  
> -struct ima_h_table {
> -	atomic_long_t len;	/* number of stored measurements in the list */
> -	atomic_long_t violations;
> -	struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
> -};
> -extern struct ima_h_table ima_htable;
> +extern atomic_long_t ima_num_entries;

-> ima_num_records	/* Total number of measurement list records */

Will this be the current or total number of measurement list records since a
hard boot?

> +extern atomic_long_t ima_num_violations;

Similarly, will this be the current number or total number of violations since a
hard boot?  Please add a comment.

> +extern struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
>  
>  static inline unsigned int ima_hash_key(u8 *digest)
>  {

thanks,

Mimi