Message ID | AANLkTimUg8Dm9mZotubcgPHz8_at=_hnbeWUo-LfSALp@mail.gmail.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
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@oracle.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@oracle.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 netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
drivers/isdn/hysdn/hysdn_defs.h | 2 +- drivers/isdn/hysdn/hysdn_init.c | 2 +- drivers/isdn/hysdn/hysdn_procconf.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/isdn/hysdn/hysdn_defs.h b/drivers/isdn/hysdn/hysdn_defs.h index 729df40..d21b4a0 100644 --- a/drivers/isdn/hysdn/hysdn_defs.h +++ b/drivers/isdn/hysdn/hysdn_defs.h @@ -227,7 +227,7 @@ extern hysdn_card *card_root; /* pointer to first card */ /*************************/ /* im/exported functions */ /*************************/ -extern char *hysdn_getrev(const char *); +extern char *hysdn_getrev(char *); /* hysdn_procconf.c */ extern int hysdn_procconf_init(void); /* init proc config filesys */ diff --git a/drivers/isdn/hysdn/hysdn_init.c b/drivers/isdn/hysdn/hysdn_init.c index b7cc5c2..4ba2123 100644 --- a/drivers/isdn/hysdn/hysdn_init.c +++ b/drivers/isdn/hysdn/hysdn_init.c @@ -53,7 +53,7 @@ static hysdn_card *card_last = NULL; /* pointer to first card */ /* extract revision number from string for log output */ /******************************************************/ char * -hysdn_getrev(const char *revision) +hysdn_getrev(char *revision) { char *rev; char *p; diff --git a/drivers/isdn/hysdn/hysdn_procconf.c b/drivers/isdn/hysdn/hysdn_procconf.c index 96b3e39..1c396e1 100644 --- a/drivers/isdn/hysdn/hysdn_procconf.c +++ b/drivers/isdn/hysdn/hysdn_procconf.c @@ -23,7 +23,7 @@ #include "hysdn_defs.h" static DEFINE_MUTEX(hysdn_conf_mutex); -static char *hysdn_procconf_revision = "$Revision: 1.8.6.4 $"; +static char hysdn_procconf_revision[] = "$Revision: 1.8.6.4 $"; #define INFO_OUT_LEN 80 /* length of info line including lf */