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 <[email protected]>
Subject: [PATCH] ima: fix coccinelle warnings
TO: Mimi Zohar <[email protected]>
CC: Roberto Sassu <[email protected]>
CC: [email protected]
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 <[email protected]>
CC: Mimi Zohar <[email protected]>
Signed-off-by: Fengguang Wu <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
---
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
