| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220521025500.GD9107@mail.hallyn.com>
Date: Fri, 20 May 2022 21:55:00 -0500
From: "Serge E. Hallyn" <serge@...lyn.com>
To: Stefan Berger <stefanb@...ux.ibm.com>
Cc: linux-integrity@...r.kernel.org, zohar@...ux.ibm.com,
serge@...lyn.com, christian.brauner@...ntu.com,
containers@...ts.linux.dev, dmitry.kasatkin@...il.com,
ebiederm@...ssion.com, krzysztof.struczynski@...wei.com,
roberto.sassu@...wei.com, mpeters@...hat.com, lhinds@...hat.com,
lsturman@...hat.com, puiterwi@...hat.com, jejb@...ux.ibm.com,
jamjoom@...ibm.com, linux-kernel@...r.kernel.org,
paul@...l-moore.com, rgb@...hat.com,
linux-security-module@...r.kernel.org, jmorris@...ei.org,
jpenumak@...hat.com
Subject: Re: [PATCH v12 06/26] ima: Move measurement list related variables
into ima_namespace
On Wed, Apr 20, 2022 at 10:06:13AM -0400, Stefan Berger wrote:
> Move measurement list related variables into the ima_namespace. This way
> a front-end like securityfs can show the measurement list inside an IMA
> namespace.
>
> Also, in order for kexec to allocate memory for the existing
> measurement list, the measurement list memory size is stored in the
> binary_runtime_size variable. To avoid special-casing init_ima_ns, as
> much as possible, move it into the ima_namespace.
>
> Signed-off-by: Stefan Berger <stefanb@...ux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>
Reviewed-by: Serge Hallyn <serge@...lyn.com>
> ---
> security/integrity/ima/ima.h | 5 +++--
> security/integrity/ima/ima_fs.c | 6 ++++--
> security/integrity/ima/ima_init_ima_ns.c | 5 +++++
> security/integrity/ima/ima_kexec.c | 12 ++++++-----
> security/integrity/ima/ima_queue.c | 27 +++++++++++-------------
> 5 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 78798cfcf46c..82e8af2bf698 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -106,7 +106,6 @@ struct ima_queue_entry {
> struct list_head later; /* place in ima_measurements list */
> struct ima_template_entry *entry;
> };
> -extern struct list_head ima_measurements; /* list of all measurements */
>
> /* Some details preceding the binary serialized measurement list */
> struct ima_kexec_hdr {
> @@ -136,6 +135,8 @@ struct ima_namespace {
> struct ima_rule_entry *arch_policy_entry;
>
> struct ima_h_table ima_htable;
> + struct list_head ima_measurements; /* list of all measurements */
> + unsigned long binary_runtime_size; /* used by init_ima_ns */
> } __randomize_layout;
> extern struct ima_namespace init_ima_ns;
>
> @@ -186,7 +187,7 @@ int ima_restore_measurement_entry(struct ima_namespace *ns,
> int ima_restore_measurement_list(struct ima_namespace *ns,
> loff_t bufsize, void *buf);
> int ima_measurements_show(struct seq_file *m, void *v);
> -unsigned long ima_get_binary_runtime_size(void);
> +unsigned long ima_get_binary_runtime_size(struct ima_namespace *ns);
> int ima_init_template(void);
> void ima_init_template_list(void);
> int __init ima_init_digests(void);
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index dca7fe32d65e..5ef0e2b2cf64 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -80,12 +80,13 @@ static const struct file_operations ima_measurements_count_ops = {
> /* returns pointer to hlist_node */
> static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
> {
> + struct ima_namespace *ns = &init_ima_ns;
> loff_t l = *pos;
> struct ima_queue_entry *qe;
>
> /* we need a lock since pos could point beyond last element */
> rcu_read_lock();
> - list_for_each_entry_rcu(qe, &ima_measurements, later) {
> + list_for_each_entry_rcu(qe, &ns->ima_measurements, later) {
> if (!l--) {
> rcu_read_unlock();
> return qe;
> @@ -97,6 +98,7 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>
> static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
> {
> + struct ima_namespace *ns = &init_ima_ns;
> struct ima_queue_entry *qe = v;
>
> /* lock protects when reading beyond last element
> @@ -107,7 +109,7 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
> rcu_read_unlock();
> (*pos)++;
>
> - return (&qe->later == &ima_measurements) ? NULL : qe;
> + return (&qe->later == &ns->ima_measurements) ? NULL : qe;
> }
>
> static void ima_measurements_stop(struct seq_file *m, void *v)
> diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
> index 1945fa8cfc4d..a7477072c587 100644
> --- a/security/integrity/ima/ima_init_ima_ns.c
> +++ b/security/integrity/ima/ima_init_ima_ns.c
> @@ -20,6 +20,11 @@ static int ima_init_namespace(struct ima_namespace *ns)
> atomic_long_set(&ns->ima_htable.len, 0);
> atomic_long_set(&ns->ima_htable.violations, 0);
> memset(&ns->ima_htable.queue, 0, sizeof(ns->ima_htable.queue));
> + INIT_LIST_HEAD(&ns->ima_measurements);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC) && ns == &init_ima_ns)
> + ns->binary_runtime_size = 0;
> + else
> + ns->binary_runtime_size = ULONG_MAX;
>
> return 0;
> }
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index d7cc5cca6f84..fd5da31f19ee 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -15,7 +15,8 @@
> #include "ima.h"
>
> #ifdef CONFIG_IMA_KEXEC
> -static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
> +static int ima_dump_measurement_list(struct ima_namespace *ns,
> + unsigned long *buffer_size, void **buffer,
> unsigned long segment_size)
> {
> struct ima_queue_entry *qe;
> @@ -36,7 +37,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>
> memset(&khdr, 0, sizeof(khdr));
> khdr.version = 1;
> - list_for_each_entry_rcu(qe, &ima_measurements, later) {
> + list_for_each_entry_rcu(qe, &ns->ima_measurements, later) {
> if (file.count < file.size) {
> khdr.count++;
> ima_measurements_show(&file, qe);
> @@ -84,6 +85,7 @@ void ima_add_kexec_buffer(struct kimage *image)
> struct kexec_buf kbuf = { .image = image, .buf_align = PAGE_SIZE,
> .buf_min = 0, .buf_max = ULONG_MAX,
> .top_down = true };
> + struct ima_namespace *ns = &init_ima_ns;
> unsigned long binary_runtime_size;
>
> /* use more understandable variable names than defined in kbuf */
> @@ -96,11 +98,11 @@ void ima_add_kexec_buffer(struct kimage *image)
> * Reserve an extra half page of memory for additional measurements
> * added during the kexec load.
> */
> - binary_runtime_size = ima_get_binary_runtime_size();
> + binary_runtime_size = ima_get_binary_runtime_size(ns);
> if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
> kexec_segment_size = ULONG_MAX;
> else
> - kexec_segment_size = ALIGN(ima_get_binary_runtime_size() +
> + kexec_segment_size = ALIGN(ima_get_binary_runtime_size(ns) +
> PAGE_SIZE / 2, PAGE_SIZE);
> if ((kexec_segment_size == ULONG_MAX) ||
> ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
> @@ -108,7 +110,7 @@ void ima_add_kexec_buffer(struct kimage *image)
> return;
> }
>
> - ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> + ima_dump_measurement_list(ns, &kexec_buffer_size, &kexec_buffer,
> kexec_segment_size);
> if (!kexec_buffer) {
> pr_err("Not enough memory for the kexec measurement buffer.\n");
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 43961d5cd2ef..0355c2b0932c 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -24,13 +24,6 @@
> /* pre-allocated array of tpm_digest structures to extend a PCR */
> static struct tpm_digest *digests;
>
> -LIST_HEAD(ima_measurements); /* list of all measurements */
> -#ifdef CONFIG_IMA_KEXEC
> -static unsigned long binary_runtime_size;
> -#else
> -static unsigned long binary_runtime_size = ULONG_MAX;
> -#endif
> -
> /* 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.
> @@ -100,7 +93,7 @@ static int ima_add_digest_entry(struct ima_namespace *ns,
> qe->entry = entry;
>
> INIT_LIST_HEAD(&qe->later);
> - list_add_tail_rcu(&qe->later, &ima_measurements);
> + list_add_tail_rcu(&qe->later, &ns->ima_measurements);
>
> atomic_long_inc(&ns->ima_htable.len);
> if (update_htable) {
> @@ -110,12 +103,14 @@ static int ima_add_digest_entry(struct ima_namespace *ns,
> INIT_HLIST_NODE(&qe->hnext);
> }
>
> - if (binary_runtime_size != ULONG_MAX) {
> + if (ns->binary_runtime_size != ULONG_MAX) {
> int size;
>
> size = get_binary_runtime_size(entry);
> - binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
> - binary_runtime_size + size : ULONG_MAX;
> + ns->binary_runtime_size =
> + (ns->binary_runtime_size < ULONG_MAX - size)
> + ? ns->binary_runtime_size + size
> + : ULONG_MAX;
> }
> return 0;
> }
> @@ -123,14 +118,16 @@ static int ima_add_digest_entry(struct ima_namespace *ns,
> /*
> * Return the amount of memory required for serializing the
> * entire binary_runtime_measurement list, including the ima_kexec_hdr
> - * structure.
> + * structure. Carrying the measurement list across kexec is limited
> + * to init_ima_ns.
> */
> -unsigned long ima_get_binary_runtime_size(void)
> +unsigned long ima_get_binary_runtime_size(struct ima_namespace *ns)
> {
> - if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> + if (ns->binary_runtime_size >=
> + (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> return ULONG_MAX;
> else
> - return binary_runtime_size + sizeof(struct ima_kexec_hdr);
> + return ns->binary_runtime_size + sizeof(struct ima_kexec_hdr);
> }
>
> static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
> --
> 2.34.1
Powered by blists - more mailing lists