Re: [PATCH v6 09/12] ima: Add support for staging measurements with prompt

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

On Tue, 2026-06-02 at 13:13 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Introduce the ability of staging the IMA measurement list and deleting them
> with a prompt.
> 
> Staging means moving the current measurement list records to a separate
> location, and allowing users to read and delete it. This causes the current
> measurement list to be emptied (since records were moved) and new
> measurements to be added on the empty list. Staging can be done only once
> at a time. In the event of kexec(), staging is aborted and staged records
> will be carried over to the new kernel.

The kexec locking changes look good, thanks.

> 
> Introduce ascii_runtime_measurements_<algo>_staged and
> binary_runtime_measurements_<algo>_staged interfaces to access and delete
> the measurements.
> 
> Use 'echo A > <IMA _staged interface>' and
> 'echo D > <IMA _staged interface>' to respectively stage and delete the
> entire measurements list. Locking of these interfaces is also mediated with
> a call to _ima_measurements_open() and with ima_measurements_release().
> 
> Implement the staging functionality by introducing the new global
> measurements list ima_measurements_staged, and ima_queue_stage() and
> ima_queue_staged_delete_all() to respectively move measurements from the
> current measurements list to the staged one, and to move staged
> measurements to the ima_measurements_trim list for deletion. Introduce
> ima_queue_delete() to delete the measurements.
> 
> Staging is forbidden after measurement is suspended, and between staging
> and deleting, so that walking the staged and current measurements list can
> be done locklessly in ima_dump_measurement_list(). Strict ordering of
> suspending and dumping is enforced by two reboot notifiers with different
> priority. Refusing to delete staged measurements also signals to user space
> that those measurements are already carried over to the secondary kernel,
> so that it does not save them twice.
> 
> Finally, introduce the BINARY_STAGED and BINARY_FULL binary measurements
> list types, to maintain the counters and the binary size of staged
> measurements and the full measurements list (including records that were
> staged). BINARY still represents the current binary measurements list.
> 
> Use the binary size for the BINARY + BINARY_STAGED types in
> ima_add_kexec_buffer(), since both measurements list types are copied to
> the secondary kernel during kexec. Use BINARY_FULL in
> ima_measure_kexec_event(), to generate a critical data record.
> 
> It should be noted that the BINARY_FULL counter is not passed through
> kexec. Thus, the number of records included in the kexec critical data
> records refers to the records since the critical data records generated
> from the previous kexec event.
> 
> Note: This code derives from the Alt-IMA Huawei project, whose license is
>       GPL-2.0 OR MIT.
> 
> Link: https://github.com/linux-integrity/linux/issues/1
> Suggested-by: Gregory Lumen <gregorylumen@linux.microsoft.com> (staging revert)
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>

Thanks for the updates to the patch description, function docs, and comments.
Just one change needed (below) — otherwise this looks great.

> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c00c133a140f..a05db5b18982 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h

[...]

> @@ -337,6 +342,7 @@ extern atomic_long_t ima_num_records[BINARY__LAST];
>  /* Total number of violations since hard boot. */
>  extern atomic_long_t ima_num_violations;
>  extern struct hlist_head __rcu *ima_htable;
> +extern struct mutex ima_extend_list_mutex;

With the kexec locking change in this version, making ima_extend_list_mutex
global isn't necessary.

>  
>  static inline unsigned int ima_hash_key(u8 *digest)
>  {
> 
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 618694d5c082..a1aa141756e1 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c

[...]

> @@ -42,11 +43,11 @@ atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
>  /* key: inode (before secure-hashing a file) */
>  struct hlist_head __rcu *ima_htable;
>  
> -/* mutex protects atomicity of extending measurement list
> +/* mutex protects atomicity of extending and staging measurement list
>   * and extending the TPM PCR aggregate. Since tpm_extend can take
>   * long (and the tpm driver uses a mutex), we can't use the spinlock.
>   */
> -static DEFINE_MUTEX(ima_extend_list_mutex);
> +DEFINE_MUTEX(ima_extend_list_mutex);

Please drop this change.

Mimi