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

[Roberto Sassu]

On Thu, 2026-03-26 at 16:19 -0700, steven chen wrote:
> On 3/26/2026 10:30 AM, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxx/T/#t>

In your approach you do:

lock ima_extend_measure_list_mutex
scan entries until N
cut list staged -> trim
unlock ima_extend_measure_list_mutex


In my approach I do:
lock ima_extend_measure_list_mutex
list replace active -> staged
unlock ima_extend_measure_list_mutex

scan entries until N

lock ima_extend_measure_list_mutex
cut list staged -> trim
splice staged ->active
unlock ima_extend_measure_list_mutex

So, I guess if you refer to less user space locking time, you mean one
lock/unlock and one list replace + list splice less in your solution.

In exchange, you propose to hold the lock in the kernel while scanning
N. I think it is a significant increase of kernel locking time vs a
negligible increase of user space locking time (in the kernel, all
processes need to wait for the ima_extend_measure_list_mutex to be
released, in user space it is just the agent waiting).

Roberto

> 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@xxxxxxxxxxxxxxxxxxx/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;
>