From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756120Ab3KVQZa (ORCPT ); Fri, 22 Nov 2013 11:25:30 -0500 Received: from fm1nodo1.polito.it ([130.192.180.11]:46150 "EHLO antispam.polito.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756057Ab3KVQZ1 (ORCPT ); Fri, 22 Nov 2013 11:25:27 -0500 X-AttachExt: patch X-ExtScanner: Niversoft's FindAttachments (free) Message-ID: <528F8515.7040201@polito.it> Date: Fri, 22 Nov 2013 17:23:49 +0100 From: Roberto Sassu Organization: Politecnico di Torino User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Heiko Carstens CC: Mimi Zohar , Martin Schwidefsky , linux-kernel@vger.kernel.org Subject: Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot References: <20131122114819.GB4208@osiris> <528F5C01.8020304@polito.it> <20131122141314.GC4208@osiris> In-Reply-To: <20131122141314.GC4208@osiris> Content-Type: multipart/mixed; boundary="------------020106010101040400030004" X-FEAS-SYSTEM-WL: 130.192.180.41 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is a multi-part message in MIME format. --------------020106010101040400030004 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 >> Subject: [PATCH] ima: fix coccinelle warnings >> TO: Mimi Zohar >> CC: Roberto Sassu >> 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 >> CC: Mimi Zohar >> Signed-off-by: Fengguang Wu >> --- >> >> 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; > --------------020106010101040400030004 Content-Type: text/x-diff; name="0002-ima-make-a-copy-of-template_fmt-in-template_desc_ini.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-ima-make-a-copy-of-template_fmt-in-template_desc_ini.pa"; filename*1="tch" >>From 2d3aa1c0328c44ecc3af7de162791c8cddfb6dfd Mon Sep 17 00:00:00 2001 From: Roberto Sassu 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 --- 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 --------------020106010101040400030004--