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.
Thanks,
Jia-Ju Bai
From: Jia-Ju Bai <[email protected]>
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 <[email protected]>
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.
Thanks!
On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
David Miller <[email protected]> wrote:
> From: Jia-Ju Bai <[email protected]>
> 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 <[email protected]>
> 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.
From: Stephen Hemminger <[email protected]>
Date: Tue, 12 Dec 2017 10:22:40 -0800
> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
> David Miller <[email protected]> wrote:
>
>> From: Jia-Ju Bai <[email protected]>
>> 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 <[email protected]>
>> 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.
On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
David Miller <[email protected]> wrote:
> From: Stephen Hemminger <[email protected]>
> Date: Tue, 12 Dec 2017 10:22:40 -0800
>
> > On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
> > David Miller <[email protected]> wrote:
> >
> >> From: Jia-Ju Bai <[email protected]>
> >> 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 <[email protected]>
> >> 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.
On 2017/12/13 13:18, Stephen Hemminger wrote:
> On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
> David Miller <[email protected]> wrote:
>
>> From: Stephen Hemminger <[email protected]>
>> Date: Tue, 12 Dec 2017 10:22:40 -0800
>>
>>> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
>>> David Miller <[email protected]> wrote:
>>>
>>>> From: Jia-Ju Bai <[email protected]>
>>>> 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 <[email protected]>
>>>> 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,
Jia-Ju Bai
On Wed, 13 Dec 2017 15:42:56 +0800
Jia-Ju Bai <[email protected]> wrote:
> On 2017/12/13 13:18, Stephen Hemminger wrote:
> > On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
> > David Miller <[email protected]> wrote:
> >
> >> From: Stephen Hemminger <[email protected]>
> >> Date: Tue, 12 Dec 2017 10:22:40 -0800
> >>
> >>> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
> >>> David Miller <[email protected]> wrote:
> >>>
> >>>> From: Jia-Ju Bai <[email protected]>
> >>>> 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 <[email protected]>
> >>>> 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?
On 2017/12/14 0:50, Stephen Hemminger wrote:
> On Wed, 13 Dec 2017 15:42:56 +0800
> Jia-Ju Bai <[email protected]> wrote:
>
>> On 2017/12/13 13:18, Stephen Hemminger wrote:
>>> On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
>>> David Miller <[email protected]> wrote:
>>>
>>>> From: Stephen Hemminger <[email protected]>
>>>> Date: Tue, 12 Dec 2017 10:22:40 -0800
>>>>
>>>>> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
>>>>> David Miller <[email protected]> wrote:
>>>>>
>>>>>> From: Jia-Ju Bai <[email protected]>
>>>>>> 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 <[email protected]>
>>>>>> 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?
Hi,
Thanks for your appreciation of my tool :)
I think it is possible. But before releasing, I need to improve it to
solve some problems:
1. It produces some false positives and negatives. Thus I need
developer's response and confirmation of my bug reports, to help me to
improve my tool's accuracy.
2. It is based on Clang-3.2 (a LLVM-based compiler, not GCC), and some
driver code cannot compile normally. Maybe I should re-implement my tool
based on a recent Clang version.
3. Some software is needed when running my tool, thus as MySQL (maybe I
directly can write to files, instead of database in the future) and
Clang (Clang is necessary).
I think it may be difficult to directly put my tool in "tools"
directory. But with some configuration, it is possible to run it
automatically in a testing environment.
I also plan to open this tool in the future, after finishing the
improvements :)
Thanks,
Jia-Ju Bai