lkml.org 
[lkml]   [2011]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ima: fix memory leak and invalid memory reference bugs
On 12/05/2011 08:10 PM, Mimi Zohar wrote:
> On Mon, 2011-12-05 at 16:38 +0100, Roberto Sassu wrote:
>> This patch frees the memory of measurements entries that have already
>> been inserted in the measurements list and prevent the release when the
>> PCR extend operation failed.
>>
>> Signed-off-by: Roberto Sassu<roberto.sassu@polito.it>
>
> Thanks for the updated patch. I still need to test it, but it looks
> good. The patch description should probably include an explanation of
> how a duplicate measurement is being added. Something like:
>
> When a new measurement is added to the measurement list, this info is
> cached in the iint for performance. When the inode is flushed from
> cache, the associated iint is flushed as well. Subsequent access to the
> inode will cause it to be re-measured and will attempt to unnecessarily
> add it to the measurement list.
>
> This patch fixes a memory leak, by freeing the memory of the unnecessary
> duplicate measurement, and also fixes an invalid memory reference, by
> preventing the freeing of a new valid measurement entry, when the PCR
> extend operation fails.
>

Hi Mimi

do you mean that can i resend this patch as is or should i wait
until you tested it?

Thanks

Roberto Sassu


> thanks,
>
> Mimi
>
>> ---
>> security/integrity/ima/ima_api.c | 4 ++--
>> security/integrity/ima/ima_queue.c | 10 ++++++----
>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
>> index 0d50df0..88a2788 100644
>> --- a/security/integrity/ima/ima_api.c
>> +++ b/security/integrity/ima/ima_api.c
>> @@ -178,8 +178,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
>> strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX);
>>
>> result = ima_store_template(entry, violation, inode);
>> - if (!result)
>> + if (!result || result == -EEXIST)
>> iint->flags |= IMA_MEASURED;
>> - else
>> + if (result< 0)
>> kfree(entry);
>> }
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> index 8e28f04..0fe41b81 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -94,7 +94,8 @@ static int ima_pcr_extend(const u8 *hash)
>>
>> result = tpm_pcr_extend(TPM_ANY_NUM, CONFIG_IMA_MEASURE_PCR_IDX, hash);
>> if (result != 0)
>> - pr_err("IMA: Error Communicating to TPM chip\n");
>> + pr_err("IMA: Error Communicating to TPM chip, result: %d\n",
>> + result);
>> return result;
>> }
>>
>> @@ -107,13 +108,14 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>> u8 digest[IMA_DIGEST_SIZE];
>> const char *audit_cause = "hash_added";
>> int audit_info = 1;
>> - int result = 0;
>> + int result = 0, tpmresult = 0;
>>
>> mutex_lock(&ima_extend_list_mutex);
>> if (!violation) {
>> memcpy(digest, entry->digest, sizeof digest);
>> if (ima_lookup_digest_entry(digest)) {
>> audit_cause = "hash_exists";
>> + result = -EEXIST;
>> goto out;
>> }
>> }
>> @@ -128,8 +130,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>> if (violation) /* invalidate pcr */
>> memset(digest, 0xff, sizeof digest);
>>
>> - result = ima_pcr_extend(digest);
>> - if (result != 0) {
>> + tpmresult = ima_pcr_extend(digest);
>> + if (tpmresult != 0) {
>> audit_cause = "TPM error";
>> audit_info = 0;
>> }
>
>



\
 
 \ /
  Last update: 2011-12-06 10:35    [W:0.036 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site