lkml.org 
[lkml]   [2013]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot
On 11/22/2013 03:13 PM, Heiko Carstens wrote:
> On Fri, Nov 22, 2013 at 02:28:33PM +0100, Roberto Sassu wrote:
>> On 11/22/2013 12:48 PM, Heiko Carstens wrote:
>>> Hi Roberto,
>>>
>>> your patch 3ce1217d6cd5 "ima: define template fields library and new helpers"
>>> causes s390 to crash on boot:
>>>
>>
>> Hi Heiko
>>
>> thanks for the information. I think this issue is related to the error
>> detected by the kbuild test robot. Please, try to apply the attached
>> patch to see if it solves the problem.
>
> No, the patch doesn't fix the problem.

Ok, sorry for the delay. I was involved in another task.

The previous patch is not correct, as I allocate an
array of pointers, not structures. You can discard it.

Another problem that I found is that strsep()
modifies the source buffer by replacing the separator
character with '\0'. In particular, this function
modifies static data initialized at the beginning
of the ima_template.c file. Maybe, this is causing
the kernel panic. I already sent a patch to fix this
problem (attached to the email) even if it is not
supposed to land on the 3.13 kernel. Let me know
if this fixes the issue. Otherwise, I will check
the code more in depth.

Thanks

Roberto


>
>> From: Fengguang Wu <fengguang.wu@intel.com>
>> Subject: [PATCH] ima: fix coccinelle warnings
>> TO: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> CC: Roberto Sassu <roberto.sassu@polito.it>
>> CC: linux-kernel@vger.kernel.org
>>
>> security/integrity/ima/ima_template.c:62:41-47: ERROR: application of sizeof to pointer
>>
>> sizeof when applied to a pointer typed expression gives the size of
>> the pointer
>>
>> Generated by: coccinelle/misc/noderef.cocci
>>
>> CC: Roberto Sassu <roberto.sassu@polito.it>
>> CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>> ---
>>
>> cocci-output-13142-271b5e-ima_template.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/security/integrity/ima/ima_template.c
>> +++ b/security/integrity/ima/ima_template.c
>> @@ -59,7 +59,7 @@ static int template_desc_init_fields(cha
>> if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX)
>> return -EINVAL;
>>
>> - *fields = kzalloc(template_num_fields * sizeof(*fields), GFP_KERNEL);
>> + *fields = kzalloc(template_num_fields * sizeof(**fields), GFP_KERNEL);
>> if (*fields == NULL) {
>> result = -ENOMEM;
>> goto out;
>

From 2d3aa1c0328c44ecc3af7de162791c8cddfb6dfd Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@polito.it>
Date: Wed, 6 Nov 2013 13:51:35 +0100
Subject: [RFC][PATCH 2/4] ima: make a copy of template_fmt in
template_desc_init_fields()

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'.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
security/integrity/ima/ima_template.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 7bcff5c..bb33576 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -113,13 +113,19 @@ static int template_desc_init_fields(char *template_fmt,
struct ima_template_field ***fields,
int *num_fields)
{
- char *c, *template_fmt_ptr = template_fmt;
+ char *c, *template_fmt_ptr, *template_fmt_copy = NULL;
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;
+
+ template_fmt_ptr = template_fmt_copy;
*fields = kzalloc(template_num_fields * sizeof(*fields), GFP_KERNEL);
if (*fields == NULL) {
result = -ENOMEM;
@@ -139,6 +145,7 @@ static int template_desc_init_fields(char *template_fmt,
*num_fields = i;
return 0;
out:
+ kfree(template_fmt_copy);
kfree(*fields);
*fields = NULL;
return result;
--
1.8.1.4
\
 
 \ /
  Last update: 2013-11-22 19:21    [W:0.074 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site