-tip testing found this stack corruption and bootup crash
in the ISDN subsystem, reported by stackprotector:
[ 25.656688] calling isdn_init+0x0/0x2c2 @ 1
[ 25.660388] ISDN subsystem Rev: 1.1.2.3/1.1.2.3/1.1.2.2/1.1.2.3/1.1.2.2/1.1.2.2
[ 25.668179] initcall isdn_init+0x0/0x2c2 returned 0 after 6510 usecs
[ 25.670005] calling isdn_bsdcomp_init+0x0/0x45 @ 1
[ 25.673336] PPP BSD Compression module registered
[ 25.676674] initcall isdn_bsdcomp_init+0x0/0x45 returned 0 after 3255 usecs
[ 25.680005] calling isdnloop_init+0x0/0x88 @ 1
[ 25.683337] isdnloop-ISDN-driver Rev 1.11.6.7
[ 25.686705] isdnloop: (loop0) virtual card added
[ 25.690004] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: c1de2d8b
[ 25.690006]
[ 25.693338] Pid: 1, comm: swapper Not tainted 2.6.31-rc8-tip-01250-geed031c-dirty #9565
[ 25.696672] Call Trace:
[ 25.700008] [<c190f517>] ? printk+0x1d/0x30
[ 25.703339] [<c190f45d>] panic+0x50/0xed
[ 25.706677] [<c1059194>] __stack_chk_fail+0x1e/0x42
[ 25.710005] [<c1de2d8b>] ? isdnloop_init+0x83/0x88
[ 25.713338] [<c1de2d8b>] isdnloop_init+0x83/0x88
[ 25.716674] [<c1001056>] _stext+0x56/0x15a
[ 25.720007] [<c1da8368>] kernel_init+0x8f/0xf1
[ 25.723338] [<c1da82d9>] ? kernel_init+0x0/0xf1
[ 25.726675] [<c1025c67>] kernel_thread_helper+0x7/0x58
[ 25.730005] Rebooting in 1 seconds..Press any key to enter the menu
The bug is that the temporary array:
char rev[10];
Is sized one byte too small to store strings based on
the 'revision' string.
This is a truly ancient bug: it has been introduced in
the v2.4.2.1 kernel, ~8.5 years ago, which extended
the length of 'revision' by 1 byte.
Instead of using a fixed size temporary array, size
it based on the 'revision' string.
Cc: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/isdn/isdnloop/isdnloop.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index a335c85..a965870 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -1494,7 +1494,7 @@ static int __init
isdnloop_init(void)
{
char *p;
- char rev[10];
+ char rev[sizeof(revision)+1];
if ((p = strchr(revision, ':'))) {
strcpy(rev, p + 1);
[ v2: use strlen instead of sizeof. ]
-tip testing found this stack corruption and bootup crash
in the ISDN subsystem, reported by stackprotector:
[ 25.656688] calling isdn_init+0x0/0x2c2 @ 1
[ 25.660388] ISDN subsystem Rev: 1.1.2.3/1.1.2.3/1.1.2.2/1.1.2.3/1.1.2.2/1.1.2.2
[ 25.668179] initcall isdn_init+0x0/0x2c2 returned 0 after 6510 usecs
[ 25.670005] calling isdn_bsdcomp_init+0x0/0x45 @ 1
[ 25.673336] PPP BSD Compression module registered
[ 25.676674] initcall isdn_bsdcomp_init+0x0/0x45 returned 0 after 3255 usecs
[ 25.680005] calling isdnloop_init+0x0/0x88 @ 1
[ 25.683337] isdnloop-ISDN-driver Rev 1.11.6.7
[ 25.686705] isdnloop: (loop0) virtual card added
[ 25.690004] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: c1de2d8b
[ 25.690006]
[ 25.693338] Pid: 1, comm: swapper Not tainted 2.6.31-rc8-tip-01250-geed031c-dirty #9565
[ 25.696672] Call Trace:
[ 25.700008] [<c190f517>] ? printk+0x1d/0x30
[ 25.703339] [<c190f45d>] panic+0x50/0xed
[ 25.706677] [<c1059194>] __stack_chk_fail+0x1e/0x42
[ 25.710005] [<c1de2d8b>] ? isdnloop_init+0x83/0x88
[ 25.713338] [<c1de2d8b>] isdnloop_init+0x83/0x88
[ 25.716674] [<c1001056>] _stext+0x56/0x15a
[ 25.720007] [<c1da8368>] kernel_init+0x8f/0xf1
[ 25.723338] [<c1da82d9>] ? kernel_init+0x0/0xf1
[ 25.726675] [<c1025c67>] kernel_thread_helper+0x7/0x58
[ 25.730005] Rebooting in 1 seconds..Press any key to enter the menu
The bug is that the temporary array:
char rev[10];
Is sized one byte too small to store strings based on
the 'revision' string.
This is a truly ancient bug: it has been introduced in
the v2.4.2.1 kernel, ~8.5 years ago, which extended
the length of 'revision' by 1 byte.
Instead of using a fixed size temporary array, size
it based on the 'revision' string.
Cc: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/isdn/isdnloop/isdnloop.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index a335c85..0c8d8cb 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -1494,7 +1494,7 @@ static int __init
isdnloop_init(void)
{
char *p;
- char rev[10];
+ char rev[strlen(revision)+1];
if ((p = strchr(revision, ':'))) {
strcpy(rev, p + 1);
On Wed, 2 Sep 2009 15:03:36 +0200
Ingo Molnar <[email protected]> wrote:
>
> [ v2: use strlen instead of sizeof. ]
>
> diff --git a/drivers/isdn/isdnloop/isdnloop.c
> b/drivers/isdn/isdnloop/isdnloop.c index a335c85..0c8d8cb 100644
> --- a/drivers/isdn/isdnloop/isdnloop.c
> +++ b/drivers/isdn/isdnloop/isdnloop.c
> @@ -1494,7 +1494,7 @@ static int __init
> isdnloop_init(void)
> {
> char *p;
> - char rev[10];
> + char rev[strlen(revision)+1];
>
> if ((p = strchr(revision, ':'))) {
> strcpy(rev, p + 1);
now it;s a runtime variable sized array.
NotNice(tm)
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Mittwoch, 2. September 2009 15:14:39 Arjan van de Ven wrote:
> On Wed, 2 Sep 2009 15:03:36 +0200
>
> Ingo Molnar <[email protected]> wrote:
> > [ v2: use strlen instead of sizeof. ]
> >
> > diff --git a/drivers/isdn/isdnloop/isdnloop.c
> > b/drivers/isdn/isdnloop/isdnloop.c index a335c85..0c8d8cb 100644
> > --- a/drivers/isdn/isdnloop/isdnloop.c
> > +++ b/drivers/isdn/isdnloop/isdnloop.c
> > @@ -1494,7 +1494,7 @@ static int __init
> > isdnloop_init(void)
> > {
> > char *p;
> > - char rev[10];
> > + char rev[strlen(revision)+1];
> >
> > if ((p = strchr(revision, ':'))) {
> > strcpy(rev, p + 1);
>
> now it;s a runtime variable sized array.
> NotNice(tm)
I will remove that crap revision printing code and replace it with some
easy printable version which do not need parsing this CVS revision string
again, since we do not use CVS anymore.
Karsten
* Arjan van de Ven <[email protected]> wrote:
> > - char rev[10];
> > + char rev[strlen(revision)+1];
> >
> > if ((p = strchr(revision, ':'))) {
> > strcpy(rev, p + 1);
>
> now it;s a runtime variable sized array.
> NotNice(tm)
ah, indeed - i thought GCC would figure out its constness but it
doesnt. v3 should fix this:
-------------->
>From aa9aff8c2633cf24ad6465d8bf5aa93aa5d91f83 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Tue, 26 May 2009 21:18:22 +0200
Subject: [PATCH] isdn: Fix stack corruption in isdnloop_init()
-tip testing found this stack corruption and bootup crash
in the ISDN subsystem, reported by stackprotector:
[ 25.656688] calling isdn_init+0x0/0x2c2 @ 1
[ 25.660388] ISDN subsystem Rev: 1.1.2.3/1.1.2.3/1.1.2.2/1.1.2.3/1.1.2.2/1.1.2.2
[ 25.668179] initcall isdn_init+0x0/0x2c2 returned 0 after 6510 usecs
[ 25.670005] calling isdn_bsdcomp_init+0x0/0x45 @ 1
[ 25.673336] PPP BSD Compression module registered
[ 25.676674] initcall isdn_bsdcomp_init+0x0/0x45 returned 0 after 3255 usecs
[ 25.680005] calling isdnloop_init+0x0/0x88 @ 1
[ 25.683337] isdnloop-ISDN-driver Rev 1.11.6.7
[ 25.686705] isdnloop: (loop0) virtual card added
[ 25.690004] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: c1de2d8b
[ 25.690006]
[ 25.693338] Pid: 1, comm: swapper Not tainted 2.6.31-rc8-tip-01250-geed031c-dirty #9565
[ 25.696672] Call Trace:
[ 25.700008] [<c190f517>] ? printk+0x1d/0x30
[ 25.703339] [<c190f45d>] panic+0x50/0xed
[ 25.706677] [<c1059194>] __stack_chk_fail+0x1e/0x42
[ 25.710005] [<c1de2d8b>] ? isdnloop_init+0x83/0x88
[ 25.713338] [<c1de2d8b>] isdnloop_init+0x83/0x88
[ 25.716674] [<c1001056>] _stext+0x56/0x15a
[ 25.720007] [<c1da8368>] kernel_init+0x8f/0xf1
[ 25.723338] [<c1da82d9>] ? kernel_init+0x0/0xf1
[ 25.726675] [<c1025c67>] kernel_thread_helper+0x7/0x58
[ 25.730005] Rebooting in 1 seconds..Press any key to enter the menu
The bug is that the temporary array:
char rev[10];
Is sized one byte too small to store strings based on
the 'revision' string.
This is a truly ancient bug: it has been introduced in
the v2.4.2.1 kernel, ~8.5 years ago, which extended
the length of 'revision' by 1 byte.
Instead of using a fixed size temporary array, size
it based on the 'revision' string.
Cc: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/isdn/isdnloop/isdnloop.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index a335c85..ea07fdd 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -15,7 +15,7 @@
#include <linux/sched.h>
#include "isdnloop.h"
-static char *revision = "$Revision: 1.11.6.7 $";
+static char revision[] = "$Revision: 1.11.6.7 $";
static char *isdnloop_id = "loop0";
MODULE_DESCRIPTION("ISDN4Linux: Pseudo Driver that simulates an ISDN card");
@@ -1494,7 +1494,7 @@ static int __init
isdnloop_init(void)
{
char *p;
- char rev[10];
+ char rev[sizeof(revision)];
if ((p = strchr(revision, ':'))) {
strcpy(rev, p + 1);
On Wed, 2 Sep 2009 16:02:01 +0200
Ingo Molnar <[email protected]> wrote:
> From: Ingo Molnar <[email protected]>
> Date: Tue, 26 May 2009 21:18:22 +0200
> Subject: [PATCH] isdn: Fix stack corruption in isdnloop_init()
>
> -tip testing found this stack corruption and bootup crash
> in the ISDN subsystem, reported by stackprotector:
I added this to my little pile of things to send to Linus tomorrow.
From: Ingo Molnar <[email protected]>
-tip testing found this stack corruption and bootup crash
in the ISDN subsystem, reported by stackprotector:
[ 25.656688] calling isdn_init+0x0/0x2c2 @ 1
[ 25.660388] ISDN subsystem Rev: 1.1.2.3/1.1.2.3/1.1.2.2/1.1.2.3/1.1.2.2/1.1.2.2
[ 25.668179] initcall isdn_init+0x0/0x2c2 returned 0 after 6510 usecs
[ 25.670005] calling isdn_bsdcomp_init+0x0/0x45 @ 1
[ 25.673336] PPP BSD Compression module registered
[ 25.676674] initcall isdn_bsdcomp_init+0x0/0x45 returned 0 after 3255 usecs
[ 25.680005] calling isdnloop_init+0x0/0x88 @ 1
[ 25.683337] isdnloop-ISDN-driver Rev 1.11.6.7
[ 25.686705] isdnloop: (loop0) virtual card added
[ 25.690004] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: c1de2d8b
[ 25.690006]
[ 25.693338] Pid: 1, comm: swapper Not tainted 2.6.31-rc8-tip-01250-geed031c-dirty #9565
[ 25.696672] Call Trace:
[ 25.700008] [<c190f517>] ? printk+0x1d/0x30
[ 25.703339] [<c190f45d>] panic+0x50/0xed
[ 25.706677] [<c1059194>] __stack_chk_fail+0x1e/0x42
[ 25.710005] [<c1de2d8b>] ? isdnloop_init+0x83/0x88
[ 25.713338] [<c1de2d8b>] isdnloop_init+0x83/0x88
[ 25.716674] [<c1001056>] _stext+0x56/0x15a
[ 25.720007] [<c1da8368>] kernel_init+0x8f/0xf1
[ 25.723338] [<c1da82d9>] ? kernel_init+0x0/0xf1
[ 25.726675] [<c1025c67>] kernel_thread_helper+0x7/0x58
[ 25.730005] Rebooting in 1 seconds..Press any key to enter the menu
The bug is that the temporary array:
char rev[10];
Is sized one byte too small to store strings based on the 'revision'
string.
This is a truly ancient bug: it has been introduced in the v2.4.2.1
kernel, ~8.5 years ago, which extended the length of 'revision' by 1 byte.
Instead of using a fixed size temporary array, size it based on the
'revision' string.
Signed-off-by: Ingo Molnar <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Karsten Keil <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
drivers/isdn/isdnloop/isdnloop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -puN drivers/isdn/isdnloop/isdnloop.c~isdn-fix-stack-corruption-in-isdnloop_init drivers/isdn/isdnloop/isdnloop.c
--- a/drivers/isdn/isdnloop/isdnloop.c~isdn-fix-stack-corruption-in-isdnloop_init
+++ a/drivers/isdn/isdnloop/isdnloop.c
@@ -15,7 +15,7 @@
#include <linux/sched.h>
#include "isdnloop.h"
-static char *revision = "$Revision: 1.11.6.7 $";
+static char revision[] = "$Revision: 1.11.6.7 $";
static char *isdnloop_id = "loop0";
MODULE_DESCRIPTION("ISDN4Linux: Pseudo Driver that simulates an ISDN card");
@@ -1494,7 +1494,7 @@ static int __init
isdnloop_init(void)
{
char *p;
- char rev[10];
+ char rev[sizeof(revision)];
if ((p = strchr(revision, ':'))) {
strcpy(rev, p + 1);
_