From: Stephen Hemminger <stephen@networkplumber.org>
To: Jia-Ju Bai <baijiaju1990@gmail.com>
Cc: David Miller <davem@davemloft.net>,
mlindner@marvell.com, shemminger@osdl.org,
shemminger@linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [BUG] skge: a possible sleep-in-atomic bug in skge_remove
Date: Wed, 13 Dec 2017 08:50:06 -0800 [thread overview]
Message-ID: <20171213085006.7093c27e@xeon-e3> (raw)
In-Reply-To: <f2dee82a-af75-a3af-9899-5dec7950b9e8@gmail.com>
On Wed, 13 Dec 2017 15:42:56 +0800
Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> On 2017/12/13 13:18, Stephen Hemminger wrote:
> > On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
> > David Miller <davem@davemloft.net> wrote:
> >
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Date: Tue, 12 Dec 2017 10:22:40 -0800
> >>
> >>> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
> >>> David Miller <davem@davemloft.net> wrote:
> >>>
> >>>> From: Jia-Ju Bai <baijiaju1990@gmail.com>
> >>>> Date: Tue, 12 Dec 2017 16:38:12 +0800
> >>>>
> >>>>> According to drivers/net/ethernet/marvell/skge.c, the driver may sleep
> >>>>> under a spinlock.
> >>>>> The function call path is:
> >>>>> skge_remove (acquire the spinlock)
> >>>>> free_irq --> may sleep
> >>>>>
> >>>>> I do not find a good way to fix it, so I only report.
> >>>>> This possible bug is found by my static analysis tool (DSAC) and
> >>>>> checked by my code review.
> >>>> This was added by:
> >>>>
> >>>> commit a9e9fd7182332d0cf5f3e601df3e71dd431b70d7
> >>>> Author: Stephen Hemminger <shemminger@vyatta.com>
> >>>> Date: Tue Sep 27 13:41:37 2011 -0400
> >>>>
> >>>> skge: handle irq better on single port card
> >>>>
> >>>> I think the free_irq() can be moved below the unlock.
> >>>>
> >>>> Stephen, please take a look.
> >>> The IRQ was being free twice.
> >>> How did you see it, I really doubt any multi-port SKGE cards
> >>> still exist.
> >> He sees it by reading the code, please take a look at this
> >> and move the free_irq() out of the spin locked section since
> >> it can sleep.
> > Thanks, I was hoping for some automated static analysis tool.
>
> This bug was found by an automated static analysis tool named DSAC,
> which is written by myself.
> Then I manually checked driver source code, and finally sent the bug report.
Thanks.
Would it be possible to put tool in tools directory and then have
it automated by kbuild robot?
next prev parent reply other threads:[~2017-12-13 16:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-12 8:38 [BUG] skge: a possible sleep-in-atomic bug in skge_remove Jia-Ju Bai
2017-12-12 13:34 ` David Miller
2017-12-12 18:22 ` Stephen Hemminger
2017-12-13 1:57 ` David Miller
2017-12-13 5:18 ` Stephen Hemminger
2017-12-13 7:42 ` Jia-Ju Bai
2017-12-13 16:50 ` Stephen Hemminger [this message]
2017-12-14 3:06 ` Jia-Ju Bai
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=20171213085006.7093c27e@xeon-e3 \
--to=stephen@networkplumber.org \
--cc=baijiaju1990@gmail.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mlindner@marvell.com \
--cc=netdev@vger.kernel.org \
--cc=shemminger@linux-foundation.org \
--cc=shemminger@osdl.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox