[PATCH 1/1] IMA event log trimming

[Roberto Sassu]

On Fri, 2025-12-05 at 10:30 +0100, Roberto Sassu wrote:
> On Tue, 2025-12-02 at 15:28 -0800, steven chen wrote:
> > This patch is for trimming N entries of the IMA event logs as well as
> > cleaning the hash table.
> > 
> > It provides a userspace interface ima_trim_log that can be used to input
> > number N to let kernel to trim N entries of IMA event logs. When read
> > this interface, it returns number of entries trimmed last tim.
> 
> High-level comments:
> - It does not offer the possibility to keep the hash table
> - There is no coordination between taking a snapshot and the readers of
>   the measurements list (I think it is necessary, since reading is
>   based on *pos, which contains the entries read until a given point;
>   if there is a truncate in the middle of the read, *pos would still
>   refer to the non-truncated list and the next read will skip some
>   measurement entries)
> - While trimming per se is ok, I like more the idea of staging changes
>   and letting the user delete the staged measurements list later
> 
> > A mutex ima_trim_list_mutex is provided to allow one trimming request
> > at a time.
> > 
> > Signed-off-by: steven chen <chenste at linux.microsoft.com>
> > ---
> >  security/integrity/ima/ima.h       |  2 +
> >  security/integrity/ima/ima_fs.c    | 78 ++++++++++++++++++++++++++++++
> >  security/integrity/ima/ima_queue.c | 42 ++++++++++++++++
> >  3 files changed, 122 insertions(+)
> > 
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index e3d71d8d56e3..ab0e30ee25ea 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
> >  
> >  #ifdef CONFIG_IMA_KEXEC
> >  void ima_measure_kexec_event(const char *event_name);
> > +long ima_purge_event_log(long number_logs);
> >  #else
> >  static inline void ima_measure_kexec_event(const char *event_name) {}
> > +static inline long ima_purge_event_log(long number_logs) { return 0; }
> >  #endif
> >  
> >  /*
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 87045b09f120..ea93448feedd 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -38,6 +38,11 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
> >  
> >  static int valid_policy = 1;
> >  
> > +#define IMA_LOG_TRIM_REQ_LENGTH 11
> > +static long trimcount;
> > +/* mutex protects atomicity of trimming measurement list requests */
> > +static DEFINE_MUTEX(ima_trim_list_mutex);
> > +
> >  static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> >  				     loff_t *ppos, atomic_long_t *val)
> >  {
> > @@ -289,6 +294,69 @@ static const struct file_operations ima_ascii_measurements_ops = {
> >  	.release = seq_release,
> >  };
> >  
> > +static int ima_log_trim_open(struct inode *inode, struct file *filp)
> > +{
> > +	if (!capable(CAP_SYS_ADMIN))
> > +		return -EPERM;
> > +	return 0;
> > +}
> > +
> > +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> > +{
> > +	char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH];	/* greater than largest 'long' string value */
> > +	ssize_t len;
> > +
> > +	len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
> > +	return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
> > +}
> > +
> > +static ssize_t ima_log_trim_write(struct file *file,
> > +				  const char __user *buf, size_t datalen, loff_t *ppos)
> > +{
> > +	unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
> > +	long count, n;
> > +	int ret;
> > +
> > +	mutex_lock(&ima_trim_list_mutex);
> > +
> > +	if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	n = (int)datalen;
> > +
> > +	ret = copy_from_user(req, buf, datalen);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	count = 0;
> > +	for (int i = 0; i < n; ++i) {
> > +		if (req[i] < '0' || req[i] > '9') {
> > +			ret = -EINVAL;
> > +			goto out;
> > +		}
> > +		count = count * 10 + req[i] - '0';
> > +	}
> > +	ret = ima_purge_event_log(count);
> > +
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	trimcount = ret;
> > +	ret = datalen;
> > +out:
> > +	mutex_unlock(&ima_trim_list_mutex);
> > +	return ret;
> > +}
> > +
> > +static const struct file_operations ima_log_trim_ops = {
> > +	.open = ima_log_trim_open,
> > +	.read = ima_log_trim_read,
> > +	.write = ima_log_trim_write,
> > +	.llseek = generic_file_llseek,
> > +};
> > +
> >  static ssize_t ima_read_policy(char *path)
> >  {
> >  	void *data = NULL;
> > @@ -528,6 +596,16 @@ int __init ima_fs_init(void)
> >  		goto out;
> >  	}
> >  
> > +	dentry = securityfs_create_file("ima_trim_log",
> > +					S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
> > +					ima_dir, NULL, &ima_log_trim_ops);
> > +	if (IS_ERR(dentry)) {
> > +		ret = PTR_ERR(dentry);
> > +		goto out;
> > +	}
> > +
> > +	trimcount = 0;
> > +
> >  	dentry = securityfs_create_file("runtime_measurements_count",
> >  				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> >  				   &ima_measurements_count_ops);
> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> > index 590637e81ad1..999cd42c517c 100644
> > --- a/security/integrity/ima/ima_queue.c
> > +++ b/security/integrity/ima/ima_queue.c
> > @@ -220,6 +220,48 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> >  	return result;
> >  }
> >  
> > +/* Delete the IMA event logs */
> > +long ima_purge_event_log(long number_logs)
> > +{
> > +	struct ima_queue_entry *qe;
> > +	long cur = 0;
> > +
> > +	if (number_logs <= 0)
> > +		return number_logs;
> > +
> > +	mutex_lock(&ima_extend_list_mutex);
> > +	rcu_read_lock();
> 
> Sorry, I'm missing why rcu_read_lock() is needed.

Ah, I guess it is because of list_for_each_entry_rcu(). In that case, I
think you can add 'true' as 4th argument, since you are holding
ima_extend_list_mutex.

Roberto

> > +
> > +	/*
> > +	 * Remove this entry from both hash table and the measurement list
> > +	 * When removing from hash table, decrease the length counter
> > +	 * so that the hash table re-sizing logic works correctly
> > +	 */
> > +	list_for_each_entry_rcu(qe, &ima_measurements, later) {
> > +		int i;
> > +
> > +		/* if CONFIG_IMA_DISABLE_HTABLE is set, the hash table is not used */
> > +		if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE))
> > +			hlist_del_rcu(&qe->hnext);
> > +
> > +		for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
> > +			kfree(qe->entry->template_data[i].data);
> > +			qe->entry->template_data[i].data = NULL;
> > +			qe->entry->template_data[i].len = 0;
> > +		}
> > +
> > +		atomic_long_dec(&ima_htable.len);
> > +		list_del_rcu(&qe->later);
> 
> Missing kfree() of qe->entry and qe?
> 
> Thanks
> 
> Roberto
> 
> > +		++cur;
> > +		if (cur >= number_logs)
> > +			break;
> > +	}
> > +
> > +	rcu_read_unlock();
> > +	mutex_unlock(&ima_extend_list_mutex);
> > +	return cur;
> > +}
> > +
> >  int ima_restore_measurement_entry(struct ima_template_entry *entry)
> >  {
> >  	int result = 0;