linux-kernel - Re: [PATCH 1/2] ima: split ima_add_digest

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1321450690.1901.24.camel@falcor>
Date:	Wed, 16 Nov 2011 08:38:10 -0500
From:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
To:	Roberto Sassu <roberto.sassu@...ito.it>
Cc:	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, jmorris@...ei.org,
	Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
Subject: Re: [PATCH 1/2] ima: split ima_add_digest_entry() function

On Wed, 2011-11-16 at 11:10 +0100, Roberto Sassu wrote:
> The ima_add_digest_entry() function has been split in order to avoid
> adding an entry in the measurements list for which the PCR extend
> operation subsequently fails. Required memory is allocated earlier in the
> new function ima_prepare_template_entry() and the template entry is added
> after ima_pcr_extend().
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@...ito.it>

Not adding a measurement would certainly resolve the problems with
validating the measurement list against the PCR, but poses a risk that
something was read/executed that wasn't included in the measurement
list.  One solution would be for IMA to queue these measurements for the
TPM, but, I think, a better solution would be for the tpm driver to
queue them.

thanks,

Mimi

> ---
>  security/integrity/ima/ima_queue.c |   35 ++++++++++++++++++++++++++---------
>  1 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 8e28f04..ddc7e18 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -59,30 +59,41 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value)
>  	return ret;
>  }
> 
> -/* ima_add_template_entry helper function:
> - * - Add template entry to measurement list and hash table.
> +/* ima_prepare_template_entry helper function:
> + * - prepare template entry before adding it to the measurement list.
>   *
>   * (Called with ima_extend_list_mutex held.)
>   */
> -static int ima_add_digest_entry(struct ima_template_entry *entry)
> +static struct ima_queue_entry *ima_prepare_template_entry(
> +					struct ima_template_entry *entry)
>  {
>  	struct ima_queue_entry *qe;
> -	unsigned int key;
> 
>  	qe = kmalloc(sizeof(*qe), GFP_KERNEL);
>  	if (qe == NULL) {
>  		pr_err("IMA: OUT OF MEMORY ERROR creating queue entry.\n");
> -		return -ENOMEM;
> +		goto out;
>  	}
>  	qe->entry = entry;
> +out:
> +	return qe;
> +}
> +
> +/* ima_add_digest_entry helper function:
> + * - Add template entry to measurement list and hash table.
> + *
> + * (Called with ima_extend_list_mutex held.)
> + */
> +static void ima_add_digest_entry(struct ima_queue_entry *qe)
> +{
> +	unsigned int key;
> 
>  	INIT_LIST_HEAD(&qe->later);
>  	list_add_tail_rcu(&qe->later, &ima_measurements);
> 
>  	atomic_long_inc(&ima_htable.len);
> -	key = ima_hash_key(entry->digest);
> +	key = ima_hash_key(qe->entry->digest);
>  	hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> -	return 0;
>  }
> 
>  static int ima_pcr_extend(const u8 *hash)
> @@ -104,6 +115,7 @@ static int ima_pcr_extend(const u8 *hash)
>  int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  			   const char *op, struct inode *inode)
>  {
> +	struct ima_queue_entry *qe;
>  	u8 digest[IMA_DIGEST_SIZE];
>  	const char *audit_cause = "hash_added";
>  	int audit_info = 1;
> @@ -118,10 +130,11 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  		}
>  	}
> 
> -	result = ima_add_digest_entry(entry);
> -	if (result < 0) {
> +	qe = ima_prepare_template_entry(entry);
> +	if (qe == NULL) {
>  		audit_cause = "ENOMEM";
>  		audit_info = 0;
> +		result = -ENOMEM;
>  		goto out;
>  	}
> 
> @@ -132,7 +145,11 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  	if (result != 0) {
>  		audit_cause = "TPM error";
>  		audit_info = 0;
> +		kfree(qe);
> +		goto out;
>  	}
> +
> +	ima_add_digest_entry(qe);
>  out:
>  	mutex_unlock(&ima_extend_list_mutex);
>  	integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode,


--
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/