* [PATCH] isdn: Fix stack corruption in isdnloop_init()
@ 2009-09-02 12:44 Ingo Molnar
2009-09-02 13:03 ` [PATCH, v2] " Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2009-09-02 12:44 UTC (permalink / raw)
To: linux-kernel, Karsten Keil, isdn4linux
Cc: Andrew Morton, Arjan van de Ven, tj
-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: <stable@kernel.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH, v2] isdn: Fix stack corruption in isdnloop_init()
2009-09-02 12:44 [PATCH] isdn: Fix stack corruption in isdnloop_init() Ingo Molnar
@ 2009-09-02 13:03 ` Ingo Molnar
2009-09-02 13:14 ` Arjan van de Ven
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2009-09-02 13:03 UTC (permalink / raw)
To: linux-kernel, Karsten Keil, isdn4linux
Cc: Andrew Morton, Arjan van de Ven, tj
[ 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: <stable@kernel.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH, v2] isdn: Fix stack corruption in isdnloop_init()
2009-09-02 13:03 ` [PATCH, v2] " Ingo Molnar
@ 2009-09-02 13:14 ` Arjan van de Ven
2009-09-02 13:34 ` Karsten Keil
2009-09-02 14:02 ` [PATCH, v3] " Ingo Molnar
0 siblings, 2 replies; 6+ messages in thread
From: Arjan van de Ven @ 2009-09-02 13:14 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, Karsten Keil, isdn4linux, Andrew Morton, tj
On Wed, 2 Sep 2009 15:03:36 +0200
Ingo Molnar <mingo@elte.hu> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH, v2] isdn: Fix stack corruption in isdnloop_init()
2009-09-02 13:14 ` Arjan van de Ven
@ 2009-09-02 13:34 ` Karsten Keil
2009-09-02 14:02 ` [PATCH, v3] " Ingo Molnar
1 sibling, 0 replies; 6+ messages in thread
From: Karsten Keil @ 2009-09-02 13:34 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: Ingo Molnar, linux-kernel, isdn4linux, Andrew Morton, tj
On Mittwoch, 2. September 2009 15:14:39 Arjan van de Ven wrote:
> On Wed, 2 Sep 2009 15:03:36 +0200
>
> Ingo Molnar <mingo@elte.hu> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH, v3] isdn: Fix stack corruption in isdnloop_init()
2009-09-02 13:14 ` Arjan van de Ven
2009-09-02 13:34 ` Karsten Keil
@ 2009-09-02 14:02 ` Ingo Molnar
2009-09-04 0:16 ` Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2009-09-02 14:02 UTC (permalink / raw)
To: Arjan van de Ven
Cc: linux-kernel, Karsten Keil, isdn4linux, Andrew Morton, tj
* Arjan van de Ven <arjan@infradead.org> 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 <mingo@elte.hu>
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: <stable@kernel.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH, v3] isdn: Fix stack corruption in isdnloop_init()
2009-09-02 14:02 ` [PATCH, v3] " Ingo Molnar
@ 2009-09-04 0:16 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2009-09-04 0:16 UTC (permalink / raw)
To: Ingo Molnar; +Cc: arjan, linux-kernel, isdn, isdn4linux, tj, David S. Miller
On Wed, 2 Sep 2009 16:02:01 +0200
Ingo Molnar <mingo@elte.hu> wrote:
> From: Ingo Molnar <mingo@elte.hu>
> 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 <mingo@elte.hu>
-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 <mingo@elte.hu>
Cc: Arjan van de Ven <arjan@infradead.org>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
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);
_
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-09-04 0:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-02 12:44 [PATCH] isdn: Fix stack corruption in isdnloop_init() Ingo Molnar
2009-09-02 13:03 ` [PATCH, v2] " Ingo Molnar
2009-09-02 13:14 ` Arjan van de Ven
2009-09-02 13:34 ` Karsten Keil
2009-09-02 14:02 ` [PATCH, v3] " Ingo Molnar
2009-09-04 0:16 ` Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.