Re: [GIT PULL] ima: bug fixes for Linus

[Roberto Sassu <roberto.sassu@polito.it>]

[-- Attachment #1.1: Type: text/plain, Size: 1497 bytes --]

On 11/25/2013 07:46 PM, Roberto Sassu wrote:
> On 11/25/2013 04:40 PM, James Morris wrote:
>> On Mon, 25 Nov 2013, Mimi Zohar wrote:
>>
>>> Hi James,
>>>
>>> These are the "essential fixes for regressions".
>>>
>>> The following changes since commit
>>> 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
>>>
>>>    Revert "KEYS: verify a certificate is signed by a 'trusted' key"
>>> (2013-11-23 16:38:17 -0800)
>>>
>>> are available in the git repository at:
>>>
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
>>> for-linus
>>>
>>> for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:
>>>
>>>    ima: make a copy of template_fmt in template_desc_init_fields()
>>> (2013-11-25 07:32:46 -0500)
>>>
>>> thanks,
>>>
>>> Mimi
>>>
>>> ----------------------------------------------------------------
>>> Roberto Sassu (3):
>>
>>>        ima: make a copy of template_fmt in template_desc_init_fields()
>>
>>
>>> template_desc_init_fields(char *template_fmt,
>>
>> That should probably be const char.
>>
>> Also, the call to kstrdup() results in a memory leak.
>>
>
> Hi James
>
> thanks for the comments. I'm implementing them and I will post
> a new version of the patch 'ima: make a copy of template_fmt in
> template_desc_init_fields()' shortly.
>

Hi everyone

attached to this email, there is the new version of the above patch.

Regards

Roberto Sassu


> Roberto Sassu
>
>
>>
>>
>>
>>
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ima-make-a-copy-of-template_fmt-in-template_desc_ini.patch --]
[-- Type: text/x-diff; name="0001-ima-make-a-copy-of-template_fmt-in-template_desc_ini.patch", Size: 4827 bytes --]

From fde6cb013e4613e87bfb2d8436782cc9a98ef906 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@polito.it>
Date: Thu, 7 Nov 2013 15:00:43 +0100
Subject: [PATCH] ima: make a copy of template_fmt in
 template_desc_init_fields()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch makes a copy of the 'template_fmt' function argument so that
the latter will not be modified by strsep(), which does the splitting by
replacing the given separator with '\0'.

 IMA: No TPM chip found, activating TPM-bypass!
 Unable to handle kernel pointer dereference at virtual kernel address 0000000000842000
 Oops: 0004 [#1] SMP
 Modules linked in:
 CPU: 3 PID: 1 Comm: swapper/0 Not tainted 3.12.0-rc2-00098-g3ce1217d6cd5 #17
 task: 000000003ffa0000 ti: 000000003ff84000 task.ti: 000000003ff84000
 Krnl PSW : 0704e00180000000 000000000044bf88 (strsep+0x7c/0xa0)
            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 EA:3
 Krnl GPRS: 000000000000007c 000000000000007c 000000003ff87d90 0000000000821fd8
            0000000000000000 000000000000007c 0000000000aa37e0 0000000000aa9008
            0000000000000051 0000000000a114d8 0000000100000002 0000000000842bde
            0000000000842bdf 00000000006f97f0 000000000040062c 000000003ff87cf0
 Krnl Code: 000000000044bf7c: a7f4000a           brc     15,44bf90
            000000000044bf80: b90200cc           ltgr    %r12,%r12
           #000000000044bf84: a7840006           brc     8,44bf90
           >000000000044bf88: 9200c000           mvi     0(%r12),0
            000000000044bf8c: 41c0c001           la      %r12,1(%r12)
            000000000044bf90: e3c020000024       stg     %r12,0(%r2)
            000000000044bf96: b904002b           lgr     %r2,%r11
            000000000044bf9a: ebbcf0700004       lmg     %r11,%r12,112(%r15)
 Call Trace:
 ([<00000000004005fe>] ima_init_template+0xa2/0x1bc)
  [<0000000000a7c896>] ima_init+0x7a/0xa8
  [<0000000000a7c938>] init_ima+0x24/0x40
  [<00000000001000e8>] do_one_initcall+0x68/0x128
  [<0000000000a4eb56>] kernel_init_freeable+0x20a/0x2b4
  [<00000000006a1ff4>] kernel_init+0x30/0x178
  [<00000000006b69fe>] kernel_thread_starter+0x6/0xc
  [<00000000006b69f8>] kernel_thread_starter+0x0/0xc
 Last Breaking-Event-Address:
  [<000000000044bf42>] strsep+0x36/0xa0

Fixes commit: adf53a7 ima: new templates management mechanism

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 security/integrity/ima/ima_template.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 4e5da99..913e192 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -90,7 +90,7 @@ static struct ima_template_field *lookup_template_field(const char *field_id)
 	return NULL;
 }
 
-static int template_fmt_size(char *template_fmt)
+static int template_fmt_size(const char *template_fmt)
 {
 	char c;
 	int template_fmt_len = strlen(template_fmt);
@@ -106,23 +106,28 @@ static int template_fmt_size(char *template_fmt)
 	return j + 1;
 }
 
-static int template_desc_init_fields(char *template_fmt,
+static int template_desc_init_fields(const char *template_fmt,
 				     struct ima_template_field ***fields,
 				     int *num_fields)
 {
-	char *c, *template_fmt_ptr = template_fmt;
+	char *c, *template_fmt_copy;
 	int template_num_fields = template_fmt_size(template_fmt);
 	int i, result = 0;
 
 	if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX)
 		return -EINVAL;
 
+	/* copying is needed as strsep() modifies the original buffer */
+	template_fmt_copy = kstrdup(template_fmt, GFP_KERNEL);
+	if (template_fmt_copy == NULL)
+		return -ENOMEM;
+
 	*fields = kzalloc(template_num_fields * sizeof(*fields), GFP_KERNEL);
 	if (*fields == NULL) {
 		result = -ENOMEM;
 		goto out;
 	}
-	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
+	for (i = 0; (c = strsep(&template_fmt_copy, "|")) != NULL &&
 	     i < template_num_fields; i++) {
 		struct ima_template_field *f = lookup_template_field(c);
 
@@ -133,10 +138,12 @@ static int template_desc_init_fields(char *template_fmt,
 		(*fields)[i] = f;
 	}
 	*num_fields = i;
-	return 0;
 out:
-	kfree(*fields);
-	*fields = NULL;
+	if (result < 0) {
+		kfree(*fields);
+		*fields = NULL;
+	}
+	kfree(template_fmt_copy);
 	return result;
 }
 
-- 
1.8.1.4


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4720 bytes --]