From: Rusty Russell <rusty@rustcorp.com.au>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
Linus Torvalds <torvalds@linux-foundation.org>,
Jeremy Fitzhardinge <jeremy@goop.org>,
Adrian Bunk <bunk@stusta.de>
Subject: Re: [BUG 2.6.36-rc6] list corruption in module_bug_finalize
Date: Tue, 5 Oct 2010 14:48:34 +1030 [thread overview]
Message-ID: <201010051448.34984.rusty@rustcorp.com.au> (raw)
In-Reply-To: <alpine.LFD.2.00.1010032141410.14550@localhost6.localdomain6>
On Mon, 4 Oct 2010 06:21:08 am Thomas Gleixner wrote:
> Current mainline triggers a list corruption bug in
> module_bug_finalize(). dmesg excerpt below.
>
> The corresponding code says:
>
> /*
> * Strictly speaking this should have a spinlock to protect against
> * traversals, but since we only traverse on BUG()s, a spinlock
> * could potentially lead to deadlock and thus be counter-productive.
> */
> list_add(&mod->bug_list, &module_bug_list);
>
> I can see the traversal problem vs. BUG(), but what's protecting the
> list_add() ? BKL probably did, but is that true anymore ?
I've never even *seen* this code before :(
Looks like it went through Adrian Bunk to Andrew, but despite the fact that
it (foolishly) doesn't touch kernel/module.c, it's generic code and I should
have seen it. It did change the linux/module.h header.
So, it used to be protected by module_mutex, but Linus and I cleaned that up.
So, we need a lock around this list for adding and removal. I'd use
list_add_rcu to try to help the lockless traversal too...
And moving it from all the archs into kernel/module.c would be a nice bonus.
Nice catch!
Rusty.
next prev parent reply other threads:[~2010-10-05 4:18 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-03 19:51 [BUG 2.6.36-rc6] list corruption in module_bug_finalize Thomas Gleixner
2010-10-04 11:00 ` Arnd Bergmann
2010-10-04 22:43 ` Thomas Gleixner
2010-10-04 23:55 ` Linus Torvalds
2010-10-05 1:11 ` Linus Torvalds
2010-10-05 5:14 ` Rusty Russell
2010-10-05 7:30 ` Thomas Gleixner
2010-10-05 15:34 ` Linus Torvalds
2010-10-05 16:40 ` Thomas Gleixner
2010-10-05 17:17 ` Linus Torvalds
2010-10-05 17:43 ` Thomas Gleixner
2010-10-06 9:10 ` Rusty Russell
2010-10-05 4:18 ` Rusty Russell [this message]
2010-10-05 11:08 ` Adrian Bunk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201010051448.34984.rusty@rustcorp.com.au \
--to=rusty@rustcorp.com.au \
--cc=arnd@arndb.de \
--cc=bunk@stusta.de \
--cc=jeremy@goop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.