[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20110209132529.76927f5f.randy.dunlap@oracle.com>
Date: Wed, 9 Feb 2011 13:25:29 -0800
From: Randy Dunlap <randy.dunlap@...cle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: netdev <netdev@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Karsten Keil <isdn@...ux-pingi.de>
Subject: Re: Linux 2.6.38-rc4 (hysdn: BUG)
On Wed, 9 Feb 2011 11:44:00 -0800 Linus Torvalds wrote:
> On Wed, Feb 9, 2011 at 9:24 AM, Randy Dunlap <randy.dunlap@...cle.com> wrote:
> >
> > on x86_64. no HYSDN hardware found (correct).
> > Nearly allmodconfig.
> >
> >
> > [ 65.397577] HYSDN: module Rev: 1.6.6.6 loaded
> > [ 65.397584] HYSDN: network interface Rev: 1.8.6.4
> > [ 65.398057] HYSDN: 0 card(s) found.
> > [ 65.398121] BUG: unable to handle kernel paging request at ffffffffa06c99f0
> > [ 65.398269] IP: [<ffffffffa06c68ba>] hysdn_getrev+0x2e/0x50 [hysdn]
> > [ 65.398379] PGD 1a14067 PUD 1a18063 PMD 6f6c1067 PTE 800000006ce8c161
> > [ 65.398613] Oops: 0003 [#1] SMP DEBUG_PAGEALLOC
> > [ 65.400030]
> > [ 65.400030] Pid: 2497, comm: modprobe Not tainted 2.6.38-rc4 #1 0TY565/OptiPlex 745
> > [ 65.400030] RIP: 0010:[<ffffffffa06c68ba>] [<ffffffffa06c68ba>] hysdn_getrev+0x2e/0x50 [hysdn]
> > [ 65.400030] RSP: 0018:ffff88006eec1e68 EFLAGS: 00010206
> > [ 65.400030] RAX: ffffffffa06c99f1 RBX: ffffffffa06c99e9 RCX: ffff88007c4159a0
>
> The instruction sequence decodes to
>
> 1e: be 24 00 00 00 mov $0x24,%esi
> 23: 48 89 df mov %rbx,%rdi
> 26: e8 5b 39 c0 e0 callq 0xffffffffe0c03986
> 2b:* c6 40 ff 00 movb $0x0,-0x1(%rax) <-- trapping instruction
>
> which seems to be this
>
> p = strchr(rev, '$');
> *--p = 0;
>
> code. And yes, it's total crap, because while "p" and "rev" are "char
> *", the string that is passed in is actually of type "const char *",
> so that function is seriously broken. It's also seriously broken to
> not test that "p" is non-NULL - the function would just break if there
> is a colon in the string but not a '$'.
>
> And hysdn_procconf_init() passes in a constant string to the thing:
>
> static char *hysdn_procconf_revision = "$Revision: 1.8.6.4 $";
>
> What happens is that it breaks when we mark the constant section as
> read-only, because you have CONFIG_DEBUG_RODATA enabled.
>
> So the fix seems to be to
> - fix the prototype for hysdn_getrev() to not have "const".
> - fix hysdn_procconf_init() to not pass in a constant string to it
>
> The minimal patch would appear to be something like the appended. UNTESTED!
for your patch:
Tested-and-acked-by: Randy Dunlap <randy.dunlap@...cle.com>
> Btw, all of this code seems to go back to before the git history even
> started, so it doesn't seem to be new. I assume you haven't tried
> booting these all-module kernels before? Or is it just the
> DEBUG_RODATA thing that is new for you?
Neither is new. I tested and reported many-modules on 2.6.37-rc1 and
reported these 2 bugs:
https://bugzilla.kernel.org/show_bug.cgi?id=22912
https://bugzilla.kernel.org/show_bug.cgi?id=22882
and that was with CONFIG_DEBUG_RODATA=y.
I don't know how hysdn was missed at that time.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists