netdev - Re: [e1000e] BUG triggered in blink path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Nov 2010 15:59:31 -0800
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	e1000-devel@...ts.sourceforge.net, netdev@...r.kernel.org
Subject: Re: [e1000e] BUG triggered in blink path

On Thu, Nov 11, 2010 at 02:17, Holger Eitzenberger
<holger@...zenberger.org> wrote:
> Hi Jesse,
>
> I've attached the patch against net-next-2.6.  Please check if it's ok
> for you.  I checked e1000, igb and ixgbe as well, they don't have that
> problem.
>
>  /holger
>

Patch looks good, I have added the patch to my tree.

Cheers,
Jeff

>> > After taking a look I think this may be caused by initializing
>> > adapter->led_blink_task several times in e1000_phys_id(), while possibly
>> > led_blink_task is running:
>> >
>> >     if ((hw->phy.type == e1000_phy_ife) ||
>> >         (hw->mac.type == e1000_pchlan) ||
>> >         (hw->mac.type == e1000_82574)) {
>> >             INIT_WORK(&adapter->led_blink_task, e1000e_led_blink_task);
>> >             if (!adapter->blink_timer.function) {
>> >
>> > I can't reproduce it after moving it inside the following if block,
>> > but I'm not quite sure if this catches all races in there.  Especially
>> > the msleep_interruptible() may be too optimistic because it may
>> > actually not wait long enough.  Someone with more knowledge of the
>> > driver should take a look.
>>
>> thanks for your investigation and troubleshooting.  I don't think it is
>> correct at all to be calling INIT_WORK more than once.  In fact the
>> INIT_WORK should just be moved into probe, and then e1000_phys_id should
>> just do schedule_work.
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html