* [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot
@ 2013-11-22 11:48 Heiko Carstens
2013-11-22 13:28 ` Roberto Sassu
0 siblings, 1 reply; 7+ messages in thread
From: Heiko Carstens @ 2013-11-22 11:48 UTC (permalink / raw)
To: Roberto Sassu, Mimi Zohar; +Cc: Martin Schwidefsky, linux-kernel
Hi Roberto,
your patch 3ce1217d6cd5 "ima: define template fields library and new helpers"
causes s390 to crash on boot:
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot 2013-11-22 11:48 [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot Heiko Carstens @ 2013-11-22 13:28 ` Roberto Sassu 2013-11-22 14:13 ` Heiko Carstens 0 siblings, 1 reply; 7+ messages in thread From: Roberto Sassu @ 2013-11-22 13:28 UTC (permalink / raw) To: Heiko Carstens, Mimi Zohar; +Cc: Martin Schwidefsky, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2287 bytes --] 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. Thanks Roberto > 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 > [-- Attachment #2: noderef-ima_template.patch --] [-- Type: text/x-diff, Size: 1101 bytes --] 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; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot 2013-11-22 13:28 ` Roberto Sassu @ 2013-11-22 14:13 ` Heiko Carstens 2013-11-22 16:23 ` Roberto Sassu 0 siblings, 1 reply; 7+ messages in thread From: Heiko Carstens @ 2013-11-22 14:13 UTC (permalink / raw) To: Roberto Sassu; +Cc: Mimi Zohar, Martin Schwidefsky, linux-kernel 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. > 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; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot 2013-11-22 14:13 ` Heiko Carstens @ 2013-11-22 16:23 ` Roberto Sassu 2013-11-22 16:35 ` Heiko Carstens 0 siblings, 1 reply; 7+ messages in thread From: Roberto Sassu @ 2013-11-22 16:23 UTC (permalink / raw) To: Heiko Carstens; +Cc: Mimi Zohar, Martin Schwidefsky, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2413 bytes --] 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; > [-- Attachment #2: 0002-ima-make-a-copy-of-template_fmt-in-template_desc_ini.patch --] [-- Type: text/x-diff, Size: 1768 bytes --] >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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot 2013-11-22 16:23 ` Roberto Sassu @ 2013-11-22 16:35 ` Heiko Carstens 2013-11-22 16:36 ` Roberto Sassu 2013-11-22 16:40 ` Mimi Zohar 0 siblings, 2 replies; 7+ messages in thread From: Heiko Carstens @ 2013-11-22 16:35 UTC (permalink / raw) To: Roberto Sassu; +Cc: Mimi Zohar, Martin Schwidefsky, linux-kernel 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). Thanks! > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot 2013-11-22 16:35 ` Heiko Carstens @ 2013-11-22 16:36 ` Roberto Sassu 2013-11-22 16:40 ` Mimi Zohar 1 sibling, 0 replies; 7+ messages in thread From: Roberto Sassu @ 2013-11-22 16:36 UTC (permalink / raw) To: Heiko Carstens; +Cc: Mimi Zohar, Martin Schwidefsky, linux-kernel 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 >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot 2013-11-22 16:35 ` Heiko Carstens 2013-11-22 16:36 ` Roberto Sassu @ 2013-11-22 16:40 ` Mimi Zohar 1 sibling, 0 replies; 7+ messages in thread From: Mimi Zohar @ 2013-11-22 16:40 UTC (permalink / raw) To: Heiko Carstens; +Cc: Roberto Sassu, Martin Schwidefsky, linux-kernel On Fri, 2013-11-22 at 17:35 +0100, 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). > > Thanks! Thanks for testing! There are a number of bug fixes for this release in linux-integrity/ima-fixes-new. Now that the security patches made it into Linus' tree, I'll send the pull request to James. thanks, Mimi ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-11-22 16:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-22 11:48 [BUG] 3ce1217d6cd5 ima patch causes s390 to crash on boot Heiko Carstens 2013-11-22 13:28 ` Roberto Sassu 2013-11-22 14:13 ` Heiko Carstens 2013-11-22 16:23 ` Roberto Sassu 2013-11-22 16:35 ` Heiko Carstens 2013-11-22 16:36 ` Roberto Sassu 2013-11-22 16:40 ` Mimi Zohar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox