Re: [PATCH v2] ima: export the measurement list when needed

[david.safford@gmail.com]

On Wed, 2020-01-08 at 13:17 +0200, Janne Karhunen wrote:
> Some systems can end up carrying lots of entries in the ima
> measurement list. Since every entry is using a bit of kernel
> memory, allow the sysadmin to export the measurement list to
> the filesystem to free up some memory.
> 
> Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>

I like this approach, as it will work easily for measurement lists in 
any format, and it will work for user or kernel triggering.

I'm getting an OOPS, though, whenever I write a filename to the 
securityfs file (e.g. echo /var/log/ima.log > list_name).
Here's the relevant from syslog:

  BUG: unable to handle page fault for address: 00005650a0e7fe30
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0001) - permissions violation

  Oops: 0001 [#1] SMP NOPTI

  RIP: 0010:ima_write_list_name+0x35/0x114

I haven't had time to debug this. Any suggestions?

Also, one embedded comment follows...

> ---
>  security/integrity/ima/ima.h       |   7 +-
>  security/integrity/ima/ima_fs.c    | 171 +++++++++++++++++++++++++++++
>  security/integrity/ima/ima_queue.c |   2 +-
>  3 files changed, 175 insertions(+), 5 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 19769bf5f6ab..78f0b706848d 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -151,20 +151,19 @@ int template_desc_init_fields(const char *template_fmt,
>  struct ima_template_desc *ima_template_desc_current(void);
>  struct ima_template_desc *lookup_template_desc(const char *name);
>  bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
> +void ima_free_template_entry(struct ima_template_entry *entry);
>  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);
>  unsigned long ima_get_binary_runtime_size(void);
>  int ima_init_template(void);
>  void ima_init_template_list(void);
> +int ima_export_list(const char *from);
>  int __init ima_init_digests(void);
>  int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
>  			  void *lsm_data);
>  
> -/*
> - * used to protect h_table and sha_table
> - */
> -extern spinlock_t ima_queue_lock;
> +extern struct mutex ima_extend_list_mutex;
>  
>  struct ima_h_table {
>  	atomic_long_t len;	/* number of stored measurements in the list */
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 2000e8df0301..b60a241b0d8b 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -22,10 +22,17 @@
>  #include <linux/rcupdate.h>
>  #include <linux/parser.h>
>  #include <linux/vmalloc.h>
> +#include <linux/fs_struct.h>
> +#include <linux/syscalls.h>
>  
>  #include "ima.h"
>  
> +#define secfs_mnt	"/sys/kernel/security"
> +#define am_filename	"/integrity/ima/ascii_runtime_measurements"

You probably really want to export the binary data, as that's 
what's needed for attestation. (Or both, but that's trickier.)

dave

> +
>  static DEFINE_MUTEX(ima_write_mutex);
> +static DEFINE_MUTEX(ima_list_mutex);
> +static char *ima_msmt_list_name;
>  
>