mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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


             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®