[PATCH] IMA: make runtime measurement list pollable

[PATCH] IMA: make runtime measurement list pollable

[Daiki Ueno]

The IMA runtime measurement list exposed on securityfs is currently
not pollable, so applications that incrementally verify the
measurements have to monitor the file periodically.  This patch makes
it possible to poll the file in a similar fashion to the sysfs files,
i.e., POLLPRI will be signalled on any changes to the list.

Signed-off-by: Daiki Ueno <dueno@redhat.com>
---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_fs.c    | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_queue.c |  6 ++++++
 3 files changed, 30 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index be965a8715e4..a7070a74a7ff 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -180,6 +180,8 @@ struct ima_h_table {
 };
 extern struct ima_h_table ima_htable;
 
+extern wait_queue_head_t ima_htable_wait;
+
 static inline unsigned int ima_hash_key(u8 *digest)
 {
 	/* there is no point in taking a hash of part of a digest */
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 3d8e9d5db5aa..f8741784217d 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -21,6 +21,7 @@
 #include <linux/rcupdate.h>
 #include <linux/parser.h>
 #include <linux/vmalloc.h>
+#include <linux/poll.h>
 
 #include "ima.h"
 
@@ -198,11 +199,31 @@ static int ima_measurements_open(struct inode *inode, struct file *file)
 	return seq_open(file, &ima_measurments_seqops);
 }
 
+static __poll_t ima_measurements_poll(struct file *file, poll_table *wait)
+{
+	struct seq_file *seq = file->private_data;
+	/* always allow reading */
+	__poll_t mask = EPOLLIN | EPOLLRDNORM;
+	int event;
+
+	poll_wait(file, &ima_htable_wait, wait);
+
+	event = atomic_long_read(&ima_htable.len);
+
+	if (seq->poll_event != event) {
+		seq->poll_event = event;
+		mask |= EPOLLERR | EPOLLPRI;
+	}
+
+	return mask;
+}
+
 static const struct file_operations ima_measurements_ops = {
 	.open = ima_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
@@ -269,6 +290,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 static ssize_t ima_read_policy(char *path)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 532da87ce519..375325dfe449 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -38,6 +38,9 @@ struct ima_h_table ima_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+/* wait queue for polling changes in ima_htable */
+DECLARE_WAIT_QUEUE_HEAD(ima_htable_wait);
+
 /* mutex protects atomicity of extending 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.
@@ -119,6 +122,9 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
+
+	wake_up_interruptible(&ima_htable_wait);
+
 	return 0;
 }
 
-- 
2.31.1

Re: [PATCH] IMA: make runtime measurement list pollable

[Mimi Zohar]

Hi Daiki,

On Tue, 2021-10-05 at 16:44 +0200, Daiki Ueno wrote:
> The IMA runtime measurement list exposed on securityfs is currently
> not pollable, so applications that incrementally verify the
> measurements have to monitor the file periodically.  This patch makes
> it possible to poll the file in a similar fashion to the sysfs files,
> i.e., POLLPRI will be signalled on any changes to the list.
> 
> Signed-off-by: Daiki Ueno <dueno@redhat.com>

With the sample code in the poll man page and without this patch, the
ascii measurement list is pollable.  I must be missing something.

> ---
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 532da87ce519..375325dfe449 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -38,6 +38,9 @@ struct ima_h_table ima_htable = {
>  	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
>  };
>  
> +/* wait queue for polling changes in ima_htable */
> +DECLARE_WAIT_QUEUE_HEAD(ima_htable_wait);
> +
>  /* mutex protects atomicity of extending 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.
> @@ -119,6 +122,9 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
>  		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
>  		     binary_runtime_size + size : ULONG_MAX;
>  	}
> +
> +	wake_up_interruptible(&ima_htable_wait);
> +
>  	return 0;
>  }
>  

The call to wake_up_interruptible() probably should not be here, but in
the caller after the TPM is extended.   With that change, you would be
able to differentiate between restoring the measurement list after
kexec and regular additional measurements.

thanks,

Mimi

Re: [PATCH] IMA: make runtime measurement list pollable

[Daiki Ueno]

Hello Mimi,

Mimi Zohar <zohar@linux.ibm.com> writes:

> Hi Daiki,
>
> On Tue, 2021-10-05 at 16:44 +0200, Daiki Ueno wrote:
>> The IMA runtime measurement list exposed on securityfs is currently
>> not pollable, so applications that incrementally verify the
>> measurements have to monitor the file periodically.  This patch makes
>> it possible to poll the file in a similar fashion to the sysfs files,
>> i.e., POLLPRI will be signalled on any changes to the list.
>> 
>> Signed-off-by: Daiki Ueno <dueno@redhat.com>
>
> With the sample code in the poll man page and without this patch, the
> ascii measurement list is pollable.  I must be missing something.

The sample code polls with POLLIN, so every poll() call returns
immediately, even if there is no change in the measurement list.  That
is not ideal in some scenarios[1], and could be a CPU hog as the calling
process never sleeps.

With this patch, poll() behaves the same for POLLIN, while the user
program is also able to wait for actual changes to the measurement list,
with POLLPRI.

> The call to wake_up_interruptible() probably should not be here, but in
> the caller after the TPM is extended.   With that change, you would be
> able to differentiate between restoring the measurement list after
> kexec and regular additional measurements.

Thank you for the suggestion; I will test that change and get it back to
you soon.

Regards,

Footnotes:
[1]  https://github.com/keylime/keylime/blob/master/keylime/cmd/ima_emulator_adapter.py#L95
     Currently the poll() in the loop is no-op and it would spin if
     there is no time.sleep()

-- 
Daiki Ueno

[PATCH v2] IMA: make runtime measurement list pollable

[Daiki Ueno]

Currently it's not possible to efficiently monitor changes to the IMA
runtime measurement list with poll().  While ascii_runtime_measurement
file is pollable with POLLIN, it returns immediately even if there is
no change to the list, and thus cannot be used as a wait condition in
a loop.

This patch makes it possible to poll the file in a similar fashion to
the sysfs files: POLLIN is still always signalled, while POLLPRI will
also be signalled on actual changes made to the list.

Signed-off-by: Daiki Ueno <dueno@redhat.com>
---
v2: moved wake_up_interruptible() call after PCR extend and expanded
the patch description to cover the use-case, suggested by Mimi.
---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_fs.c    | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_queue.c |  8 +++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index be965a8715e4..a7070a74a7ff 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -180,6 +180,8 @@ struct ima_h_table {
 };
 extern struct ima_h_table ima_htable;
 
+extern wait_queue_head_t ima_htable_wait;
+
 static inline unsigned int ima_hash_key(u8 *digest)
 {
 	/* there is no point in taking a hash of part of a digest */
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 3d8e9d5db5aa..f8741784217d 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -21,6 +21,7 @@
 #include <linux/rcupdate.h>
 #include <linux/parser.h>
 #include <linux/vmalloc.h>
+#include <linux/poll.h>
 
 #include "ima.h"
 
@@ -198,11 +199,31 @@ static int ima_measurements_open(struct inode *inode, struct file *file)
 	return seq_open(file, &ima_measurments_seqops);
 }
 
+static __poll_t ima_measurements_poll(struct file *file, poll_table *wait)
+{
+	struct seq_file *seq = file->private_data;
+	/* always allow reading */
+	__poll_t mask = EPOLLIN | EPOLLRDNORM;
+	int event;
+
+	poll_wait(file, &ima_htable_wait, wait);
+
+	event = atomic_long_read(&ima_htable.len);
+
+	if (seq->poll_event != event) {
+		seq->poll_event = event;
+		mask |= EPOLLERR | EPOLLPRI;
+	}
+
+	return mask;
+}
+
 static const struct file_operations ima_measurements_ops = {
 	.open = ima_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
@@ -269,6 +290,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 static ssize_t ima_read_policy(char *path)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 532da87ce519..de4941909a93 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -38,6 +38,9 @@ struct ima_h_table ima_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+/* wait queue for polling changes in ima_htable */
+DECLARE_WAIT_QUEUE_HEAD(ima_htable_wait);
+
 /* mutex protects atomicity of extending 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.
@@ -119,6 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
+
 	return 0;
 }
 
@@ -193,9 +197,11 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			 tpmresult);
 		audit_cause = tpm_audit_cause;
 		audit_info = 0;
-	}
+	} else
+		wake_up_interruptible(&ima_htable_wait);
 out:
 	mutex_unlock(&ima_extend_list_mutex);
+
 	integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
 			    op, audit_cause, result, audit_info);
 	return result;
-- 
2.31.1