From: Vadim Lobanov <vlobanov@speakeasy.net>
To: thomas.dahlmann@amd.com
Cc: linux-kernel@vger.kernel.org
Subject: amd5536udc interrupts bug
Date: Wed, 7 Jan 2009 15:10:00 -0800 [thread overview]
Message-ID: <200901071510.00673.vlobanov@speakeasy.net> (raw)
Hello,
>From perusing the code and playing with the module, it seems to me that
the amd5536udc driver's handling of interrupts is currently "bustificated".
The long story:
During the amd5536udc initialization sequence within udc_pci_probe(),
the code attempts to request a shared irq for the device thusly:
request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev)
where 'dev' is the internal state structure. By the time this call is made,
the 'dev' structure is still not fully initialized and contains blank/zero
data for many of the fields, in particular both 'dev->lock' and 'dev->regs'
which are both clearly used within the udc_irq() handler. Those get
initialized a bit later, namely inside the udc_probe() call at the bottom of
udc_pci_probe().
It is my understanding that a handler for a shared interrupt can be
invoked at any time after the corresponding request_irq() call is made,
simply because some other device on the same interrupt may already
be active. This leaves us with a very noticeable race condition, where
udc_irq() can be invoked with uninitialized 'dev' data.
In particular, this effect is very noticeable when I try modprobing the
amd5536udc driver on a kernel that is built with CONFIG_DEBUG_SHIRQ
set. Given that the debug config option forces an invocation of the irq
handler from within the request_irq() function, the immediate effect is a
masterful kernel NULL-dereference OOPS within udc_irq().
The simple fix may be to say that amd5536udc does not support shared
irqs, and to remove the IRQF_SHARED flag from the request_irq() call. A
more complicated fix may be to try to shuffle all the code around to
make sure that 'dev' is fully initialized before we request the irq, but I
don't understand the code well enough (yet) to comfortably do this.
Comments? Thoughts?
On a side note, it occurs to me that the CONFIG_DEBUG_SHIRQ option
went into the kernel a bit less than two years ago, and that it exposes a
very immediate and reproducible OOPS in this driver. Does this mean
that noone uses the 5536 UDC functionality with any recent kernels?
Should I be worried? :)
-- Vadim Lobanov
next reply other threads:[~2009-01-07 23:18 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-07 23:10 Vadim Lobanov [this message]
2009-01-08 2:32 ` Robert Hancock
2009-01-08 3:30 ` Vadim Lobanov
[not found] <49661E83.2070703@arcor.de>
2009-01-08 16:40 ` Thomas Dahlmann
2009-01-08 18:27 ` Vadim Lobanov
2009-01-09 2:02 ` Vadim Lobanov
2009-01-09 11:41 ` Thomas Dahlmann
2009-01-09 22:40 ` Vadim Lobanov
2009-01-10 20:28 ` Thomas Dahlmann
2009-01-12 19:02 ` Vadim Lobanov
2009-01-13 19:19 ` Vadim Lobanov
2009-01-14 12:43 ` Thomas Dahlmann
2009-01-14 22:49 ` Vadim Lobanov
2009-01-15 9:26 ` Thomas Dahlmann
2009-01-17 0:17 ` Vadim Lobanov
2009-01-19 12:23 ` Thomas Dahlmann
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=200901071510.00673.vlobanov@speakeasy.net \
--to=vlobanov@speakeasy.net \
--cc=linux-kernel@vger.kernel.org \
--cc=thomas.dahlmann@amd.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®