| 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
| ||
|
Date: Mon, 02 Dec 2013 21:28:23 -0500
From: Mimi Zohar <zohar@...ux.vnet.ibm.com>
To: Roberto Sassu <roberto.sassu@...ito.it>
Cc: jmorris@...ei.org, d.kasatkin@...sung.com,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ima: properly free ima_template_entry structures
On Mon, 2013-12-02 at 19:40 +0100, Roberto Sassu wrote:
> The new templates management mechanism records information associated
> to an event into an array of 'ima_field_data' structures and makes it
> available through the 'template_data' field of the 'ima_template_entry'
> structure (the element of the measurements list created by IMA).
>
> Since 'ima_field_data' contains dynamically allocated data (which length
> varies depending on the data associated to a selected template field),
> it is not enough to just free the memory reserved for a
> 'ima_template_entry' structure if something goes wrong.
>
> This patch creates the new function ima_free_template_entry() which
> walks the array of 'ima_field_data' structures, frees the memory
> referenced by the 'data' pointer and finally the space reserved for
> the 'ima_template_entry' structure. Further, it replaces existing kfree()
> that have a pointer to an 'ima_template_entry' structure as argument
> with calls to the new function.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@...ito.it>
Thanks, Roberto. James, I've been running with this patch since the end
of last week without problems. Both Christoph Paasch's patch and this
patch are available from linux-security/next-free-memory.
thanks,
Mimi
> ---
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_api.c | 21 +++++++++++++++++----
> security/integrity/ima/ima_init.c | 2 +-
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 9636e17..0356e1d 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -148,6 +148,7 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint,
> int xattr_len, struct ima_template_entry **entry);
> int ima_store_template(struct ima_template_entry *entry, int violation,
> struct inode *inode, const unsigned char *filename);
> +void ima_free_template_entry(struct ima_template_entry *entry);
> const char *ima_d_path(struct path *path, char **pathbuf);
>
> /* rbtree tree calls to lookup, insert, delete
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 8037484..c38bbce 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -22,6 +22,19 @@
> #include "ima.h"
>
> /*
> + * ima_free_template_entry - free an existing template entry
> + */
> +void ima_free_template_entry(struct ima_template_entry *entry)
> +{
> + int i;
> +
> + for (i = 0; i < entry->template_desc->num_fields; i++)
> + kfree(entry->template_data[i].data);
> +
> + kfree(entry);
> +}
> +
> +/*
> * ima_alloc_init_template - create and initialize a new template entry
> */
> int ima_alloc_init_template(struct integrity_iint_cache *iint,
> @@ -37,6 +50,7 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint,
> if (!*entry)
> return -ENOMEM;
>
> + (*entry)->template_desc = template_desc;
> for (i = 0; i < template_desc->num_fields; i++) {
> struct ima_template_field *field = template_desc->fields[i];
> u32 len;
> @@ -51,10 +65,9 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint,
> (*entry)->template_data_len += sizeof(len);
> (*entry)->template_data_len += len;
> }
> - (*entry)->template_desc = template_desc;
> return 0;
> out:
> - kfree(*entry);
> + ima_free_template_entry(*entry);
> *entry = NULL;
> return result;
> }
> @@ -134,7 +147,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
> }
> result = ima_store_template(entry, violation, inode, filename);
> if (result < 0)
> - kfree(entry);
> + ima_free_template_entry(entry);
> err_out:
> integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
> op, cause, result, 0);
> @@ -269,7 +282,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
> if (!result || result == -EEXIST)
> iint->flags |= IMA_MEASURED;
> if (result < 0)
> - kfree(entry);
> + ima_free_template_entry(entry);
> }
>
> void ima_audit_measurement(struct integrity_iint_cache *iint,
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 76b8e2c..3712276 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -75,7 +75,7 @@ static void __init ima_add_boot_aggregate(void)
> result = ima_store_template(entry, violation, NULL,
> boot_aggregate_name);
> if (result < 0)
> - kfree(entry);
> + ima_free_template_entry(entry);
> return;
> err_out:
> integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, boot_aggregate_name, op,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists