Messages in this thread | | | Date | Sat, 4 Oct 2008 13:02:18 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [RFC PATCH 02/12] On Tue, 23 Sep 2008, David Miller wrote: |
| |
On Sat, 4 Oct 2008, Jiri Kosina wrote: > On Fri, 3 Oct 2008, Jesse Brandeburg wrote: > > Our experience is different. We are also testing with the "protection > > patch" reverted. > > We see that the problem specifically comes and goes when > > removing/adding the use of set_memory_ro/set_memory_rw to the driver. > > But if this patch (which is an obvious workaround, compared to the other > patches which fix real bugs, right?) would be catching some malicious > accessess to the mapped EEPROM, there should be stacktraces present in the > kernel log, right?
Exactly. The access to a ro region results in a fault. I have nowhere seen that trigger, but I can reproduce the trylock() WARN_ON, which confirms that there is concurrent access to the NVRAM registers. The backtrace pattern is similar to the one you have seen.
There are two possible bad results from that concurrent access:
1) Task A issues command A Task B issues command B Task A writes data for A which end up in B
2) Task A acquires the software flag ......
Task B acquires the software flag
Task A releases the software flag
The firmware accesses NVRAM Task B accesses the NVRAM Both are probably serious enough to result in random NVRAM corruption. There is no doubt: The missing serialization is a real bug.
Your question why this just happens now, while the bug is there for ever, is definitely a good one. My opinion on that is that we just have been lucky or some minor modification somewhere else in the e1000e code or even in the generic/architecture code removed an accidental serializing effect.
I was not able to reproduce the trylock warning on Fedora 8, but Fedora 10-Beta triggers it once in 50 boots. I'm not going to remove the mutex to verify whether it actually would corrupt the NVRAM :)
In theory we should be able to reproduce the problem with older kernel versions as well. Maybe not the corruption, but we might see the mutex_trylock check trigger.
Thanks,
tglx
| |