netdev - Re: watchdog timeout panic in e1000 driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <45DB1D7B.9020008@intel.com>
Date:	Tue, 20 Feb 2007 08:10:35 -0800
From:	Auke Kok <auke-jan.h.kok@...el.com>
To:	Kenzo Iwami <k-iwami@...jp.nec.com>
CC:	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	netdev@...r.kernel.org, "Ronciak, John" <john.ronciak@...el.com>
Subject: Re: watchdog timeout panic in e1000 driver

Kenzo Iwami wrote:
> Hi,
> 
> I created a patch that uses watchdog_task but fixes the race condition
> that occurred in old the e1000 driver.
> 
> I've obtained information about the panic caused by the old e1000 driver
> using e1000_watchdog_task. According to the crash dump, the panic was
> caused by a timer_list whose contents were NULLs. Further trace
> information revealed that the function in the timer list was
> e1000_watchdog().
> 
> This function is registered in timer_list during e1000_watchdog_task.
> It seems that e1000_watchdog_task could be called after the adapter is
> removed, and freed memory is registered to timer_list.
> 
> By looking at the source code, e1000_watchdog_task will be scheduled if
> e1000_watchdog is invoked during e1000_remove, after flush_scheduled_work()
> is called, but before del_timer_sync() is called in e1000_down().
> 
> The attached patch adds back the e1000_watchdog_task(), but it will
> prevent the old race condition from happening by deleting e1000_watchdog
> from timer_list before flush_scheduled_work() is called.


Kenzo,

this looks a lot better than the previous patch!! However, we already have a 
state marker for _down_ that we should probably reuse. Can you try the attached 
patch and see if it works for you? It's basically your patch without the added 
remove flag and instead using the already available atomic state trackers.

If this works for you then that is great news and I'll push this patch to the 
upstream kernel maintainers after testing.

Cheers,

Auke




View attachment "e1000_git_kenzo_use_state_flags.patch" of type "text/x-patch" (2556 bytes)