Re: [PATCH v4 11/13] ima: Support staging and deleting N measurements entries

[steven chen <chenste@linux.microsoft.com>]

On 3/26/2026 10:30 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Add support for sending a value N between 1 and ULONG_MAX to the staging
> interface. This value represents the number of measurements that should be
> deleted from the current measurements list.
>
> This staging method allows the remote attestation agents to easily separate
> the measurements that were verified (staged and deleted) from those that
> weren't due to the race between taking a TPM quote and reading the
> measurements list.
>
> In order to minimize the locking time of ima_extend_list_mutex, deleting
> N entries is realized by staging the entire current measurements list
> (with the lock), by determining the N-th staged entry (without the lock),
> and by splicing the entries in excess back to the current measurements list
> (with the lock). Finally, the N entries are deleted (without the lock).
>
> Flushing the hash table is not supported for N entries, since it would
> require removing the N entries one by one from the hash table under the
> ima_extend_list_mutex lock, which would increase the locking time.
>
> The ima_extend_list_mutex lock is necessary in ima_dump_measurement_list()
> because ima_queue_staged_delete_partial() uses __list_cut_position() to
> modify ima_measurements_staged, for which no RCU-safe variant exists. For
> the staging with prompt flavor alone, list_replace_rcu() could have been
> used instead, but since both flavors share the same kexec serialization
> path, the mutex is required regardless.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Suggested-by: Steven Chen <chenste@linux.microsoft.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>   security/integrity/ima/Kconfig     |  3 ++
>   security/integrity/ima/ima.h       |  1 +
>   security/integrity/ima/ima_fs.c    | 22 +++++++++-
>   security/integrity/ima/ima_queue.c | 70 ++++++++++++++++++++++++++++++
>   4 files changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index e714726f3384..6ddb4e77bff5 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -341,6 +341,9 @@ config IMA_STAGING
>   	  It allows user space to stage the measurements list for deletion and
>   	  to delete the staged measurements after confirmation.
>   
> +	  Or, alternatively, it allows user space to specify N measurements
> +	  entries to be deleted.
> +
>   	  On kexec, staging is reverted and staged measurements are prepended
>   	  to the current measurements list when measurements are copied to the
>   	  secondary kernel.
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 699b735dec7d..de0693fce53c 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -319,6 +319,7 @@ struct ima_template_desc *lookup_template_desc(const char *name);
>   bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
>   int ima_queue_stage(void);
>   int ima_queue_staged_delete_all(void);
> +int ima_queue_staged_delete_partial(unsigned long req_value);
>   int ima_restore_measurement_entry(struct ima_template_entry *entry);
>   int ima_restore_measurement_list(loff_t bufsize, void *buf);
>   int ima_measurements_show(struct seq_file *m, void *v);
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 39d9128e9f22..eb3f343c1138 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -28,6 +28,7 @@
>    * Requests:
>    * 'A\n': stage the entire measurements list
>    * 'D\n': delete all staged measurements
> + * '[1, ULONG_MAX]\n' delete N measurements entries
>    */
>   #define STAGED_REQ_LENGTH 21
>   
> @@ -319,6 +320,7 @@ static ssize_t ima_measurements_staged_write(struct file *file,
>   					     size_t datalen, loff_t *ppos)
>   {
>   	char req[STAGED_REQ_LENGTH];
> +	unsigned long req_value;
>   	int ret;
>   
>   	if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
> @@ -346,7 +348,25 @@ static ssize_t ima_measurements_staged_write(struct file *file,
>   		ret = ima_queue_staged_delete_all();
>   		break;
>   	default:
> -		ret = -EINVAL;
> +		if (ima_flush_htable) {
> +			pr_debug("Deleting staged N measurements not supported when flushing the hash table is requested\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = kstrtoul(req, 10, &req_value);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (req_value == 0) {
> +			pr_debug("Must delete at least one entry\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = ima_queue_stage();
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = ima_queue_staged_delete_partial(req_value);
The default processing is "Trim N" idea plus performance improvement.

Here do everything in one time. And this is what I said in v3.

[PATCH v3 1/3] ima: Remove ima_h_table structure 
<https://lore.kernel.org/linux-integrity/c61aeaa79929a98cb3a6d30835972891fac3570f.camel@linux.ibm.com/T/#t>


The important two parts of trimming is "trim N" and performance improvement.

The performance improvement include two parts:
     hash table staging
     active log list staging

And I think "Trim N" plus performance improvement is the right direction 
to go.
Lots of code for two steps "stage and trim" "stage" part can be removed.

Also race condition may happen if not holding the list all time in user 
space
during attestation period: from stage, read list, attestation and trimming.

So in order to improve the above user space lock time, "Trim T:N" can be 
used
not to hold list long in user space during attestation.

For Trim T:N, T represent total log trimmed since system boot up. Please 
refer to
https://lore.kernel.org/linux-integrity/20260205235849.7086-1-chenste@linux.microsoft.com/T/#t

Thanks,

Steven
>   	}
>   
>   	if (ret < 0)
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index f5c18acfbc43..4fb557d61a88 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -371,6 +371,76 @@ int ima_queue_staged_delete_all(void)
>   	return 0;
>   }
>   
> +int ima_queue_staged_delete_partial(unsigned long req_value)
> +{
> +	unsigned long req_value_copy = req_value;
> +	unsigned long size_to_remove = 0, num_to_remove = 0;
> +	struct list_head *cut_pos = NULL;
> +	LIST_HEAD(ima_measurements_trim);
> +	struct ima_queue_entry *qe;
> +	int ret = 0;
> +
> +	/*
> +	 * Safe walk (no concurrent write), not under ima_extend_list_mutex
> +	 * for performance reasons.
> +	 */
> +	list_for_each_entry(qe, &ima_measurements_staged, later) {
> +		size_to_remove += get_binary_runtime_size(qe->entry);
> +		num_to_remove++;
> +
> +		if (--req_value_copy == 0) {
> +			/* qe->later always points to a valid list entry. */
> +			cut_pos = &qe->later;
> +			break;
> +		}
> +	}
> +
> +	/* Nothing to remove, undoing staging. */
> +	if (req_value_copy > 0) {
> +		size_to_remove = 0;
> +		num_to_remove = 0;
> +		ret = -ENOENT;
> +	}
> +
> +	mutex_lock(&ima_extend_list_mutex);
> +	if (list_empty(&ima_measurements_staged)) {
> +		mutex_unlock(&ima_extend_list_mutex);
> +		return -ENOENT;
> +	}
> +
> +	if (cut_pos != NULL)
> +		/*
> +		 * ima_dump_measurement_list() does not modify the list,
> +		 * cut_pos remains the same even if it was computed before
> +		 * the lock.
> +		 */
> +		__list_cut_position(&ima_measurements_trim,
> +				    &ima_measurements_staged, cut_pos);
> +
> +	atomic_long_sub(num_to_remove, &ima_num_entries[BINARY_STAGED]);
> +	atomic_long_add(atomic_long_read(&ima_num_entries[BINARY_STAGED]),
> +			&ima_num_entries[BINARY]);
> +	atomic_long_set(&ima_num_entries[BINARY_STAGED], 0);
> +
> +	if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
> +		binary_runtime_size[BINARY_STAGED] -= size_to_remove;
> +		binary_runtime_size[BINARY] +=
> +					binary_runtime_size[BINARY_STAGED];
> +		binary_runtime_size[BINARY_STAGED] = 0;
> +	}
> +
> +	/*
> +	 * Splice (prepend) any remaining non-deleted staged entries to the
> +	 * active list (RCU not needed, there cannot be concurrent readers).
> +	 */
> +	list_splice(&ima_measurements_staged, &ima_measurements);
> +	INIT_LIST_HEAD(&ima_measurements_staged);
> +	mutex_unlock(&ima_extend_list_mutex);
> +
> +	ima_queue_delete(&ima_measurements_trim, false);
> +	return ret;
> +}
> +
>   static void ima_queue_delete(struct list_head *head, bool flush_htable)
>   {
>   	struct ima_queue_entry *qe, *qe_tmp;