| Message ID | 20211005144414.64547-1-dueno@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IMA: make runtime measurement list pollable | expand |
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
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()
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; }
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(+)