Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot

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

On 11/22/2013 05:35 PM, Heiko Carstens wrote:
> On Fri, Nov 22, 2013 at 05:23:49PM +0100, Roberto Sassu wrote:
>> 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.
>
> With your patch below applied the kernel boots again.
> So it should go into 3.13 (or a different fix).
>

Ok, good. Yes, we will apply this patch to the 3.13 kernel.


> Thanks!

No problem.

Roberto


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